Order send based on bollinger bands works partially

New comment
 

I made an Expert Advisor that opens an order based on price compared to the Bollinger Bands, it sends the order correctly, but sometimes it ignores some bars and opens, or ignores all of them and waits for another trigger.

Here is the part of code

 void NewOrder(){

       int OrderSended;
      
       double BMiddle = iBands ( NULL , 0 , 20 , 2 , 0 , PRICE_CLOSE ,MODE_MAIN, 0 );
         
       if ( iLow ( NULL , 0 , 0 ) < iBands ( NULL , 0 , 20 , 2 , 0 , PRICE_CLOSE ,MODE_LOWER, 0 ) && AvailableB == true ){
      
         OrderSended = OrderSend ( NULL ,OP_BUY,CalcSize(Risk),Ask, 50 ,StopLoss( 0 ), 0 , NULL ,MagicN, 0 , clrGreen );
         
         if (OrderSended > 0 ){
          
            AvailableB = false ;
            ticketB = OrderSended;
            
         }
         else 
             Print ( "error" );
      }
      
       if ( iHigh ( NULL , 0 , 0 ) > iBands ( NULL , 0 , 20 , 2 , 0 , PRICE_CLOSE ,MODE_UPPER, 0 ) && AvailableS == true ){  
      
         OrderSended = OrderSend ( NULL ,OP_SELL,CalcolaLotti(Rischio),Bid, 50 ,StopLoss( 1 ), 0 , NULL ,MagicN, 0 , clrGreen );
         
         if (OrderSended > 0 ){

            AvailableS = false ;
            ticketS = OrderSended;
         
         }
         else 
             Print ( "error" );
      }

}
 
  1. Once your code sets AvailableB or AvailableS to false, it can never trade that direction again. Your problem is elsewhere.

  2. 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. As is "if available B". 

  3.        if ( iHigh ( NULL , 0 , 0 ) > iBands ( NULL , 0 , 20 , 2 , 0 , PRICE_CLOSE ,MODE_UPPER, 0 ) && AvailableS == true ){  
      
         OrderSended = OrderSend ( NULL ,OP_SELL,CalcolaLotti(Rischio),Bid, 50 ,StopLoss( 1 ), 0 , NULL ,MagicN, 0 , clrGreen );
    Don't use NULL.
    • You can use NULL in place of _Symbol only in those calls that the documentation specially says you can. iHigh does, iCustom does, MarketInfo does not. OrderSend does not.
    • Don't use NULL (except for pointers where you explicitly check for it.) Use _Symbol and _Period, that is minimalist as possible and more efficient.
    • Zero is the same as PERIOD_CURRENT which means _Period. Don't hard code numbers.
    • MT4: No need for a function call with iHigh(NULL,0,s) just use the predefined arrays, i.e. High[].

 
William Roeder:
  1. Once your code sets AvailableB or AvailableS to false, it can never trade that direction again. Your problem is elsewhere.

  2. 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. As is "if available B".

  3. Don't use NULL.
    • You can use NULL in place of _Symbol only in those calls that the documentation specially says you can. iHigh does, iCustom does, MarketInfo does not. OrderSend does not.
    • Don't use NULL (except for pointers where you explicitly check for it.) Use _Symbol and _Period, that is minimalist as possible and more efficient.
    • Zero is the same as PERIOD_CURRENT which means _Period. Don't hard code numbers.
    • MT4: No need for a function call with iHigh(NULL,0,s) just use the predefined arrays, i.e. High[].

Thanks for the advices, to set that variables back to false i have another part of code that i forgot to post sorry.

Here it is

void CloseOrder(){
   
   if(OrderSelect(ticketB,SELECT_BY_TICKET,MODE_TRADES)){
   
      bool ClosedB;
      
      if(Bid > iMA(NULL,PERIOD_CURRENT,20,1,MODE_SMA,PRICE_CLOSE,1) && !AvailableB){
      
         ClosedB = OrderClose(OrderTicket(),OrderLots(),Ask,2);
         AvailableB = true;
      
      }
   }
   
   if(OrderSelect(ticketS,SELECT_BY_TICKET,MODE_TRADES)){
        
      bool ClosedS;
             
      if(Ask < iMA(NULL,PERIOD_CURRENT,20,1,MODE_SMA,PRICE_CLOSE,1) && !AvailableS){
               
         ClosedS = OrderClose(OrderTicket(),OrderLots(),Bid,2);
         AvailableS = true;
      
      }
   }
}
 
  1. Don't double post! You already had this thread open.
              General rules and best pratices of the Forum. - General - MQL5 programming forum 

  2.    if(OrderSelect(ticketB,SELECT_BY_TICKET,MODE_TRADES)){
⋮
         ClosedS = OrderClose(OrderTicket(),OrderLots(),Ask,2);
    You buy at the Ask and sell at the Bid. So to close a buy you use the Bid. You can use OrderClosePrice() instead of Bid/Ask.
  3. When selecting by ticket, the mode is ignored. You must check if the order has already closed.
  4. Check your return codes for errors, and report them including GLE/LE. Don't look at it unless you have an error. Don't just silence the compiler, it is trying to help you.
              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
    Only those functions that return a value (e.g. iClose, MarketInfo, etc.) must you call ResetLastError before in order to check after.

New comment