return value of 'OrderSelect' should be checked

 

In the past this code did not throw errors

I understand the return value and wondered what changed to cause errors where none existed prior ? 

Can anyone see fault with my highlighted solution code below. It's not throwing errors but is there a flaw in doing this ? 
Old code commented and highlighted in grey. My new code highlighted in orange. 

Or should I make the if condition above the entire comparisons first ? 

Please advise thanks

//+------------------------------------------------------------------+
//|                                                 1min5minmacd.mq4 |
//|                        Copyright 2020, MetaQuotes Software Corp. |
//|                                             https://www.mql5.com |
//+------------------------------------------------------------------+
#property strict
//---- input parameters
extern double    TakeProfit=20.0;
extern double    Lots=0.1;
extern double    StopLoss=15.0;
//++++ These are adjusted for 5 digit brokers.


int     pips2points;    // slippage  3 pips    3=points    30=points
double  pips2dbl;       // Stoploss 15 pips    0.0015      0.00150
//int     Digits.pips;    // DoubleToStr(dbl/pips2dbl, Digits.pips)

    // OrderSend(... Slippage.Pips * pips2points, Bid - StopLossPips * pips2dbl


//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
  {
//---
    if (Digits == 5 || Digits == 3)
   {    // Adjust for five (5) digit brokers.
      pips2dbl    = Point*10; pips2points = 10;   //Digits = 1;
   } 
   else 
    {    
      pips2dbl    = Point;    pips2points =  1;   //Digits = 0; 
    }
    // OrderSend(... Slippage.Pips * pips2points, Bid - StopLossPips * pips2dbl
//---- 

//----
   return(0);
        
//---
   return(INIT_SUCCEEDED);
  }
//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
  {
//---
   
  }
//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+
void OnTick()
  {
//--- 
   int ticket,i,total,result; 
     
   double   faster_1 = iMACD("EURUSD",PERIOD_M1,12,26,9,PRICE_CLOSE,MODE_MAIN,1), //MODE_MAIN
            slower_1 = iMACD("EURUSD",PERIOD_M1,12,26,9,PRICE_CLOSE,MODE_SIGNAL,1), //MODE_SIGNAL
            faster_2 = iMACD("EURUSD",PERIOD_M5,12,26,9,PRICE_CLOSE,MODE_MAIN,1), //MODE_MAIN
            slower_2 = iMACD("EURUSD",PERIOD_M5,12,26,9,PRICE_CLOSE,MODE_SIGNAL,1), //MODE_SIGNAL
            faster_3 = iMACD("EURUSD",PERIOD_H4,12,26,9,PRICE_CLOSE,MODE_MAIN,1), //MODE_MAIN
            slower_3 = iMACD("EURUSD",PERIOD_H4,12,26,9,PRICE_CLOSE,MODE_SIGNAL,1); //MODE_SIGNAL

   total  = OrdersTotal(); 
   
   if(total < 1)
      {
      if(faster_1 > slower_1)
         {
         ticket = OrderSend(Symbol(),OP_BUY,Lots,Ask,3*pips2points,Bid-StopLoss*pips2dbl,Bid+TakeProfit*pips2dbl,"My EA",12345,0,Green);
         if(ticket > 0)
            {
            if(OrderSelect(ticket,SELECT_BY_TICKET,MODE_TRADES))Print("BUY order opened : ",OrderOpenPrice());
            }
      else Print("Error opening BUY order : ",GetLastError());
      return;
      }
         
      if(faster_1 < slower_1)
      {
      ticket = OrderSend(Symbol(),OP_SELL,Lots,Bid,3*pips2points,Ask+StopLoss*pips2dbl,Ask-TakeProfit*pips2dbl,"My EA",12345,0,Red);  
      if(ticket > 0)
         {
         if(OrderSelect(ticket,SELECT_BY_TICKET,MODE_TRADES))Print("SELL order opened : ",OrderOpenPrice());
         }
      else Print("Error opening SELL order : ",GetLastError());
      return;        
      }
     } 
          

for (i = total - 1; i >= 0; i--)
      { 
      //OrderSelect(i, SELECT_BY_POS, MODE_TRADES); //***this line gives me error "return value of 'OrderSelect' should be checked
      if(OrderSelect(i, SELECT_BY_POS, MODE_TRADES)==true && OrderType() == OP_BUY && Symbol() == OrderSymbol())
      {
         if(faster_1 < slower_1)
            {
            result = OrderClose( OrderTicket(), OrderLots(), Bid,3, White);
            if(result == false)
               {
                Print("Order", OrderTicket()," failed to close Error ",GetLastError());
                return;
               }
            }
      }         
       
      if(OrderType() == OP_SELL && Symbol() == OrderSymbol())
      {   
         if(faster_1 > slower_1)
         {
            result = OrderClose(OrderTicket(), OrderLots(), Ask,3, White);
            if(result == false)
            {
               Print("Order", OrderTicket()," failed to close Error ", GetLastError());
               return;
            }   
         }    
      }
        
    }              
   return;
   }    
    
//+------------------------------------------------------------------+

Thanks

 
Agent86:

In the past this code did not throw errors

I understand the return value and wondered what changed to cause errors where none existed prior ? 

Can anyone see fault with my highlighted solution code below. It's not throwing errors but is there a flaw in doing this ? 
Old code commented and highlighted in grey. My new code highlighted in orange. 

Or should I make the if condition above the entire comparisons first ? 

Please advise thanks

Thanks

for(i = total - 1; i >= 0; i--)
     {
      if(!OrderSelect(i,SELECT_BY_POS,MODE_TRADES))
         break;
      if(OrderSymbol()==Symbol()&&OrderMagicNumber()==12334566)//Input your ea'smagic here
        {
         if(OrderType() == OP_BUY)
           {
            if(faster_1 < slower_1)
              {
               result = OrderClose(OrderTicket(), OrderLots(), Bid,3, White);
               if(result == false)
                 {
                  Print("Order", OrderTicket()," failed to close Error ",GetLastError());
                  return;
                 }
              }
           }
         if(OrderType() == OP_SELL)
           {
            if(faster_1 > slower_1)
              {
               result = OrderClose(OrderTicket(), OrderLots(), Ask,3, White);
               if(result == false)
                 {
                  Print("Order", OrderTicket()," failed to close Error ", GetLastError());
                  return;
                 }
              }
           }
        }
     }

Try something like this....not tested!

 

Hello, this is my first answer here :) i'll try my best to help. 

i'm not a pro so don't expect the best here :D, but anyways, what i've seen people do for this warning is the same as you did. 

you don't need to write it like this: 

if(OrderSelect(i, SELECT_BY_POS, MODE_TRADES)==true)

because the return value of OrderSelect() is either true of false, the better way to write it is: 

if(OrderSelect(i, SELECT_BY_POS, MODE_TRADES)) //this will be true if the order is selected. 

so, the part where you've coded: 

result = OrderClose(OrderTicket(), OrderLots(), Ask,3, White);
            if(result == false)
            {
               Print("Order", OrderTicket()," failed to close Error ", GetLastError());
               return;
            }   

can be done simpler with: 

if ( !(OrderClose(OrderTicket(), OrderLots(), Ask,3, White)) ) {
 		//print error
}

this way, if it fails, it prints error, if it succeeds, it closes/selects the order. so basically, the return value is checked


I hope this help. 

the pros feel free to correct me if i'm wrong. 

 
Mahdi24w:

Hello, this is my first answer here :) i'll try my best to help. 

i'm not a pro so don't expect the best here :D, but anyways, what i've seen people do for this warning is the same as you did. 

you don't need to write it like this: 

because the return value of OrderSelect() is either true of false, the better way to write it is: 

so, the part where you've coded: 

can be done simpler with: 

this way, if it fails, it prints error, if it succeeds, it closes/selects the order. so basically, the return value is checked. 


I hope this help. 

the pros feel free to correct me if i'm wrong. 

if(!OrderClose(OrderTicket(), OrderLots(), Bid,3, White))
     {
      Print("error");
      return;
     }

correct one...

if(!OrderClose(OrderTicket(), OrderLots(), OrderClosePrice(),3, White))
     {
      Print("error");
      return;
     }

and use of OrderClosePrice instead of Bid or Ask

if(!OrderSelect(i, SELECT_BY_POS, MODE_TRADES))break; //If not true, operation breaks...no need to use 'true' or 'false' 
no need to use 'true' or 'false' 
 
Kenneth Parling:

correct one...

and use of OrderClosePrice instead of Bid or Ask

no need to use 'true' or 'false' 

Thanks all

 
Mahdi24w:

Hello, this is my first answer here :) i'll try my best to help. 

i'm not a pro so don't expect the best here :D, but anyways, what i've seen people do for this warning is the same as you did. 

you don't need to write it like this: 

because the return value of OrderSelect() is either true of false, the better way to write it is: 

so, the part where you've coded: 

can be done simpler with: 

this way, if it fails, it prints error, if it succeeds, it closes/selects the order. so basically, the return value is checked. 


I hope this help. 

the pros feel free to correct me if i'm wrong. 

Sorry for the delay, and thanks.

I still get confusing that !OrderSelect() when false is the equivalent to if(OrderSelect() == true) 

 
Agent86: I still get confusing that !OrderSelect() when false is the equivalent to if(OrderSelect() == true) 

You should be able to read your code out loud and have it make sense. You would never write if( (2+2 == 4) == true) would you? if(2+2 == 4) is sufficient. So don't write if(bool == true), just use if(bool) or if(!bool). Code becomes self documenting when you use meaningful variable names, like bool isLongEnabled where as Long_Entry sounds like a trigger price or a ticket number and "if long entry" is an incomplete sentence.

Reason: