Adviser's project - page 3

New comment
 
Alexey Volchanskiy:

And then you have to look up what those four brackets at the bottom refer to.

By the way, it makes me very nervous when nesting is more than two levels. I try never to write it that way, spreading code over functions.

And even when there are two levels of nesting - be sure to write comments after each closing bracket - which block it buries (say, duplicate loop header).

As for style, here's my code for selectinga history position for MT5 (by a specified magician, symbol, with a specified date range):

int CMT5TradeHistory::Select(ulong ulMagic,ECurrencySymbol csSymbol,datetime dtFrom = MIN_DATETIME,datetime dtTill = NEVER_EXPIRES)
{
   ASSERT(dtFrom <= dtTill);

   // Очистим список ядер позиции
   m_aoPosCores.Clear();
   
   // Запросим историю ордеров и сделок
   if(HistorySelect(dtFrom,dtTill)!=true)
      return(WRONG_VALUE);
   
   // Соберем тикеты исторических позиций
   // Просмотрим все сделки выхода, и выпишем оттуда тикеты позиций.
   int iHistoryDealsTotal=HistoryDealsTotal();

   CArrayLong   alHistoryPosIDs;
   int iI = WRONG_VALUE;
   ulong ulCurTicket = 0;
   long lCurPosID = 0;
   long lCurMagic = 0;
   long lCurEntry = 0;
   string strCurSymbol;
   
   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;
         };
      
      // Получим символ сделки
      if(HistoryDealGetString(ulCurTicket,DEAL_SYMBOL,strCurSymbol)!=true)
         {
         TRACE_INTEGER("Не удалось получить символ ордера ! Тикет: ",ulCurTicket);
         continue;
         };
      
      // Проверим символ
      if(csSymbol != CS_UNKNOWN)
         if(csSymbol == CS_CURRENT)
            {
            if(_Symbol2CurrencyEnum(strCurSymbol) != _Symbol2CurrencyEnum(Symbol()))
               {
               //TRACE2("Symbol выбираемой позиции: ",_Enum2CurrencySymbol(csSymbol));
               //TRACE2("Выбранный ордер имеет неверный символ: ",strCurSymbol);
               continue;
               };
            }
         else 
            {
            if(_Symbol2CurrencyEnum(strCurSymbol) != csSymbol)
               {
               //TRACE2("Symbol выбираемой позиции: ",_Enum2CurrencySymbol(csSymbol));
               //TRACE2("Выбранный ордер имеет неверный символ ! Символ ордера: ",poiBuffer.GetSymbolString());
               continue;
               };
            };

      // Получим ID позиции
      if(HistoryDealGetInteger(ulCurTicket,DEAL_POSITION_ID,lCurPosID)!=true)
         {
         TRACE_INTEGER("Не удалось получить ID позиции ! Тикет: ",ulCurTicket);
         continue;
         };
         
      // Проверим ID позиции
      if(lCurPosID <= NULL)
         continue;        
         
      if(alHistoryPosIDs.Add(lCurPosID)!=true)
         return(WRONG_VALUE);
      };  // цикл перебора всех сделок
   
   // Здесь ID всех позиций собраны в массиве alHistoryPosIDs, необходимо убрать повторения (в позиции может быть много ордеров)
   CArrayLong alUnicalHistoryPosIDs;   
   
   if(_DeleteDoubles(GetPointer(alHistoryPosIDs),GetPointer(alUnicalHistoryPosIDs))!=true)
      return(WRONG_VALUE);

   TRACE_INTEGER("Уникальных ID позиций в истории: ",alUnicalHistoryPosIDs.Total());

   // Здесь массив alUnicalHistoryPosIDs заполнен уникальными ID позиций в истории.
   // Заполним ядра позиции
   CMT5HistoryPositionInfoCore* phpiHistPosCore = NULL;
   
   for(iI=0;iI<alUnicalHistoryPosIDs.Total(); ++iI)
      {
      //TRACE_INTEGER("Выберем позицию: ",iI);
      
      // Выберем очередной тикет
      lCurPosID = alUnicalHistoryPosIDs.At(iI);

      // Позиция является нужной компонентой 
      ASSERT(phpiHistPosCore == NULL);
      
      phpiHistPosCore = new CMT5HistoryPositionInfoCore;
      
      if(phpiHistPosCore == NULL)
         {
         m_aoPosCores.Clear();
         ASSERT_DSC(false,"Не удалось создать объект CMT5HistoryPositionInfoCore по new");
         return(WRONG_VALUE);
         };
      
      ASSERT_MYPOINTER(phpiHistPosCore);

      if(phpiHistPosCore.SelectByID(lCurPosID)!=true)
         {
         TRACE("Не удалось создать выбрать позицию ! Возможно, позиция открыта, и еще не полностью в истории.");
         TRACE_INTEGER("ID невыбранной позиции: ",lCurPosID);
         delete phpiHistPosCore;
         phpiHistPosCore = NULL;
         continue;
         };
      
      ASSERT(phpiHistPosCore.GetTPCOpenTime() > MIN_DATETIME && phpiHistPosCore.GetTPCOpenTime() < phpiHistPosCore.GetTPCCloseTime() && phpiHistPosCore.GetTPCCloseTime() < NEVER_EXPIRES);
      
      // Найдена и выбрана еще одна компонента позиции
      if(m_aoPosCores.Add(phpiHistPosCore) == false)
         {
         delete phpiHistPosCore;
         m_aoPosCores.Clear();
         ASSERT_DSC(false,"Не удалось добавить новый объект в список ядер позиции");
         return(WRONG_VALUE);
         };
      
      phpiHistPosCore = NULL;   
      }; // цикл перебора уникальных PosID

   // TRACE_INTEGER("Ядер в выбранной позиции: ",m_aoPosCores.Total());     
   
   return(m_aoPosCores.Total());
};

The history class itself is a descendant of the CTradeHistoryI abstract interface:

class CTradeHistoryI: public CMyObject
{
public:
   void CTradeHistoryI() {    SetMyObjectType(MOT_TRADE_HISTORY_I); };
   virtual void ~CTradeHistoryI() {};
   
   // Выбор существующей истории. 
   // Указывается магик и символ, по которому выбираются исторические ордера, а также промежуток времени, в котором необходимо искать их.
   // Если ulMagic = 0 - выбираются все позиции по всем магикам.
   // Если ECurrencySymbol = CS_UNKNOWN - выбираются все позиции по всем символам
   // Если ECurrencySymbol = CS_CURRENT - запрашивается функция Symbol(), и выбираются все позиции по этому символу
   // Возвращает число компонент позиции внутри истории (может быть нулевым если ничего не найдено) или WRONG_VALUE в случае ошибок
   // NOTE !!! 
   // При выборе - отложенные ордера не учитываются.
   virtual int Select(ulong ulMagic = 0,ECurrencySymbol csSymbol = CS_CURRENT,datetime dtFrom = MIN_DATETIME,datetime dtTill = NEVER_EXPIRES) = 0;

   virtual uint GetTotalComponents() const = 0;  // Получение общего числа компонент
   virtual CHistoryPosComponentI* GetComponent(uint uiComponentIdx) const = 0;
   
   // Расширенный интерфейс
   virtual void Sort(ESortTPCMode stmMode = STM_BY_OPEN_TIME_A) = 0;
   
   
   // Функция ищет внутри истории компоненту с указанным тикетом. 
   // В случае, если ее нет - возвращается false.
   // Если компонента найдена - возвращается true, и uiComponentIdx устанавливается на индекс компоненты внутри позиции.
   virtual bool FindComponentByTicket(long lTicket,uint &uiComponentIdx) const = 0;
};

By selecting the required history - you can recalculate its components (positions for MT5 or orders for MT4), and get an interface to any component as an abstract interface:

class CTradePosComponentI: public CMyObject
{
public:
   void CTradePosComponentI() {    SetMyObjectType(MOT_TRADEPOS_COMPONENT_I); };
   virtual void ~CTradePosComponentI() {};
   
   // Основной интерфейс
   virtual long               GetTPCTicket()       const = 0;
   virtual long               GetTPCMagic()        const = 0;
   virtual ECurrencySymbol    GetTPCSymbol()       const = 0;
   virtual ENUM_POSITION_TYPE GetTPCType()         const = 0;
   virtual datetime           GetTPCOpenTime()     const = 0;
   virtual double             GetTPCVolume()       const = 0;
   virtual double             GetTPCOpenPrice()    const = 0;
   virtual double             GetTPCStopLoss()     const = 0;
   virtual double             GetTPCTakeProfit()   const = 0;
   virtual string             GetTPCCommentary()   const = 0;
   
   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 double             GetTPDistance() const { if(GetTPCTakeProfit() == 0 || GetTPCTakeProfit() == EMPTY_VALUE) return(EMPTY_VALUE); if(GetTPCType() == POSITION_TYPE_BUY) return(GetTPCTakeProfit() - GetTPCOpenPrice()); return(GetTPCOpenPrice() - GetTPCTakeProfit());  };
   virtual double             GetSLDistance() const { if(GetTPCStopLoss() == 0 || GetTPCStopLoss() == EMPTY_VALUE) return(EMPTY_VALUE); if(GetTPCType() == POSITION_TYPE_BUY) return(GetTPCOpenPrice()- GetTPCStopLoss()); return(GetTPCStopLoss() - GetTPCOpenPrice());  };
};

class CHistoryPosComponentI: public CTradePosComponentI
{
public:
   void CHistoryPosComponentI() {    SetMyObjectType(MOT_HISTORYPOS_COMPONENT_I); };
   virtual void ~CHistoryPosComponentI() {};

   virtual datetime           GetTPCCloseTime()    const = 0;
   virtual double             GetTPCClosePrice()   const = 0;
   virtual double             GetTPCProfit()       const = 0;  // Возвращает профит по исторической позиции
   
   virtual bool               IsProfitClosePrice() const = 0;   // Возвращает true, если цена зарытия отличается от цены открытия в прибыльную сторону   
   
   // Возвращает профит исторической позиции для случая, когда бы ход цены (в сторону профита) был бы равен dPriceMove, а лот был бы единичным.
   // Функция используется для расчета лота для такой же позиции с нужным ходом цены 
   // Рекомендуется отнимать от цены двойной спред.
   virtual double             CalculateOneLotProfit(double dPriceMove) const = 0;  
  
};

For MT4 - there are corresponding history classes also inherited from these interfaces - thus, at the same time, the cross-platformity is provided - an EA does not need to find out where it works, all work with the history is done through the abstract interfaces.

 
Vitaly Muzichenko:

Don't write functions that are always constant and never change in this style

Write them concisely, no one ever looks at them anyway, and they take half as many lines

Since these functions never change, why do you put a bunch of unnecessary curly brackets there? Remove them and everything will shrink by itself. Because your example looks absurd: you yourself have blurred the code and then invent crutches to reduce it.
 
Alexey Navoykov:
Since these functions don't change, why did you put a bunch of unnecessary curly brackets there? Remove them, and everything will be compressed. Because your example looks absurd: you yourself have blurred the code and then invent crutches to reduce it.

I agree, you can cut out 3 more lines, and shorten the code, but the purpose was not to put the code to use, it is in fact not even mine, but to shorten, and such functions can be put five in one screen, not one. After that the programs are easier to read and you don't have to scroll 150 times. And the weight of the file decreases.

 
George Merts:

Nice work, I like it, but I don't like OOP and try to do without it. I don't like processors with thread division (for example, 4 cores and 8 threads). It must be clear that division and any virtualization is a loss of performance and loss of machine time for its implementation, be it thread division in the kernel or virtualization of functions in the code.

Vitaly Muzichenko:

I agree, you can cut out 3 more lines, and shorten the code, but the purpose was not to put the code to use, it's not even mine in fact, but to shorten it, and you can put five such functions in one screen, not one. After that the programs are easier to read and you don't have to scroll 150 times. And the weight of the file is reduced.

Brevity is the sister of talent, I think it sounds better.

Sincerely.
 
Vitaly Muzichenko:

27" working screen

I'm not going to reread it, I'll just quote:"Don't write functions that are always constant and never change in that style"

Why pick your eyes over a function that is written once when the platform is released and will never change in the future? Do you often change/edit the code in the functions to get lot size, number of orders and typical? Then why stretch it across 3 screens of a 32" monitor?

P.S. The code attached is forged from kodobase.


Counter question ))) I have such functions are in MyFunc.mqh file, I do not see the slightest sense in compressing it. Why, to save 10-20 KB on disk? And frankly speaking, such codestream makes me sick ))

 
Alexey Volchanskiy:

Counter question ))) I have such functions in file MyFunc.mqh, I don't see the slightest sense in compressing it. Why, to save 10-20 KB on disk? To be honest, this codestream makes me sick )).

I also use include files, it's convenient. Especially when you write a custom script, a lot of functions are the same and it's silly to write one and the same thing, just include the file and the function is in your EA.
As for me, the code should be clear, short, fast to work and should work in all conditions without errors.


Sincerely.

 
Alexey Volchanskiy:

Counter question ))) I have such functions in file MyFunc.mqh, I don't see the slightest sense in compressing it. Why, to save 10-20 KB on disk? And frankly speaking, such a codestream makes me sick ))

The programmer has written that the principle "I carry everything myself"; the entire code of the Expert Advisor is crammed into a single file. Accordingly, he copies all of these functions in each EA.
So, count: 1000 EAs x 10 Kb = 10 Mb - you already have to think about economizing ))
 
Alexey Volchanskiy:

Counter question ))) I have such functions in file MyFunc.mqh, I don't see the slightest sense in compressing it. Why, to save 10-20 KB on disk? And frankly speaking, such codestream makes me sick ))

Me too, but long ago I came to the conclusion that the code must be compact in places where one never looks at it, where it is never corrected and never will be corrected.

Scattering user code with all over the inludes is an additional headache, when you need to drag and drop a file into another terminal, or share it, you will need to drag and drop several files. Of course, you can transfer the includniks to all terminals, but if you change or add something in one terminal, then all of them must be replaced with a new one.

The Expert Advisors and indicators are so small that there is no point in distancing them from the body of the program. To be more correct, they are not small, they are single file, it's not like a site with 10 000 pages where you cannot do without classes and inludes. Moreover, there are structures now, and they are enough to write compact, 100% workable code.

 
George Merts:

By the way, it makes me very nervous when nesting is more than two levels. I try never to write it that way, spreading code over functions.

And even when there are two nesting levels - be sure to write comments after each closing parenthesis, which block it buries (say, duplicate loop header).

As for style, here's my code for selectinga history position for MT5 (by specified magician, symbol, with specified date range):

The history class itself is a descendant of the abstract CTradeHistoryI interface:

By selecting the required history - you can recalculate its components (positions for MT5 or orders for MT4), and get an interface to any component as an abstract interface:

For MT4 there are corresponding history classes which also inherit from these interfaces - thus at the same time the cross-platformity is provided - the Expert Advisor doesn't need to find out where it works, all work with the history is done through abstract interfaces.


Looks nice, can we also look at TRACE_*** and ASSERT?

 
Vitaly Muzichenko:

To drag and drop a file to another terminal, or to share it, you need to drag not just one file, but several. You can, of course, transfer the inludes to all terminals, but if you change or add something in one terminal, then you need to replace it with a new one in all terminals.

I recommend using symbolic links or junction links for the MQL-folder. All terminals will look in one folder.
1234567
New comment