Having trouble closing orders with OrderClose()

 
I do not know why I cannot OrdersClose()

I understand the parameters which seem clearly defined in the documentation
I am not getting any errors and believe the condition to close the orders are coded correctly.

I have moved the function call around, and used many print statment to attempt to diagnose the problem
I can print OrdersTotal() inside the function and shows 1 when an order is placed, but never closes it.

Please advise
Thanks

//+------------------------------------------------------------------+
//|                                                 functiontest.mq4 |
//|                                                 function testing |
//|                      Function Testing                            |
//+------------------------------------------------------------------+
#property copyright "Open Function Testing"


//---- input parameters
extern double LotSize=0.1;
extern int    TakeProfit=20;
extern int    StopLoss=10;
extern int    AB_distance=10; //not used
extern int    MagicNumber=123486;
extern double fibo_retrace=0.500;  //not used
extern double fibo_extension=0.618;  //not used
  
//for reference: http://www.forexfibonacci.com/calculate_fibonacci_levels/04/

double val1; //not used
double val2; //not used
double mlots = AB_distance*10; //not used - A to B distance normalizer

//++++ 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


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

//----
   return(0);
  }
  
  
  
//+------------------------------------------------------------------+
//| expert deinitialization function                                 |
//+------------------------------------------------------------------+
int deinit()
  {
//---- 
   
//----
   return(0);
  }
   
    
//+------------------------------------------------------------------+
//| expert start function                                            |
//+------------------------------------------------------------------+
int start()
  {

   static datetime PreviousTime=0;
   
   if(PreviousTime==Time[0])
      return(0);
   else PreviousTime=Time[0]; // PreviousTime==Time[0] returns until PremiousTime!=Time[0] else makes it ==[0] again and should run the start once for each change

   macdsignaltrade(); 
                     

//+------------------------------------------------------------------+
//| end start function                                               |
//+------------------------------------------------------------------+                
   return(0);
   }    


//+------------------------------------------------------------------+
//|     My Functions (testing)                                       |
//+------------------------------------------------------------------+

void macdsignaltrade()
{
   double faster=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_MAIN,1);//MODE_MAIN
   double slower=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_SIGNAL,1); //MODE_SIGNAL
   double prevfaster=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_MAIN,2);//MODE_MAIN previous faster
   double prevslower=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_SIGNAL,2); //MODE_SIGNAL previous slower
   //make one for shift 3 and 4 to evaluate the fractals
   
   if(prevfaster < prevslower && faster > slower)OrderEntry(0);
      //Print(" MACD UP");
   if(prevfaster > prevslower && faster < slower)OrderEntry(1);
      //Print(" MACD DOWN");
 
}

/*
void engulfingsignal() //Bullish Engulfing Candle
{
   if(Open[2] > Close[2]        //bear candle identified
      && Open[1] < Close[1]  //bull candle identified
      && Open[1]<= Close[2]  //bull engulfing condition
      && Open[2] < Close[1]) Print(Close[1]," Bullish Engulfing Candle "," and engulfingsize = ",DoubleToStr(Close[1]-Open[1],Digits));  
      //DoubleToStr( Close[1]-Open[1],Digits) forums help
     
   if(Open[2]<Close[2]//bull candle identified
      && Open[1]>Close[1]//bear candle identified
      && Open[1]>= Close[2]  //bear engulfing condition
      && Open[2] > Close[1]) Print(Close[1]," Bearish Engulfing Candle "," and engulfingsize = ",DoubleToStr(Close[1]-Open[1],Digits));  
      //DoubleToStr( Close[1]-Open[1],Digits) forums help);
}

void martingale()
{
//future function
// to double lot sizes each time a trade loses
}

int fractaltrade()
{
//future function
// needs to determine the ABDC's and trade entries
return(0);
}

*/

void macdclosetrade()
{
   
   double faster=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_MAIN,1);//MODE_MAIN
   double slower=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_SIGNAL,1); //MODE_SIGNAL
   double prevfaster=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_MAIN,2);//MODE_MAIN previous faster
   double prevslower=iMACD(NULL,0,12,26,9,PRICE_CLOSE,MODE_SIGNAL,2); //MODE_SIGNAL previous slower
   

   for(int i = OrdersTotal()-1; i >= 0 ; i--)
   bool result;
      { 
      if(OrderSelect(i, SELECT_BY_POS))  
         if(OrderMagicNumber() == MagicNumber)        
            if(OrderSymbol()== Symbol())
               if(OrderType() == OP_BUY)          
                  if(prevfaster > prevslower && faster < slower) //if direction goes down on a buy order then close the order
                     {
                      result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
                        if(result == false)
                           Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                           Print(" MACD DOWN CLOSE");
                     }
                     
                     
                  if(OrderType() == OP_SELL)           
                     if(prevfaster < prevslower && faster > slower) //if direction goes up on a sell order then close the order     
                        {
                         result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
                           if(result == false)
                              Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                              Print(" MACD UP CLOSE");
                        }   
                          
      }


} //end macdclosetrade() function



int OrderEntry(int direction)
{     
   int ticket;
   if(OrdersTotal() > 0) macdclosetrade();
   
   if(direction==0)
      if(OrdersTotal()==0)
         ticket = OrderSend(Symbol(),OP_BUY,LotSize,Ask,3*pips2points,Ask-(StopLoss*pips2dbl),Ask+(TakeProfit*pips2dbl),NULL,MagicNumber,0,Green);
         if(ticket<0) Alert("OrderSend Failed: ", GetLastError());
         
   if(direction==1)
      if(OrdersTotal()==0)
         ticket = OrderSend(Symbol(),OP_SELL,LotSize,Bid,3*pips2points,Bid+(StopLoss*pips2dbl),Bid-(TakeProfit*pips2dbl),NULL,MagicNumber,0,Red);
         if(ticket<0) Alert("OrderSend Failed: ", GetLastError());

         

return(0);
}      

//+------------------------------------------------------------------+
 
Agent86:
I do not know why I cannot OrdersClose()

I understand the parameters which seem clearly defined in the documentation
I am not getting any errors and believe the condition to close the orders are coded correctly

The issue is your for loop . . .  study the { }  braces . . .

 

This is what your for loop does . . .

   for(int i = OrdersTotal()-1; i >= 0 ; i--)
      {
      bool result;
      }

 

This is what you wanted it to do . . .

   bool result;

   for(int i = OrdersTotal()-1; i >= 0 ; i--)
      { 
      if(OrderSelect(i, SELECT_BY_POS))  
         if(OrderMagicNumber() == MagicNumber)        
            if(OrderSymbol()== Symbol())
                                           // <----- note this  . . .
               {
               if(OrderType() == OP_BUY)          
                  if(prevfaster > prevslower && faster < slower) //if direction goes down on a buy order then close the order
                     {
                      result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
                        if(result == false)
                           Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                           Print(" MACD DOWN CLOSE");
                     }
                     
                     
                  if(OrderType() == OP_SELL)           
                     if(prevfaster < prevslower && faster > slower) //if direction goes up on a sell order then close the order     
                        {
                         result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
                           if(result == false)
                              Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                              Print(" MACD UP CLOSE");
                        }   
                   }       
      }

 
Also I added an additional set of braces . . .  what you had will only work for OP_BUY .  . . study and understand.

 
Thanks, I think I understand
So no other code before the first code block brace ?
The bool result before the (for loop) brackets seemed to be the biggest problem


I am a little confused why the if(OrderType()==OP_BUY)== false would not then read if(OrderType() ==OP_SELL) and why the extra brackets are required ?

If here is some do's and don't on if() statement flow I will be sure to read it.

I'll keep searching around for something on that so I can understand better, thanks again
 

In order not to get mixed up by many ifs in a loop I put at the beginning of the loop everything I don't want:

   for(int i = OrdersTotal()-1; i >= 0 ; i--)
      {       
      if( !OrderSelect(i, SELECT_BY_POS))    continue;
      if( OrderMagicNumber() != MagicNumber) continue;     
      if( OrderSymbol() != Symbol())         continue;

      bool result;// has been on a wrong place!!
      if(OrderType() == OP_BUY) 
         {         
         if(prevfaster > prevslower && faster < slower) //if direction goes down on a buy order then close the order
            {
             result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
               if(result == false)
                  Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                  Print(" MACD DOWN CLOSE");
            }
         } // else ?
      if(OrderType() == OP_SELL)    
         {          
         if(prevfaster < prevslower && faster > slower) //if direction goes up on a sell order then close the order     
            {
             result = OrderClose(OrderTicket(), OrderLots(), Bid, 3*pips2points, Blue);
               if(result == false)
                  Print("Order",OrderTicket(),"failed to close Error",GetLastError());   
                  Print(" MACD UP CLOSE");
            }   
         }
   }
 
Agent86:
Thanks, I think I understand
So no other code before the first code block brace ?
The bool result before the (for loop) brackets seemed to be the biggest problem


I am a little confused why the if(OrderType()==OP_BUY)== false would not then read if(OrderType() ==OP_SELL) and why the extra brackets are required ?


If you don't use { }  braces only the line following the if is executed due to the if . . .  so in your code . . .

if(OrderSymbol()== Symbol())

 if this is true this line is executed . . . . .

if(OrderType() == OP_BUY)  

 

if it's true or false  this line is also executed . . .

if(OrderType() == OP_SELL) 

 

which is probably not what you wanted . . . 

 
RaptorUK:

If you don't use { }  braces only the line following the if is executed due to the if . . .  so in your code . . .

 if this is true this line is executed . . . . .

 

if it's true or false  this line is also executed . . .

 

which is probably not what you wanted . . . 

Got it thanks.
Reason: