2

In my mql4 Expert Advisor, I've written this function to close all the buy trades at once. When I test it via the Strategy Tester, it closes some buy trades, but for a few other buy trades it returns the: OrderClose error 4051:invalid ticket for OrderClose function error.

void Close_All_Buy_Trdes(){
   for (int i=0;i<=OrdersTotal();i++){
      OrderSelect(i,SELECT_BY_POS);
      if(OrderType()==OP_BUY {
           OrderClose(OrderTicket(),OrderLots(),Bid,3);
      }
   }
}

What is the reason for this happening?

not2qubit
  • 14,531
  • 8
  • 95
  • 135

4 Answers4

1

Once you close one of the orders the position number will be different for the rest of them... so the loop is trying ro close orders that wont be available at that position anymore.

You could select by ticket number (these don't change) instead of position, but that means having the ticket numbers stored.

If you prefer use position, this is a way of doing this:

void close_all()
{
    int get_out=0,hstTotal=OrdersHistoryTotal();
    if (hstTotal<1) get_out=1;
    while (get_out==0)
    {
        if(!OrderSelect(0,SELECT_BY_POS,MODE_TRADES)) get_out=1;
        else
        {
            //you can add here any specific if condition 
            OrderClose(OrderTicket(),OrderLots(),Bid,3);
        }
        hstTotal=OrdersHistoryTotal();
        if (hstTotal<1) get_out=1;
    }
}

The code always closes the first order(position=0) as many times as necessary while the number of open orders is >=1 (exit condition of the while is: hstTotal<1)

J_P
  • 761
  • 8
  • 17
  • J_P do you mean `OrderSelect(i,SELECT_BY_POS)` should change to `OrderSelect(i,SELECT_BY_TICKET)` and it would solve the problem? –  Mar 03 '18 at 15:36
  • Yes, that what I mean, but... for using that solution you'd need to pass the actual ticket numbers retrieving them first. I'm travelling now, so no metatrader available for me, but I'll write the whole code on Thursday if it's not too late for you. – J_P Mar 04 '18 at 08:52
  • Ok J_P it is better if you can –  Mar 05 '18 at 05:46
  • I've typed my code in the answer, please check if it does what you want, I've just tested in my computer and it works fine. – J_P Mar 07 '18 at 19:58
  • This is a very useful answer. Who would have known they re-arrange the order positions after close! For a slight improvement, I'd rather replace `get_out=1;` with `break;` and `while ( 1 )`. – not2qubit Jun 05 '18 at 16:36
0

You just need to loop backward

for(int i = OrdersTotal() - 1; i >= 1; i--)
{
    OrderSelect(i,SELECT_BY_POS,MODE_TRADES);
    OrderDelete(OrderTicket());
}
Viktor
  • 685
  • 2
  • 7
  • 11
0

Just for clarity, whilst the comment is valid by Viktor, the code is very poor. It should be as a bare minimum:

for(int i=OrdersTotal()-1; i>=0; i--)
{
   if(OrderSelect(Ticket, SELECT_BY_TICKET))
   {
      bool attempt; double price=0;
      if(OrderType()==OP_BUY) price=Bid; if(OrderType()==OP_SELL) price=Ask;
      if(OrderType()<=1) attempt=OrderClose(OrderTicket(), OrderLots(), price, 50);
      else attempt=OrderDelete(OrderTicket());
      if(!attempt) Print("Order Closeure failed with error #",GetLastError());
   }
}
PaulB
  • 1,262
  • 1
  • 6
  • 17
0

use this code:

void Close_All_Buy_Trdes(){
   for(int i = OrdersTotal() - 1; i >= 0; i--){
      OrderSelect(i, SELECT_BY_POS);
      if(OrderType() == OP_BUY){
         OrderClose(OrderTicket(), OrderLots(), Bid, 3);
        }
     }
  }
Jalal Nouri
  • 26
  • 5