Проект советника - страница 4

 
Vitaly Muzichenko:

Мне тоже, но как-то давно пришёл к такому выводу, что код должен быть компактный в тех местах, где в него никогда не смотрят, он никогда не правится, и правится не будет.

Разбрасывать пользовательский код по инклудам, это создать ещё одну дополнительную головную боль, чтоб перетащить файл в другой терминал, или им поделиться, нужно таскать не один файл, а несколько. Можно конечно перенести во все терминалы инклудники, но если в одном что подправил или добавил, тогда нужно во всех сделать замену на новый.

Советники и индикаторы настолько малы, что не имеет смысла что-то куда отдалять от тела программы. Вернее они не малы, а однофайловые, это-же не сайт в 10 000 страниц, где без класса и инклудов не обойтись. Тем более, сейчас есть структуры, и их вполне достаточно, чтоб писать компактные на 100% работоспособные коды.

Ну приехали.... А насчет символьных ссылок на папки вы в курсе? http://skesov.ru/sozdanie-simvolnoy-ssyilki-dlya-papki/

У меня все мои библиотеки лежат в одной папке, а в куче терминалов, их несколько десятков, в папках mql*\includes находятся символьные ссылки на эту реальную папку. Ничего никуда таскать не нужно.

Также я еще активно пользуюсь хранилищем, если там держать все важное, зв 5 секунд можно скачать это в другой терминал. Но для либ символьные ссылки удобнее, всегда полная синхронизация.

Создание символьной ссылки для папки в Windows 8.1, 8, 7, Vista
Создание символьной ссылки для папки в Windows 8.1, 8, 7, Vista
  • 2013.07.24
  • skesov.ru
Доброго времени суток! Сегодня рассмотрим интересную тему под названием «Символьные ссылки». Вариантов использования данного инструмента не так уж много. К примеру, если вы используете часть оперативной памяти как RAM-диск, можно перенести какую-либо игру или её часть (скажем папки с графикой) и создать символьную ссылку. Это значительно...
 
Alexey Navoykov:
Рекомендую использовать символические ссылки или junction-связи для папки MQL. Все терминалы будут смотреть в одну папку.

А поделится с кем-то?

 
Vitaly Muzichenko:

А поделится с кем-то?

Ну тут уж надо определяться, что вам важнее, шашечки или ехать )   Неужели простота процесса деления с кем-то вам важнее комфорта кодирования?   Если вы допустим обнаружили ошибку в какой-то функции, используемой многими советниками, то вам придётся залезать в код каждого из них и переправлять эту функцию - вас как программиста это не коробит?

 
Спасибо всем за обсуждение моего вопроса. 
Решил для начала смотреть в сторону ООП в mqlx, хранить свои (повторяемые) функции в отдельном файле (файлах). И не лениться комментировать. 

И ещё + за символическую ссылку в Windows! В линуксе как то использовал, а про Windows забыл. Нужно будет попробовать.. 
 
Alexey Navoykov:

Ну тут уж надо определяться, что вам важнее, шашечки или ехать )   Неужели простота процесса деления с кем-то вам важнее комфорта кодирования?   Если вы допустим обнаружили ошибку в какой-то функции, используемой многими советниками, то вам придётся залезать в код каждого из них и переправлять эту функцию - вас как программиста это не коробит?

У меня был такой случай всего один раз, где-то месяц назад, но там пришлось добавить проверку на открытость рынка, есть все проверки, кроме этой, и вылезло это первый раз за всё время использования.

Если что нужно добавить, то и добавляю в текущую программу, и после этого уже использую файл в качестве шаблона для следующей программы. В итоге за несколько лет в шаблоне есть всё, ну или почти всё, так что бот любой сложности пишется за пол-часа.

Весь исполнительный код помещается в один экран, хотя в файле чуть более 4 000 строк, но Я туда смотрю крайне редко, если только что нужно добавить. От функций в циклах отказался, используется всего два, один по открытым собирает информацию, второй по истории, и всё это в структуре в самом низу кода. Всё очень просто, и рядом. Основной код комментирован. Проект расширяется крайне просто и быстро, без каких либо потерь.

 
Alexey Volchanskiy:

Выглядит вкусно, можно еще посмотреть TRACE_*** и ASSERT?

Ну... Для автора мастер-класса по охмурению баб, которому я завидую черной завистью - всегда пожалуйста.

Дебаг-версия у меня включается автоматически, если определен соответствующий системный макрос. Однако, если он не включен - можно также включить ассерты и трейсы дефайнами:

#define _FORCETRACE 1
#define _FORCEASSERT 1

В этом случае - независимо от системных установок - генерируются дебаг-трейсы и дебаг-ассерты.

Подключение этих макросов у меня осуществляется директивой:

#include <MyLib\DebugOrRelease\DebugSupport.mqh>

Эта директива подключает все необходимые файлы и определения. Все они у меня лежат в отдельной папке DebugOrRelease. Прикрепляю их. (Код писался давно, в основном "на скорую руку", поэтому не столь красивый, как в интерфейсах и классе истории). Сами ассерты и трейсы для дебаг-версии лежат в файлах AssertD и TraceD, реально это функции PerformAssert() и PerformTrace()

Кроме того, эти файлы и макросы используют глобальный лог-файл (если установлен вывод в лог-файл), я его уже когда-то постил, но, еще раз. Лог файл лежит у меня в папке "Common"

Файлы:
 
Andrey Kisselyov:

хорошая работа, мне нравиться, но мне не нравиться ООП и стараюсь обходиться без него. как и не нравятся процессоры, с разделением потоков (пример 4 ядра и 8 потоков). должно быть понятно что разделение и любая виртуализация это потеря производительности и потеря машинного времени на ее осуществление, будь то разделение потоков в ядре или виртуализация функций в коде.

краткость сестра таланта, я думаю так лучше звучит.

Я давно убедился, что удобство поддержки и реюза кода - куда более важно, чем уменьшение производительности.

ООП - мне очень сильно помогает, когда я возвращаюсь к коду спустя некоторое время для модификации. Не говорю уж об повторном использовании.

Но, согласен, далеко не всегда надо использовать ООП.

Скажем, у меня есть класc CDataProvider:pulic CDataProviderI - дата провайдер, который обеспечивает эксперт таймсериями, индикаторами, данными по терминалу и окружению. Внутри эксперта может быть много ТС - каждая будет получать указатели на таймсерии и индикаторы от дата-провайдера (в результате чего не надо каждой ТС заботиться о создании этих таймсерий - дата-провайдер даст указатель на нужную таймсерию, если она уже есть, а создавать будет только те, которые еще не созданы).

Когда надо получить индикатор из дата-провайдера - надо заполнить структуру описания индикатора, и потом запросить у провайдера индикатор, указав на эту структуру.

Соответственно, каждый индикатор внутри дата-провайдера должен уметь опознавать свою структуру (дата-провайдер "знаком" только с абстрактным базовым классом структуры), и по ней - создать готовый объект индикатора - который и будет выдан дата-провайдером.

Но, городить весь этот огород ради того, чтобы проверить новую идею какого-то индикатора - конечно, неразумно. В результате - для таких новых индикаторов - все "собирается на коленке", безо всякого ООП. Однако, если я вижу, что индикатор полезен для советников - то он пишется "как положено" - с полной ООП-поддержкой, и созданием внутри дата-провайдера.

P.S.

Кстати, вот в случае с индикаторами и дата-провайдером хорошо видно преимущество в виртуализации наследовании.  У нас есть базовый интерфейс параметров индикатора  CIndicatorParametersI, наследник этого интерфейса - это реальные параметры необходимого индикатора. При запросе индикатора мы объявляем эти параметры, и передаем дата-провайдеру указатель на абстрактный интерфейс. Таким образом, сам дата-провайдер даже не знает, какой индикатор запрошен - это определяется в одной функции, в которой индикатор создается по new, того типа, который нужен. И про то, какие именно параметры переданы - знает только этот созданный индикатор, который и извлекает нужные параметры из переданного объекта.

Фишка в том, что практически везде внутри дата-провайдера идет работа с простым базовым классом параметров (или индикаторов) - дата-провайдеру доступны только самые простые и общие функции базовых интерфейсов. Это упрощает модификацию кода (когда она необходима), и не создает соблазн "влезать" в код индикаторов из дата-провайдера. Если нужно изменить индикатор - то это делается только внутри самого индикатора, дата-провайдер же - это только хранилище индикаторов, максимум, что он может сделать - это создать новый индикатор.

 
George Merts:

Кстати, меня очень нервирует, когда вложенность более двух уровней.  Стараюсь так никогда не писать, раскидывая код по функциям.

И даже когда два уровня вложенности - обязательно после каждой закрывающей скобки - пишу комментарии, какой блок она зарывает (скажем, дублирую заголовок цикла).

Что касается стиля, то вот мой код выбора исторической позиции для МТ5 (по указанному магику, символу, с указанным диапазоном дат):

При этом сам класс истории - это наследник абстрактного  интерфейса CTradeHistoryI:

Выбрав необходимую историю - можно пересчитать ее компоненты (позици для МТ5 или ордера для МТ4), и получить интерфейс на любую компоненту в виде абстрактного интерфейса:

Для МТ4 - есть соответствующие классы истории, также наследующиеся от этих интерфейсов - таким образом, заодно обеспечивается и кропссплатформенность - советнику совершенно не надо выяснять, где он работает, вся работа с историей ведется через абстрактные интерфейсы.


Не много критики:

class CTradePosComponentI: public CMyObject
{
...
}

Зачем изобретать велосипед в виде CMyObject, когда есть стандартный и всем понятный CObject?

class class CTradeHistoryI: public CMyObject
{
// Расширенный интерфейс
   virtual void Sort(ESortTPCMode stmMode = STM_BY_OPEN_TIME_A) = 0;
}

Здесь уже явно копируется функционал CObject и CArrayObj. Зачем? Быстрая сортировка встроена в стандартные контейнеры данных. Используйте их.

class CTradePosComponentI: public CMyObject
{
public:
   void CTradePosComponentI() {    SetMyObjectType(MOT_TRADEPOS_COMPONENT_I); };
   virtual void ~CTradePosComponentI() {};
}

Если класс интерфейс - его конструктор лучше скрыть в protected секции. Тогда его объект невозможно будет создать непосредственно.

Определять пустой диструктор? Ну не знаю. Я бы так не делал. О диструкторе, если он не нужен, лучше не упоминать.

for(iI=0;iI<iHistoryDealsTotal; ++iI)
...

Нестандартный инкремент iI, нестандартная итерация ++iI, iHistoryDealsTotal - определен где-то там, далеко перед циклом. Лучше проще:

for(int i = 0; i < HistoryDealsTotal();i++)

Это также быстро как и в предыдущем варианте, но гораздо очевидней.

virtual bool               IsTPCInUnloss() const { if(GetTPCStopLoss() <= 0 || GetTPCStopLoss() == EMPTY_VALUE) return(false); if(GetTPCType() == POSITION_TYPE_BUY) { if(GetTPCStopLoss() >= GetTPCOpenPrice()) return(true); } else { if(GetTPCStopLoss() <= GetTPCOpenPrice())return(true); }; return (false); };

Сами вроде против таких простыней, но сами же кое-где такое пишите. Такую байду никто разбирать не захочет. Что мешало написать так:

virtual bool IsTPCInUnloss() const
{
   if(GetTPCStopLoss() <= 0 || GetTPCStopLoss() == EMPTY_VALUE)
      return(false);
   if(GetTPCType() == POSITION_TYPE_BUY)
   { 
      if(GetTPCStopLoss() >= GetTPCOpenPrice())
         return(true);
   } 
   else
   {
     if(GetTPCStopLoss() <= GetTPCOpenPrice())
        return(true);
   }; 
   return (false);
};

Ну и ';' в конце фигурных скобок - это устарело, сейчас так уже не надо делать.

Гигантский метод Select состоит из гиганского цикла for:

for(iI=0;iI<iHistoryDealsTotal; ++iI)
      {
      ulCurTicket = HistoryDealGetTicket(iI);
      
      if(ulCurTicket == 0)
         return(WRONG_VALUE);
      
      // Получим направление сделки   
      if(HistoryDealGetInteger(ulCurTicket,DEAL_ENTRY,lCurEntry)!=true)
         {
         TRACE_INTEGER("Не удалось получить направление сделки ! Тикет: ",ulCurTicket);
         continue;
         };
      
      // Проверим направление сделки
      if(lCurEntry != DEAL_ENTRY_OUT)
         continue;
      
      // Получим магик сделки
      if(HistoryDealGetInteger(ulCurTicket,DEAL_MAGIC,lCurMagic)!=true)
         {
         TRACE_INTEGER("Не удалось получить магик сделки ! Тикет: ",ulCurTicket);
         continue;
         };
         
      // Проверим магик
      if(ulMagic != NULL && lCurMagic != ulMagic)
         {
         //TRACE_INTEGER("Сделка не подходит ! Имеет неверный магик ! Magic сделки: ",lCurMagic);
         //TRACE_INTEGER("Требуемый Magic : ",ulMagic);
         continue;
         };
      ...
}

Очевидно, что все проверки на соответствие сделки текущему эксперту лучше вынести в отдельный метод, например так:

for(iI=0;iI<iHistoryDealsTotal; ++iI)
{
   if(!CheckDeal(iI))
      continue;
   ...
}

И вообще, select нужно дробить еще на 3-4 метода, что бы было понятно то, что в нем происходит.

George Merts
При этом сам файл советника состоит из пяти строк. В этом файле объявляется сам объект фабрики частей эксперта, и подключаются инклюды.

Фабрика очень спорный паттерн. Используйте - хорошо, но не рекомендую все делать через фабрику.

George Merts
И даже когда два уровня вложенности - обязательно после каждой закрывающей скобки - пишу комментарии, какой блок она зарывает (скажем, дублирую заголовок цикла). 

Дык, потому и пишите, что скобки расставляете по мерзкому способу от MQL. Если б писали так:

if(OrderSelect())
{
   ...
}

Всегда видели бы какая скобка, какой блок кода закрывает.

Можно было бы еще с десяток предупреждений найти в Вашем коде. Код не идеален конечно, но уже чувствуется тяга автора к прекрасному:))

 
Vasiliy Sokolov:

Немного критики:

О. Вот такие обсуждения я люблю. Итак.

Зачем изобретать велосипед в виде CMyObject, когда есть стандартный и всем понятный CObject?

Здесь уже явно копируется функционал CObject и CArrayObj. Зачем? Быстрая сортировка встроена в стандартные контейнеры данных. Используйте их.

CMyObject - и есть наследник от стандартного CObject, все списки и массивы в моем коде - это наследники от CArray (и других массивов стандартной библиотеки). Я почти никогда не использую стандартные массивы array[].

И, понятное дело, сортировка и работа со списком использует базовую функциональность CObject.

А разница у них такая: Стандартный CObject - это "объект списка или сортированного массива". CMyObject - это CObject, имеющий определенный тип, и содержащий некоторое значение, данное при его создании. Этот объект мне понадобился всвязи с повсеместным приведением объектов к базовому абстрактному классу - чтобы понимать по указателю, на какой именно объект "на самом деле" указывает. Тип CMyObject - устанавливается как раз той самой функцией SetMyObjectType(). Эта функция в обязательном порядке вызывается в конструкторах любых наследников от CMyObject, чтобы назначить идентификатор класса, к которому принадлежит создаваемый объект.

Также есть в нем и функция SetUDCreationValue() - установка значения, определяемого пользователем при создании. Редко используемая. Она нужна для того, чтобы различать различные объекты одного и того же класса.

Если класс интерфейс - его конструктор лучше скрыть в protected секции. Тогда его объект невозможно будет создать непосредственно.

Protected конструктор ???  Ну... Да, пожалуй, разумно для интерфейсов, я не знал, что так можно.

Определять пустой диструктор? Ну не знаю. Я бы так не делал. О диструкторе, если он не нужен, лучше не упоминать.

Это - "проклятое наследие прошлого". Когда-то писали довольно большой проект, так вот там, если не определить пустой деструктор - то удаление объектов почему-то происходило довольно долго. Так что я с тех пор уже давно на автомате так делаю. Деструктор, вобще говоря, еще и должен быть виртуальным.

Нестандартный инкремент iI, нестандартная итерация ++iI, iHistoryDealsTotal - определен где-то там, далеко перед циклом. Лучше проще:

Не согласен. Инкремент - совершенно обычный, - i, просто стандартизированная нотация - сперва с маленькой буквы ее тип integer, и потом - с большой буквы ее называние I.

Сами вроде против таких простыней, но сами же кое-где такое пишите. Такую байду никто разбирать не захочет. Что мешало написать так:

В данном случае - пришлось выбирать между "обозримостью" класса и красотой этой функции. Выбрал "обозримость". Красота - пострадала.

Гигантский метод Select состоит из гиганского цикла for:

Очевидно, что все проверки на соответствие сделки текущему эксперту лучше вынести в отдельный метод, например так:

И вообще, select нужно дробить еще на 3-4 метода, что бы было понятно то, что в нем происходит.

Согласен. Тут, в принципе, этот самый цикл "разросся", сперва он был не так велик.

Хотя, выносить мелкие проверки в приватные функции - не всегда удобно, за этими проверками потом в коде не всегда уследишь.

Фабрика очень спорный паттерн. Используйте - хорошо, но не рекомендую все делать через фабрику.

Сейчас уже не помню, было несколько вариантов построения советника. Остановился на "фабрике частей эксперта". В принципе, сейчас это уже не чистый классический паттерн "фабрика". Изначально-то он планировался "классическим", но сейчас это уже скорее просто "конструктор-концентратор" частей эксперта. И за удаление этих частей - отвечает также он, что для фабрики несвойственно. Ну а название - осталось.

Дык, потому и пишите, что скобки расставляете по мерзкому способу от MQL. Если б писали так:

Всегда видели бы какая скобка, какой блок кода закрывает.

Почему "по мерзкому" ?

Заголовок цикла, дальше открывающая скобка с отступом, далее - весь блок с тем же отступом, и наконец, закрывающая скобка - также с отступом.

А как по-вашему, лучше ?

 
Gregory Kovalenko:
 

Мне нужно получить профиты по 2м открытым ордерам. Последний открытый ордер, называю OrderProfit2, а следующий - OrderProfit1.

Сначала открылся 1, потом 2й ордер, по-этому 1 попавшийся в цикле, у меня называется 2 )

Где ошибка?

Вы просто перебираете ордера. Нигде нет проверки, какой из них первый, а какой второй.

Вам надо ввести проверку на время открытия. Тогда вы сможете отличить ордер, открывшийся раньше, и ордер, открывшийся позже. А возможно, они могут открыться и одновременно.

Причина обращения: