a critical look at my code please

New comment
 

Hello, sorry for my bad english

To continue learning MQL4, I wrote this simple code. There is a detecting 2 Moving Average crossings.

This code works correctly , but I'm open to constructive criticism to learn and improve

Thank you for your comments


//+------------------------------------------------------------------+
//|                                              Dual Moving Average |
//+------------------------------------------------------------------+

extern int Magic_Number = 1;
extern double Lots      = 0.1;
extern int StopLoss     = 30;
extern int TakeProfit   = 50;
extern int TrailingStop = 15;
extern int Slippage     = 1;

double MM_Courte, MM_Courte_Previous;
extern int MM1 = 20;
extern int MM1_Methode = 1;                     //0 - SMA, 1 - EMA, 2 - SMMA, 3 - LWMA

double MM_Longue, MM_Longue_Previous;
extern int MM2 = 50;
extern int MM2_Methode = 1;                     //0 - SMA, 1 - EMA, 2 - SMMA, 3 - LWMA

int MyPoint;                                    // Variable contenant un multiple pour le calcul du Profit en fonction du nombre de Digits du Broker

double Achat, Prix_Achat, Prix_Achat_TakeProfit, Prix_Achat_StopLoss;  // Variables pour un trade Achat
double Vente, Prix_Vente, Prix_Vente_TakeProfit, Prix_Vente_StopLoss;  // Variables pour un trade Vente

bool Modification;                              // Variable pour injecter le TakeProfit et StopLoss

int Total_Orders, Compteur_cet_EA, Position_Index;

int BarsCount=0;                                // Variable contenant le nombre de Bars sur le graphique. Permet de limiter un unique Trade sur une barre

//+------------------------------------------------------------------+
//| Initialization                                                   |
//+------------------------------------------------------------------+
int init()
{
   if(Digits==3||Digits==5) MyPoint=10; else MyPoint=1;
        
   MM1 = MathMin(MM1, MM2);
   MM2 = MathMax(MM1, MM2);

   if(TakeProfit < 6) 
      {
           TakeProfit = 6;
           return(0);
           }

   if(TrailingStop < 15) 
      {
           TrailingStop = 15;
           return(0);
           }
           
    if(TrailingStop > TakeProfit) 
      {
           TakeProfit = TrailingStop + 1;
           return(0);
           }
           
   if(Bars < MM2)                                 // Vérifie que le graphique dispose au minimum d'un historique = à la MM longue
      {
      Print("Historique du graphique insuffisant");
      return(0);  
      }
        
        // Contrôle si les MMs sont identiques
   if (MM1 == MM2)
      {
      Print("Les moyennes mobiles courte et longue doivent être différentes");
      return(0);
      }
   
   return(0);
}

//+------------------------------------------------------------------+
//| Start function                                                   |
//+------------------------------------------------------------------+
int start()
{     
   //+------------------------------------------------------------------+
   //| Contrôle si des ordres sont en cours                             |
   //+------------------------------------------------------------------+
   Total_Orders = OrdersTotal();
   Compteur_cet_EA = 0;
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      if (
         OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES) &&
         ((OrderType() == OP_BUY && OrderMagicNumber()==Magic_Number) ||       // Buying position.
         (OrderType() == OP_SELL && OrderMagicNumber()==Magic_Number))         // Selling position.
         )   
         
         {
         Compteur_cet_EA = Compteur_cet_EA + 1;
         }     
      }

   //+------------------------------------------------------------------+
   //| Récupération des MM                                              |
   //+------------------------------------------------------------------+ 
   MM_Courte = iMA(NULL, 0, MM1, 0, MM1_Methode, PRICE_CLOSE, 0);
   MM_Courte_Previous = iMA(NULL, 0, MM1, 0, MM1_Methode, PRICE_CLOSE, 1);
   MM_Longue = iMA(NULL, 0, MM2, 0, MM2_Methode, PRICE_CLOSE, 0);
   MM_Longue_Previous = iMA(NULL, 0, MM2, 0, MM2_Methode, PRICE_CLOSE, 1);
     
   //+------------------------------------------------------------------+
   //| Position Longue                                                  |
   //+------------------------------------------------------------------+
   if (
      Compteur_cet_EA==0 && 
      MM_Courte > MM_Longue &&
      MM_Courte_Previous < MM_Longue_Previous &&
      Bars != BarsCount
      )
      {
      Achat = OrderSend(Symbol(), OP_BUY, Lots, Ask, Slippage, Ask - (StopLoss * Point * MyPoint), Ask + (TakeProfit * Point * MyPoint), "Achat croisement MMs", Magic_Number, 0, Green);
      if(Achat > 0)
         {
         if(OrderSelect(Achat,SELECT_BY_TICKET,MODE_TRADES)) 
         Print("Achat effectué suite croisement MMs : ",OrderOpenPrice());
         }
      else Print("Echec sur l'Achat croisement MMs : ",GetLastError());
      BarsCount = Bars; 
      return(0);
      }
      
   //+------------------------------------------------------------------+
   //| Position Courte                                                  |
   //+------------------------------------------------------------------+
   if (
      Compteur_cet_EA==0 && 
      MM_Courte < MM_Longue &&
      MM_Courte_Previous > MM_Longue_Previous &&
      Bars != BarsCount
      )
      {
      Vente = OrderSend(Symbol(), OP_SELL, Lots, Bid, Slippage, Bid + (StopLoss * Point * MyPoint), Bid - (TakeProfit * Point * MyPoint), "Vente croisement MMs", Magic_Number, 0, Red);
      if(Vente > 0)
         {
         if(OrderSelect(Vente,SELECT_BY_TICKET,MODE_TRADES)) 
         Print("Vente effectuée suite croisement MMs : ",OrderOpenPrice());
         }
      else Print("Echec sur la Vente croisement MMs : ",GetLastError()); 
      BarsCount = Bars;
      return(0);
      }
      
   //+------------------------------------------------------------------+
   //| Sortie Position Longue                                           |
   //+------------------------------------------------------------------+        
   Total_Orders = OrdersTotal();
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      bool Selection_Achat = OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES); 
         if (
            OrderType() == OP_BUY && 
            OrderMagicNumber()==Magic_Number &&
            MM_Courte < MM_Longue                                                                        // Si la tendance des MMs s'est inversé                
            )
            {
            bool Cloture_Achat = OrderClose(OrderTicket(), OrderLots(), Bid, 1, Yellow); 
            Print("Tendance des MMs inversées, MM_Courte < MM_Longue : ",OrderOpenPrice());
            return(0); 
            } 
             
         if (
            OrderType() == OP_BUY && 
            OrderMagicNumber()== Magic_Number &&
            TrailingStop > 0 &&
            Bid - OrderOpenPrice() > TrailingStop * Point * MyPoint &&
            OrderStopLoss() < Bid - (TrailingStop * Point * MyPoint)                    
            )
            {
            bool Modification_Achat = OrderModify (OrderTicket(), OrderOpenPrice(), Bid - (TrailingStop * Point * MyPoint), OrderTakeProfit(), 0, Yellow); 
            return(0); 
            }            
      }
    
   //+------------------------------------------------------------------+
   //| Sortie Position Courte                                           |
   //+------------------------------------------------------------------+       
   Total_Orders = OrdersTotal();
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      bool Selection_Vente = OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES); 
         if (
            OrderType() == OP_SELL && 
            OrderMagicNumber()== Magic_Number &&
            MM_Courte > MM_Longue                                                                        // Si la tendance des MMs s'est inversé                
            )
            {
            bool Cloture_Vente = OrderClose(OrderTicket(), OrderLots(), Ask, 1, Pink);
            Print("Tendance des MMs inversées, MM_Courte > MM_Longue : ",OrderOpenPrice()); 
            return(0); 
            } 
             
         if (
            OrderType() == OP_SELL && 
            OrderMagicNumber()== Magic_Number &&
            TrailingStop > 0 &&
            OrderOpenPrice() - Ask > TrailingStop * Point * MyPoint &&
            OrderStopLoss() > Ask + (TrailingStop * Point * MyPoint)                    
            )
            {
            bool Modification_Vente = OrderModify (OrderTicket(), OrderOpenPrice(), Ask + (TrailingStop * Point * MyPoint), OrderTakeProfit(), 0, Pink); 
            return(0); 
            }            
      } 

   return(0);
}


 

Bars isn't a reliable way to detect new bar. Use time only.

For your cross, if MM_Courte_Previous is strictly equal to MM_Longue_Previous you will not detect the cross.

 
arsouille: This code works correctly, but I'm open to constructive criticism to learn and improve
MM_Courte > MM_Longue &&
MM_Courte_Previous < MM_Longue_Previous &&
Bars != BarsCount
bool Selection_Vente = OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES);
   if (
      OrderType() == OP_SELL &&
  1. Volume is unreliable (can miss ticks)
    Bars is unreliable (max bars on chart)
    Always use time.
  2. if OrderSelect fails, so does your OT=OPx What are Function return values ? How do I use them ? - MQL4 forum
 

hello, thank you for you comments

I think I have integrated your feedback
I'm listening to all new constructive criticism

The entire code below if anyone wants to use it

Regards

//+------------------------------------------------------------------+
//|                                              Dual Moving Average |
//+------------------------------------------------------------------+

extern int Magic_Number = 1;
extern double Lots      = 0.1;
extern string Buy_Sell = "=== 0 pour Non, 1 pour Oui ===";
extern int Buy          = 1;
extern int Sell         = 1;
extern int StopLoss     = 30;
extern int TakeProfit   = 50;
extern int TrailingStop = 15;
extern int Slippage     = 1;

double MM_Courte, MM_Courte_Previous;
extern int MM1 = 20;
extern int MM1_Methode = 1;                     //0 - SMA, 1 - EMA, 2 - SMMA, 3 - LWMA

double MM_Longue, MM_Longue_Previous;
extern int MM2 = 50;
extern int MM2_Methode = 1;                     //0 - SMA, 1 - EMA, 2 - SMMA, 3 - LWMA

int MyPoint;                                    // Variable contenant un multiple pour le calcul du Profit en fonction du nombre de Digits du Broker

double Achat, Prix_Achat, Prix_Achat_TakeProfit, Prix_Achat_StopLoss;  // Variables pour un trade Achat
double Vente, Prix_Vente, Prix_Vente_TakeProfit, Prix_Vente_StopLoss;  // Variables pour un trade Vente

bool Modification;                              // Variable pour injecter le TakeProfit et StopLoss

int Total_Orders, Compteur_cet_EA, Position_Index;

int BarsCount=0;                                // Variable contenant le nombre de Bars sur le graphique afin de contrôler que le graphique contient assez d'historique
datetime Controle_Bars;                         // Permet de stocker l'heure d'achat ou de vente, et de limiter un ordre par barre

//+------------------------------------------------------------------+
//| Initialization                                                   |
//+------------------------------------------------------------------+
int init()
{
   if(Digits==3||Digits==5) MyPoint=10; else MyPoint=1;
        
   MM1 = MathMin(MM1, MM2);
   MM2 = MathMax(MM1, MM2);

   if(TakeProfit < 6) 
      {
           TakeProfit = 6;
           return(0);
           }

   if(TrailingStop < 15) 
      {
           TrailingStop = 15;
           return(0);
           }
           
    if(TrailingStop > TakeProfit) 
      {
           TakeProfit = TrailingStop + 1;
           return(0);
           }
           
   if(Bars < MM2)                                 // Vérifie que le graphique dispose au minimum d'un historique = à la MM longue
      {
      Print("Historique du graphique insuffisant");
      return(0);  
      }
        
        // Contrôle si les MMs sont identiques
   if (MM1 == MM2)
      {
      Print("Les moyennes mobiles courte et longue doivent être différentes");
      return(0);
      }
      
   Controle_Bars = iTime (NULL, 0, 0);
   
   return(0);
}

//+------------------------------------------------------------------+
//| Start function                                                   |
//+------------------------------------------------------------------+
int start()
{     
   //+------------------------------------------------------------------+
   //| Contrôle si des ordres sont en cours                             |
   //+------------------------------------------------------------------+
   Total_Orders = OrdersTotal();
   Compteur_cet_EA = 0; 
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      if (
         (OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES) == true) &&
         ((OrderType() == OP_BUY && OrderMagicNumber()==Magic_Number) ||       // Buying position.
         (OrderType() == OP_SELL && OrderMagicNumber()==Magic_Number))         // Selling position.
         )   
         
         {
         Compteur_cet_EA = Compteur_cet_EA + 1;
         }     
      }

   //+------------------------------------------------------------------+
   //| Récupération des MM                                              |
   //+------------------------------------------------------------------+ 
   MM_Courte = iMA(NULL, 0, MM1, 0, MM1_Methode, PRICE_CLOSE, 0);
   MM_Courte_Previous = iMA(NULL, 0, MM1, 0, MM1_Methode, PRICE_CLOSE, 1);
   MM_Longue = iMA(NULL, 0, MM2, 0, MM2_Methode, PRICE_CLOSE, 0);
   MM_Longue_Previous = iMA(NULL, 0, MM2, 0, MM2_Methode, PRICE_CLOSE, 1);
     
   //+------------------------------------------------------------------+
   //| Position Longue                                                  |
   //+------------------------------------------------------------------+
   if (
      Buy == 1 &&
      Compteur_cet_EA == 0 && 
      MM_Courte > MM_Longue &&
      MM_Courte_Previous <= MM_Longue_Previous &&
      Controle_Bars < iTime (NULL, 0, 0)
      )
      {
      Achat = OrderSend(Symbol(), OP_BUY, Lots, Ask, Slippage, Ask - (StopLoss * Point * MyPoint), Ask + (TakeProfit * Point * MyPoint), "Achat croisement MMs", Magic_Number, 0, Green);
      if(Achat > 0)
         {
         if(
            (OrderSelect(Achat,SELECT_BY_TICKET,MODE_TRADES) == true)
            ) 
            Print("Achat effectué suite croisement MMs : ",OrderOpenPrice());
         }
      else Print("Echec sur l'Achat croisement MMs : ",GetLastError()); 
      Controle_Bars = iTime (NULL, 0, 0);
      return(0);
      }
      
   //+------------------------------------------------------------------+
   //| Position Courte                                                  |
   //+------------------------------------------------------------------+
   if (
      Sell == 1 &&
      Compteur_cet_EA==0 && 
      MM_Courte < MM_Longue &&
      MM_Courte_Previous >= MM_Longue_Previous &&
      Controle_Bars < iTime (NULL, 0, 0)
      )
      {
      Vente = OrderSend(Symbol(), OP_SELL, Lots, Bid, Slippage, Bid + (StopLoss * Point * MyPoint), Bid - (TakeProfit * Point * MyPoint), "Vente croisement MMs", Magic_Number, 0, Red);
      if(Vente > 0)
         {
         if(
            (OrderSelect(Vente,SELECT_BY_TICKET,MODE_TRADES) == true)
            ) 
            Print("Vente effectuée suite croisement MMs : ",OrderOpenPrice());
         }
      else Print("Echec sur la Vente croisement MMs : ",GetLastError()); 
      Controle_Bars = iTime (NULL, 0, 0);
      return(0);
      }
      
   //+------------------------------------------------------------------+
   //| Sortie Position Longue                                           |
   //+------------------------------------------------------------------+        
   Total_Orders = OrdersTotal();
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      if ((OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES) == true))
         {
         if (
            OrderType() == OP_BUY && 
            OrderMagicNumber()==Magic_Number &&
            MM_Courte < MM_Longue                                                                        // Si la tendance des MMs s'est inversé                
            )
            {
            bool Cloture_Achat = OrderClose(OrderTicket(), OrderLots(), Bid, 1, Yellow); 
            Print("Tendance des MMs inversées, MM_Courte < MM_Longue : ",OrderOpenPrice());
            return(0); 
            } 
             
         if (
            OrderType() == OP_BUY && 
            OrderMagicNumber()== Magic_Number &&
            TrailingStop > 0 &&
            Bid - OrderOpenPrice() > TrailingStop * Point * MyPoint &&
            OrderStopLoss() < Bid - (TrailingStop * Point * MyPoint)                    
            )
            {
            bool Modification_Achat = OrderModify (OrderTicket(), OrderOpenPrice(), Bid - (TrailingStop * Point * MyPoint), OrderTakeProfit(), 0, Yellow); 
            return(0); 
            }            
         }
      }
    
   //+------------------------------------------------------------------+
   //| Sortie Position Courte                                           |
   //+------------------------------------------------------------------+       
   Total_Orders = OrdersTotal();
   
   for(Position_Index = Total_Orders - 1; Position_Index >= 0 ; Position_Index --)
      {        
      if ((OrderSelect(Position_Index, SELECT_BY_POS, MODE_TRADES) == true))
         { 
         if (
            OrderType() == OP_SELL && 
            OrderMagicNumber()== Magic_Number &&
            MM_Courte > MM_Longue                                                                        // Si la tendance des MMs s'est inversé                
            )
            {
            bool Cloture_Vente = OrderClose(OrderTicket(), OrderLots(), Ask, 1, Pink);
            Print("Tendance des MMs inversées, MM_Courte > MM_Longue : ",OrderOpenPrice()); 
            return(0); 
            } 
             
         if (
            OrderType() == OP_SELL && 
            OrderMagicNumber()== Magic_Number &&
            TrailingStop > 0 &&
            OrderOpenPrice() - Ask > TrailingStop * Point * MyPoint &&
            OrderStopLoss() > Ask + (TrailingStop * Point * MyPoint)                    
            )
            {
            bool Modification_Vente = OrderModify (OrderTicket(), OrderOpenPrice(), Ask + (TrailingStop * Point * MyPoint), OrderTakeProfit(), 0, Pink); 
            return(0); 
            }            
         }
      } 

   return(0);
}


New comment