Help! EA not excecuting code correctly

 
Hello everyone, I am working on an price EMA crossover expert advisor on metatrader 5 which should have the following conditions : 1. if price closes above ema ,an alert is triggered as well a buy stop pending order 2. if price  closes below ema ,an alert is triggered as well a sell stop pending order. 3. offset parameters of the pending orders should be adjustable... I am able to compile the code but  problem is that it's not opening only one pending order after the condition has been met but multiple orders after each bar close . I would like it to be only one order per signal.I am a total beginer in coding.. Here is the code below and thank you in advance
/+------------------------------------------------------------------+

//|                                                     EA_Sample fix.mq5 |

//|                        Copyright 2021, MetaQuotes Software Corp. |

//|                                             https://www.mql5.com |

//+------------------------------------------------------------------+


#property copyright "MetaQuotes Software Corp."

#property link      "https://www.mql5.com"

#property version   "1.00"

#property strict


// EMA period

input int InpEMAPeriod=14; 

// Risk parameters

input double LotSize = 0.01;

input double Offset = 20; // Offset in points


// Global variable for the last bar

datetime lastBarTime;



datetime openTimeBuy = 0;

datetime openTimeSell =0;

//+------------------------------------------------------------------+

//| Expert initialization function                                   |

//+------------------------------------------------------------------+

int OnInit()

{

    lastBarTime = TimeCurrent();

    return(INIT_SUCCEEDED);

}


//+------------------------------------------------------------------+

//| Expert tick function                                             |

//+------------------------------------------------------------------+

void OnTick()

{

    MqlRates rates[];

    ArraySetAsSeries(rates, true);

    int copied = CopyRates(Symbol(), 0, 0, 1, rates);

    

    if(copied <= 0)

    {

        Print("Error in CopyRates: ", GetLastError());

        return;

    }

    

    if (lastBarTime != rates[0].time)

    {

        lastBarTime = rates[0].time;

        

        double price = rates[0].close;

        double offset = Offset * _Point;

        int ma_handle = iMA(Symbol(), 0, InpEMAPeriod, 0, MODE_EMA, PRICE_CLOSE);

        

        if(ma_handle == INVALID_HANDLE)

        {

            Print("Error in iMA: ", GetLastError());

            return;

        }

        

        double ema[];

        int copied_ema = CopyBuffer(ma_handle, 0, 0, 1, ema);

        

        if(copied_ema <= 0)

        {

            Print("Error in CopyBuffer: ", GetLastError());

            return;

        }

        

         MqlTradeRequest request={};

         MqlTradeResult  result={};

        

        ZeroMemory (request);

        request.action   =TRADE_ACTION_PENDING;                             // type of trade operation

        request.symbol = Symbol();

        request.volume = LotSize;

        request.deviation = 2;

        request.magic = 12345;

        double point=SymbolInfoDouble(_Symbol,SYMBOL_POINT);                // value of point

        int digits=SymbolInfoInteger(_Symbol,SYMBOL_DIGITS);                // number of decimal places (precision)                                                   

        

        double ask = SymbolInfoDouble(_Symbol, SYMBOL_ASK);

        double bid = SymbolInfoDouble(_Symbol, SYMBOL_BID);

        double spread = ask - bid;

        

        if (price > ema[0])

        {

            if (Offset * _Point < spread)

            {

                Print("Offset too small compared to spread, adjustment recommended");

            }

            Alert("Price crossed above EMA");

            request.type = ORDER_TYPE_BUY_STOP;

            price        =SymbolInfoDouble(Symbol(),SYMBOL_ASK)+Offset*point; // price for opening 

            request.price=NormalizeDouble(price,digits);                      // normalized opening price;

            if(!OrderSend(request, result))

            {

                PrintFormat("OrderSend error %d",GetLastError());

            }

        }

        else if (price < ema[0])

        {

            if (Offset * _Point < spread)

            {

                Print("Offset too small compared to spread, adjustment recommended");

            }

            Alert("Price crossed below EMA");

            request.type = ORDER_TYPE_SELL_STOP;

            price        =SymbolInfoDouble(Symbol(),SYMBOL_BID)-Offset*point; // price for opening 

            request.price=NormalizeDouble(price,digits);                      // normalized opening price

            if(!OrderSend(request, result))

            {

                PrintFormat("OrderSend error %d",GetLastError());

            }

        }

    }

}
 

Looks like your EA is placing multiple pending orders for each bar close instead of just one pending order per signal. 

 

If you want to have only one position opened at a time, you need to check if there are positions already opened for that symbol/EA. You could create a function like this:


int HasPosition(string symbol, ulong ea_magic)
{
    int total_positions = PositionsTotal(); // get the total opened positions

    for(int i = 0; i < total_positions; ++i) // loop through the list
    {
        ulong ticket = PositionGetTicket(i); // get the ticket of the position at index i
        ulong position_magic = PositionGetInteger(POSITION_MAGIC); // get the ea_magic of the position at index i
        
        if(position_magic == ea_magic) // if the position magic matches our ea magic
        {
            string position_symbol = PositionGetString(POSITION_SYMBOL); 
            
            if(position_symbol == symbol) // if the position symbol matches our symbol
            {
                ENUM_POSITION_TYPE pos_type = PositionGetInteger(POSITION_TYPE); // get the position direction
                
                if(pos_type == POSITION_TYPE_BUY)
                {
                    return 1; // if it is a long position, return 1
                }
                else if (pos_type == POSITION_TYPE_SELL)
                {
                    return 2; // if it is a short position, return 2
                }
                
            }
        
        }
        
    }
    
    return 0; // if there're no positions for the current symbol/ea magic, return 0
}


For example, if this function returns 0, there're no positions opened for the current symbol/ea, so you could create a logic to open a new one at this point.


Also, there're a few errors to be corrected and improvements to be done with your code. Note that you're calling int ma_handle = iMA(Symbol(), 0, InpEMAPeriod, 0, MODE_EMA, PRICE_CLOSE); every tick. This is fundamentally wrong - you only need one handle of an indicator in the EA. Also, you're not releasing this handle, causing a huge memory leak and performance issues. Store the handle in a global variable and call iMA inside the OnInit function, then just copy the values to the buffer.


ps: this code was not tested.

 
Relax Zone: . I would like it to be only one order per signal
        if (price > ema[0])

You are looking at a signal (opening one order per tick). Act on a change of signal.
          Too many orders - MQL4 programming forum #1 (2017)

 

Hi

Also, you can create ma handle on the init function, handles should be created once and here you repeat this on every new candle.

As for the multiple trades: you can check change of signal only (as William Roeder wrote: so compare current price and last price with current ema and last ema) or simply check if there are already opened trades in the market (only check signal if there are no other trades opened yet – but here you limit the number of trades to maximum one at the same time – and I’m not sure if this is what you want).

Best Regards

 
Emanuel Cavalcante Amorim Filho #:

If you want to have only one position opened at a time, you need to check if there are positions already opened for that symbol/EA. You could create a function like this:



For example, if this function returns 0, there're no positions opened for the current symbol/ea, so you could create a logic to open a new one at this point.


Also, there're a few errors to be corrected and improvements to be done with your code. Note that you're calling int ma_handle = iMA(Symbol(), 0, InpEMAPeriod, 0, MODE_EMA, PRICE_CLOSE); every tick. This is fundamentally wrong - you only need one handle of an indicator in the EA. Also, you're not releasing this handle, causing a huge memory leak and performance issues. Store the handle in a global variable and call iMA inside the OnInit function, then just copy the values to the buffer.


ps: this code was not tested.

Using PositionsTotal won't work for this specific problem since they are operating with pending orders. OrdersTotal() would solve the issue though

Just check if OrdersTotal() == 0, if so, a pending order may be executed.
 
Monwabisi Balani #:
Using PositionsTotal won't work for this specific problem since they are operating with pending orders. OrdersTotal() would solve the issue though

Just check if OrdersTotal() == 0, if so, a pending order may be executed.

oh, yeah, i misunderstood the problem, sry. i thought it was opening multiple positions at a time. The logic still serves, though, because only checking if there's a pending order may cause the EA to actually detect placed orders to other EAs/symbols.

Reason: