# a critical look at my code please

78
2014.07.17 14:27

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

```//+------------------------------------------------------------------+
//|                                              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 (
(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)
{
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)
{
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 (
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 (
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);
}

```

Moderator
29476
2014.07.17 15:07

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.

13941
2014.07.17 15:25
 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 && Volume is unreliable (can miss ticks)Bars is unreliable (max bars on chart)Always use time.if OrderSelect fails, so does your OT=OPx What are Function return values ? How do I use them ? - MQL4 forum
78
2014.07.19 12:58

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 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_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 &&
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(
)
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(
)
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 (
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 (
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);
}

```

/