Order Close Query on Multiple Charts

 

Hi guys, I recently finished my first EA (thread can be found here) with a lot of help from you, thanks! I have done a bit of backtesting, so far so good and I have moved onto demo testing. Here is where the issue lies. The first position opened by the EA was running well, doing everything it should be. However, when the EA opened a trade on another pair, this second trade was closed within a few minutes for no apparent reason. My best guess is that the EA is not distinguishing closing rules from one position to the next but I could be completely wrong. I have attached a couple of snips from the Journal and Expert tab. For reference, GBPJPY was the original trade and EURUSD was the second trade. It looks like the EA on the GBPJPY trade is calling for the EURUSD to be closed. Code for the EA is below. Any help is greatly appreciated. (The exit signal for long trades is the FastMA being below the SlowMA).


As I said before, this is my first EA so any general tips or mistakes you spot would be a great help!

#define MAGICNUMBER 111222

//--- input parameters
input int      FastMA=100;
input int      SlowMA=200;
input int      TakeProfit=3000;
input int      StopLoss=1500;
input int      TrailingStop=1500;
//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
  {
//---

//---
   return(INIT_SUCCEEDED);
  }
//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
  {
//---

  }
//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+

void OnTick()
  {
   int i=0;
   double CurrentSlowMA    = iMA (NULL, 0, SlowMA, 0, MODE_SMA, PRICE_CLOSE, 1);
   double CurrentFastMA    = iMA (NULL, 0, FastMA, 0, MODE_SMA, PRICE_CLOSE, 1);
   double Aa               = StopLoss*.1;
   double Bb               = AccountBalance()*.01;
   double LotSize          = Bb/Aa;
     {
      if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES)==false)
        {
         Print("OrderSelect() returned error - ",GetLastError());
        }
      if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
         if(OrderType()==OP_BUY)
           {
            if(((Bid-OrderOpenPrice())>(Point*TrailingStop)) && (OrderStopLoss()<(Bid-Point*TrailingStop)))
               if(OrderModify(OrderTicket(),OrderOpenPrice(),Bid-Point*TrailingStop,OrderTakeProfit(),NULL,NULL)==false)
                 {
                  Print("OrderModify() returned error - ",GetLastError());
                 }
            if(CurrentFastMA<CurrentSlowMA)
               if(OrderClose(OrderTicket(),LotSize, Bid, 50, NULL)==false)
                  {
                  Print("OrderClose() returned error - ",GetLastError());
                  }
                    
           }
      if(OrderType()==OP_SELL)
        {
         if(((OrderOpenPrice()-Ask)>(Point*TrailingStop)) && (OrderStopLoss()>(Ask+Point*TrailingStop)))
            if(OrderModify(OrderTicket(),OrderOpenPrice(),Ask+Point*TrailingStop,OrderTakeProfit(),NULL,NULL)==false)
              {
               Print("OrderModify() returned error - ",GetLastError());
              }
         if(CurrentFastMA>CurrentSlowMA)
            if(OrderClose(OrderTicket(),LotSize,Ask,50,NULL)==false)
               {
               Print("OrderClose() returned error - ",GetLastError());
               }      
        }

     }

     {
      int total,ord=0,a;

      int mn=MAGICNUMBER;

      string symbol;

      total=OrdersTotal();

      for(a=0;a<total;a++)

        {

         if(OrderSelect(i,SELECT_BY_POS)==false)
           {
            Print("OrderSelect() returned error - ",GetLastError());
           }

         if(OrderSymbol()==Symbol() && OrderMagicNumber()==mn)ord++;

        }

      if(ord>0)
         return;


      double PreviousKline    = iStochastic (NULL, 0, 14, 3, 3, MODE_SMA, 0, MODE_MAIN, 2);
      double CurrentKline     = iStochastic (NULL, 0, 14, 3, 3, MODE_SMA, 0, MODE_MAIN, 1);
      double PreviousDline    = iStochastic (NULL, 0, 14, 3, 3, MODE_SMA, 0, MODE_SIGNAL, 2);
      double CurrentDline     = iStochastic (NULL, 0, 14, 3, 3, MODE_SMA, 0, MODE_SIGNAL, 1);
      static bool OpenShort   = false;
      static bool OpenLong    = false;


      if(CurrentFastMA>CurrentSlowMA && !OpenLong)
         if(CurrentDline<CurrentKline && CurrentKline>25)
            if(PreviousDline<25 && CurrentDline>25)
               if(OrderSend(Symbol(),OP_BUY,LotSize,Ask,50,Ask-StopLoss*_Point,Ask+TakeProfit*_Point,NULL,111222)>0)
                 {
                  OpenShort= false;
                  OpenLong = true;
                 }
      else Print(__FUNCTION__+" OrderSendError: ",GetLastError());

      if(CurrentFastMA<CurrentSlowMA && !OpenShort)
         if(CurrentDline>CurrentKline && CurrentKline<75)
            if(PreviousDline>75 && CurrentDline<75)
               if(OrderSend(Symbol(),OP_SELL,LotSize,Bid,50,Bid+StopLoss*_Point,Bid-TakeProfit*_Point,NULL,111222)>0)
                 {
                  OpenShort= true;
                  OpenLong = false;
                 }
      else Print(__FUNCTION__+" OrderSendError: ",GetLastError());

      //---
     }
  }

Thanks!

Beginner Problems!
Beginner Problems!
  • 2017.08.09
  • www.mql5.com
Firstly let me start by saying I have no coding background (bar a small bit of HTML and CSS, not much use here...
 
  1. if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES)==false)
            {
             Print("OrderSelect() returned error - ",GetLastError());
            }
          if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
    You check if the select for order position zero failed and log it. Then you continue processing what ever.
  2. That is the only position you ever process. What happens when you run on multiple charts (the position isn't zero?)
  3. In the presence of multiple orders (one EA multiple charts, multiple EAs, manual trading,) while you are waiting for the current operation (closing, deleting, modifying) to complete, any number of other operations on other orders could have concurrently happened and changed the position indexing:
    1. For non-FIFO (US brokers,) (or the EA only opens one order per symbol,) you can simply count down in a position loop, and you won't miss orders. Get in the habit of always counting down.
                Loops and Closing or Deleting Orders - MQL4 and MetaTrader 4 - MQL4 programming forum
    2. For FIFO (US brokers,) and you (potentially) process multiple orders per symbol, you must count up and on a successful operation, reprocess all positions (set index to -1 before continuing.)
    3. and check OrderSelect in case earlier positions were deleted.
                What are Function return values ? How do I use them ? - MQL4 and MetaTrader 4 - MQL4 programming forum
                Common Errors in MQL4 Programs and How to Avoid Them - MQL4 Articles
    4. and if you (potentially) process multiple orders, must call RefreshRates() after server calls if you want to use the Predefined Variables (Bid/Ask) or OrderClosePrice() instead, on the next order/server call.

  4. Make your first modify/close a for loop
  5. if(OrderClose(OrderTicket(),LotSize,Ask,50,NULL)==false)
    NULL is not a valid color. Don't hard code the size, use OrderLots. Don't need Ask/Bid see 3.4.
 
whroeder1:
  1. You check if the select for order position zero failed and log it. Then you continue processing what ever.
  2. That is the only position you ever process. What happens when you run on multiple charts (the position isn't zero?)
  3. In the presence of multiple orders (one EA multiple charts, multiple EAs, manual trading,) while you are waiting for the current operation (closing, deleting, modifying) to complete, any number of other operations on other orders could have concurrently happened and changed the position indexing:
    1. For non-FIFO (US brokers,) (or the EA only opens one order per symbol,) you can simply count down in a position loop, and you won't miss orders. Get in the habit of always counting down.
                Loops and Closing or Deleting Orders - MQL4 and MetaTrader 4 - MQL4 programming forum
    2. For FIFO (US brokers,) and you (potentially) process multiple orders per symbol, you must count up and on a successful operation, reprocess all positions (set index to -1 before continuing.)
    3. and check OrderSelect in case earlier positions were deleted.
                What are Function return values ? How do I use them ? - MQL4 and MetaTrader 4 - MQL4 programming forum
                Common Errors in MQL4 Programs and How to Avoid Them - MQL4 Articles
    4. and if you (potentially) process multiple orders, must call RefreshRates() after server calls if you want to use the Predefined Variables (Bid/Ask) or OrderClosePrice() instead, on the next order/server call.

  4. Make your first modify/close a for loop
  5. NULL is not a valid color. Don't hard code the size, use OrderLots. Don't need Ask/Bid see 3.4.

Thanks for the input Whroeder1, as you can see I still struggle a bit with the basics. Cycling through orders isn't something I really understood, however, the thread you linked to in 3.1 was very informative. Below is the solution I went for due to the fact that this EA will only ever open one order per symbol. I wasn't sure if the second check for MagicNumber and Symbol was required in the for loop, but I put it in just in case. It's probably a bit late to test this evening as it can be quite a slow EA to pick multiple trades so I will give it a run next week and see how it goes. Thanks for the informative and friendly reply.

 int IndexPosition;
   int TotalOrders;
   TotalOrders=OrdersTotal();
   double CurrentSlowMA    = iMA (NULL, 0, SlowMA, 0, MODE_SMA, PRICE_CLOSE, 1);
   double CurrentFastMA    = iMA (NULL, 0, FastMA, 0, MODE_SMA, PRICE_CLOSE, 1);
   double Aa               = StopLoss*.1;
   double Bb               = AccountBalance()*.01;
   double LotSize          = Bb/Aa;

   for(IndexPosition=TotalOrders -1; IndexPosition>=0; IndexPosition--)
     {
      if(!OrderSelect(IndexPosition,SELECT_BY_POS,MODE_TRADES)) continue;
      if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
         if(OrderType()==OP_BUY)
           {
            if(((Bid-OrderOpenPrice())>(Point*TrailingStop)) && (OrderStopLoss()<(Bid-Point*TrailingStop)))
               if(OrderModify(OrderTicket(),OrderOpenPrice(),Bid-Point*TrailingStop,OrderTakeProfit(),NULL,NULL)==false)
                 {
                  Print("OrderModify() returned error - ",GetLastError());
                 }
            if(CurrentFastMA<CurrentSlowMA)
               if(OrderClose(OrderTicket(),OrderLots(),OrderClosePrice(),50,CLR_NONE)==false)
                 {
                  Print("OrderClose() returned error - ",GetLastError());
                 }

           }
      if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
         if(OrderType()==OP_SELL)
           {
            if(((OrderOpenPrice()-Ask)>(Point*TrailingStop)) && (OrderStopLoss()>(Ask+Point*TrailingStop)))
               if(OrderModify(OrderTicket(),OrderOpenPrice(),Ask+Point*TrailingStop,OrderTakeProfit(),NULL,NULL)==false)
                 {
                  Print("OrderModify() returned error - ",GetLastError());
                 }
            if(CurrentFastMA>CurrentSlowMA)
               if(OrderClose(OrderTicket(),OrderLots(),OrderClosePrice(),50,CLR_NONE)==false)
                 {
                  Print("OrderClose() returned error - ",GetLastError());
                 }
           }

     }
 
UYPTrade: I wasn't sure if the second check for MagicNumber and Symbol was required in the for loop,
It is because the way you wrote your code
  if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
         if(OrderType()==OP_BUY){ ... }
  if(OrderMagicNumber()==MAGICNUMBER && OrderSymbol()==Symbol())
         if(OrderType()==OP_SELL){ ... }
That is equivalent to
  if(OrderMagicNumber()==MAGICNUMBER 
  && OrderSymbol()==Symbol()
  && OrderType()==OP_BUY){ ... }

  if(OrderMagicNumber()==MAGICNUMBER 
  && OrderSymbol()==Symbol()
  && OrderType()==OP_SELL){ ... }

Without the second test you would be operating on all sell orders.

If you read the link closer you could simplify your code

 for(IndexPosition=TotalOrders -1; IndexPosition>=0; IndexPosition--) if(
    OrderSelect(IndexPosition,SELECT_BY_POS,MODE_TRADES)
 && OrderMagicNumber()==MAGICNUMBER 
 && OrderSymbol()==Symbol()){
         if(OrderType()==OP_BUY){ ... }
         else{  // assuming no pending orders //if(OrderType()==OP_SELL)
 

Thanks for clearing that up. I included it as I had a feeling that was the case due to the way the EA acted with the original code. I need to get used to simplifying code, I'm sure it will be a lot easier to work with, in the long run, Thanks again.

Reason: