Not closing all open positions with magic number error

[Franzel Botha]

The following EA Closes all open positions on account with magic number related when reaching profit target.
Problem I am experiencing is that the EA at times do not close all the open positions when required.
I can't find the problem. Can anyone point out what the problem is and how I can solve it?

input double profitToClosePositions = 5;
input int magic_number = 1;
//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
  {
//---
   
//---
   return(INIT_SUCCEEDED);
  }
//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
  {
//---
   
  }
//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+
void OnTick()
  {
//---
   
if(Profit()>= profitToClosePositions)
{
CloseOrders();
}
  }
//+------------------------------------------------------------------+

double Profit(){
double Prof=0;
for (int i=0; i<OrdersTotal(); i++) {
if (OrderSelect(i, SELECT_BY_POS, MODE_TRADES)) {
if(OrderMagicNumber() == magic_number){

Prof = Prof + OrderProfit()+ OrderCommission() + OrderSwap();

}
} 
}
return(Prof);
}


void CloseOrders(void)
{for( int i = 0 ; i < OrdersTotal() ; i++ ) {
      //We select the order of index i selecting by position and from the pool of market/pending trades
      RefreshRates();
      int value =OrderSelect( i, SELECT_BY_POS, MODE_TRADES );
      //If the pair of the order (OrderSymbol() is equal to the pair where the EA is running (Symbol()) then return true
      
         if(OrderType()==OP_BUY && OrderMagicNumber()==magic_number){
            OrderClose(OrderTicket(),OrderLots(),MarketInfo(OrderSymbol(),MODE_BID),3,clrWhite);
            return;
         }
         if(OrderType()==OP_SELL && OrderMagicNumber()==magic_number){
            OrderClose(OrderTicket(),OrderLots(),MarketInfo(OrderSymbol(),MODE_ASK),3,clrWhite);
            return;
         }
      
      
   }
   //If the loop finishes it mean there were no open orders for that pair
return;
}

[Fabio Cavalloni]

Franzel Botha:
The following EA Closes all open positions on account with magic number related when reaching profit target.
Problem I am experiencing is that the EA at times do not close all the open positions when required.
I can't find the problem. Can anyone point out what the problem is and how I can solve it?

You always need to count down from OrdersTotal()-1 to 0 rather than the opposite in all orders cycles.

And you also need to remove "return" after orderclose, otherwise at the first order that get closed your function terminates.

[Franzel Botha]

Great, see now what's wrong.
Thnx for the help

[William Roeder]

Fabio Cavalloni: You always need to count down

Not necessarily.

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 (non-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 programming forum
   For In First Out (FIFO rules-US brokers,) and you (potentially) process multiple orders per symbol, you must find the earliest order, close it, and on a successful operation, reprocess all remaining positions.
             CloseOrders by FIFO Rules - Strategy Tester - MQL4 programming forum - Page 2 #16
             MetaTrader 5 platform beta build 2155: #1 № 11 ACCOUNT_FIFO_CLOSE

2. and check OrderSelect in case earlier positions were deleted.
             What are Function return values ? How do I use them ? - MQL4 programming forum
             Common Errors in MQL4 Programs and How to Avoid Them - MQL4 Articles
3. and if you (potentially) process multiple orders, must call RefreshRates() after server calls if you want to use, on the next order / server call, the Predefined Variables (Bid/Ask) or (be direction independent and use) OrderClosePrice().