magic number into signals

New comment
 

Hello,

I'm having a stupid hard time setting one position by signal.

Signals are both buy and sell.

They're based on 2 strategies : flat & trending (which I got inspired by one of the incredible articles on the site).

What I did is setting magic numbers to track which signal created the trade  :

int flat_buy_magic = 1111;
int flat_sell_magic = 2222;
int trend_buy_magic = 3333;
int trend_sell_magic = 4444;

 then depending on condition, set the magic number and open a position, for eg : 

      if (buy_signal_flat() && !no_flat_buy())
         { 
         trade.SetExpertMagicNumber(flat_buy_magic);
         trade.Buy(lotsize, NULL, Ask, Ask - 1000*_Point, Ask + 1000*_Point, "flat buy");
         }

I'm using a bool to return if there's an already opened position by the sub-strategy (flat or trend). In this instance : 

bool no_flat_buy()
{
      int i = 0;
      for ( i = 0; i < PositionsTotal(); i++)
      {
           ulong ticket = PositionGetTicket(i);
           if (PositionSelectByTicket(ticket))
           {
           if (flat_buy_magic == PositionGetInteger(POSITION_MAGIC))
           return true;
           }   
      }
       return false; 
}

EA is working as expected. But I'm trying to improve my coding skills.

What I would like to do is :

for the 4 scenariis :

flat_buy // flat_sell

trend_buy // trend_sell

rather than having 4 bool functions to return if there's already an opened position with the set magic number



have a single looping kind-of function that returns the magic number to check if trade is possible....


basically what I tried is :

int is_already_opened()
{
      int i = 0;
      for ( i = 0; i < PositionsTotal(); i++)
      {    
           int position_magic = 0;      
           ulong ticket = PositionGetTicket(i);
           if (PositionSelectByTicket(ticket))
           {
           position_magic = PositionGetInteger(POSITION_MAGIC))
           return (position_magic);
           }   
      }
       return 0; 
}

then the condition would be 

if (buy_signal_flat && is_already_opened() != flat_buy_magic)

but I feel like the int function I made doesn't return a correct value or something beyong what I'm capable to grasp.

So it opens more than one position per signal..

What am I missing?


In any case, hope this is clear enough and thanks for your time and help.

 

I'm just looking at your last function:

int is_already_opened()
{
   int i = 0;
   for ( i = 0; i < PositionsTotal(); i++)
   {    
      int position_magic = 0;      
      ulong ticket = PositionGetTicket(i);
      if (PositionSelectByTicket(ticket))
      {
         position_magic = PositionGetInteger(POSITION_MAGIC))
         return (position_magic);
      }   
   }
   return 0; 
}

Some remarks:

  1. The function name is_already_opened implies that the outcome is either true or false, so I'd expect it to return a bool which is not the case.
  2. In Mql5 a magic number is of type long, but you use int.
  3. Your for loop traverses orders front to back, this can cause it to skip an order if concurrently one of the orders has been removed from the list.
  4. You're selecting an order twice, first with PositionGetTicket() then again with PositionSelectByTicket(). The first select will be sufficient.
  5. Your function exits as soon as any magic number has been found, but what if it's not the magic you were looking for? You need to pass the wanted magic to your function so it can seek for it:
    6. if (buy_signal_flat && ! is_already_opened(flat_buy_magic))

I didn't look at the rest. Happy coding.

 
lippmaje:

I'm just looking at your last function:

Some remarks:

  1. The function name is_already_opened implies that the outcome is either true or false, so I'd expect it to return a bool which is not the case.
  2. In Mql5 a magic number is of type long, but you use int.
  3. Your for loop traverses orders front to back, this can cause it to skip an order if concurrently one of the orders has been removed from the list.
  4. You're selecting an order twice, first with PositionGetTicket() then again with PositionSelectByTicket(). The first select will be sufficient.
  5. Your function exits as soon as any magic number has been found, but what if it's not the magic you were looking for? You need to pass the wanted magic to your function so it can seek for it:

I didn't look at the rest. Happy coding.

Wow, thank you very much for the clear and detailed explanations.
I see that my attempt was very sloppy and that I don't really comprehend how functions work. 

Will dig deeper into it. 

Best regards, 

TS
New comment
Reason: