Trailingstop EA won't work - page 4

I really appreciate the help that you and everyone else is providing.  I have tried everything over the last couple days to get that compiled and I apologize that I didn't ask.  Most of you have suggested that I defer from using my own code and use someone else's code, but I'm not going to learn if I do not understand what I am doing wrong.  To me, my code looks sound.  I am asking it to count the orders by position and select only those that meet my criteria and then modify that specific trade.  Thus far, after 4 pages nobody has pointed out which line of my code is broken.  Unless I first see and understand what I am doing wrong, it will not help me to learn by using someone else's code.  My goal now is not to have a Trailingstop EA that works, because there are plenty out there, but for me to learn how to code, so that I can move on.   In fact, my code was almost identical to the code that Jimdandy posted in his Youtube Tutorial on Trailingstops.  Since it is virtually identical to his, I copied the code almost verbatim (except that I want to trade manually), but his code only works sometimes as well.

I have printed off some stuff.  This trade is almost 300 points beyond the point where I specified that the Trailingstop kicks in(which was 150 points):

 2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrderModify = false

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: Ask = 1.45926

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: Selectbypos = 0

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: Selectbyticket = 1

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrderTicket = 50381828

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: ticket = 0

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrderSelect = true

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrdersTotal = 16

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrderSymbol = EURUSD

2016.04.22 00:18:02.301 Trailing_v33 EURAUD,M15: OrderTicket = 50381828

2016.04.22 00:17:58.934 Trailing_v33 EURAUD,M15: OrderModify = false

This EA is only placed on the EURAUD.  As you can see there are 16 Orders open on 9 other charts.  Ticket number Selected for this pair is completely wrong and OrderSymbol is complete wrong.  Both are for an open EURUSD order.  Based on the above, my for loop selected Position 0 in this case, whereas the EURAUD it was supposed to select was in position 14 (15-1).  Why?  I told it to ignore this.

 In the above mentioned video Jim actually get into this problem, and said he uses the following code to overcome my problem, however, this is a counter for properly sending orders and not a function to filter orders.  I tried to use it as a filter, but did not succeed.

int OpenOrdersThisPair(string pair){
   int total=0;
      for(int s= OrdersTotal()-1; s>= 0; s--){
         OrderSelect(s,SELECT_BY_POS,MODE_TRADES);
           if(OrderSymbol()==pair) total++;
       }
   return(total);  
}

 My code now looks like this.  Will someone please critique my code so that I can learn from this and tell me why it doesn't work on multiple charts. I know it cannot be that bad because it actually works perfectly sometimes.  Thank you

#property strict;
extern string Label_TrailingStart="Pip threshold to activate TrailingStop";
extern int TrailingStart=10;
extern string Label_TrailingStop="Pips trailing behind";
extern int TrailingStop=5;
double stoplevel=(MarketInfo(Symbol(),MODE_STOPLEVEL))/10,Pip=Point*10;
int TS=TrailingStart-TrailingStop;
bool UseTrailingStop=true,UseAutotrading=true;
//+------------------------------------------------------------------+
//|  Expert initialization function                                  |
//+------------------------------------------------------------------+
int init(){
   if(TS<stoplevel){
    MessageBox("Please note: Your inputs cannot be less than the minimum levels required"+
            "\nby your broker. Please reload the EA and either increase the value of the"+
            "\nTrailingStart and/or decrease the value of the TrailingStop so that "+
            "\nTrailingStart-TrailingStop >= "+StringConcatenate(stoplevel)+" pips");
     } 
   return(0);
  }
//+------------------------------------------------------------------+
//|    Expert deinitialization function                              |
//+------------------------------------------------------------------+
int deinit(){
   return(0);
  }
//+------------------------------------------------------------------+
//|   Expert start function                                          |
//+------------------------------------------------------------------+
int start(){
   if(UseTrailingStop) Trailing(); 
   if(UseAutotrading) Autotrading();  
   return(0);
}
//+------------------------------------------------------------------+
void Trailing(){
for(int b=OrdersTotal()-1; b>=0; b--){
      if(OrderSelect(b,SELECT_BY_POS,MODE_TRADES))
         if(OrderSymbol()==Symbol())
            if(OrderType()==OP_BUY)
                  if((Bid-OrderOpenPrice())>(TrailingStart*Pip))
                     if(OrderStopLoss()<Bid-(TrailingStop*Pip))
                        OrderModify(OrderTicket(),OrderOpenPrice(),Bid-(TrailingStop*Pip),OrderTakeProfit(),Blue);
   }     
   for(int s=OrdersTotal()-1; s>=0; s--){
     if(OrderSelect(s,SELECT_BY_POS,MODE_TRADES))
         if(OrderSymbol()==Symbol())
            if(OrderType()==OP_SELL)
                  if((OrderOpenPrice()-Ask)>(TrailingStart*Pip))
                     if(OrderStopLoss()>Ask+(TrailingStop*Pip) || OrderStopLoss()==0)
                        OrderModify(OrderTicket(),OrderOpenPrice(),Ask+(TrailingStop*Pip),OrderTakeProfit(),Red);
   }
}
void Autotrading(){
   if(!IsTradeAllowed()){
      MessageBox("This Expert Advisor requires Auto Trading. Please reload the EA or right click on"+
                 "\nthe chart to bring up the inputs window. Under the common tab check the box to"+
                 "\nallow live trading");
      Sleep(50000);
     }
   if(!IsExpertEnabled()){
      MessageBox("This Expert Advisor requires Auto Trading. Please click the button at the top");
      Sleep(50000);
     } 
}

 

Not true! We have described your errors and offered solutions for them in many flavours and alternatives. Yet you ignore them and continue to do it YOUR WAY. Not only that, you have now even made it worse by adding more mistakes.

When a painter learns to paint, he does so by first coping the techniques of others that have mastered it. Only once he is comfortable with these techniques does he start to create his OWN WAY of painting.

Here is a short list of "my critique":

1. Use your braces ("curly" brackets). Don't try to skimp and save on them. Until your code is fully functional and debugged, add them even if just for a single line is contained in the block. Place then on a separate line as well. Don't worry if you have plenty of white space. You can always go back and clean it up, but in the beginning, you want to make sure that everything is within their correct code block and braces and parenthesis are balanced. Stringing several "if" statements in a cadence is a recipe for disaster as it is very difficult to debug, especially with a finicky compiler as this one.
   
2. Your original code had only one loop which now you have made into two for no apparent reason except to make your code even slower and repeat the places that you have to fix when there are bugs.
3. Use variables to store results of expressions! You repeat the same expressions several times over, making your code not only slower, but more prone to errors and more difficult to read and fix because you have to change them in several places.

There are more things that I could list, but fix these at least, once and for all!

Coding is just like any other language. Yes, you can just string words together, but that does not make you a poet. In coding you also have to have rhythm and structure, in order to make it readable and functional.

Ok thank you very much for this.  I actually had brackets for all my 'if' statements initially as per the code I posted here earlier on, but since that code didn't work I tried various other things, which included copying the code that I pasted in my previous post.  As I mentioned this is actually not even my code, but it also doesn't work on all pairs.  I will however replace the brackets again and follow the other advice.  In the meantime I learned that OrderSelect will print the Symbol of the  first order in the terminal and not the one that the EA is on, so that was actually not wrong.

EDIT:  Thank you for suggestion #3!!!!!!!  I think this was the problem, because the Order must first be selected by Symbol and THEN the value must be stored.  So I moved the variable to below those functions as below, and it seems to work now (but more testing is required before I will know with certainty

int start(){
      for(int i=OrdersTotal()-1; i>=0; i--) {
         if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES))
            if(OrderSymbol()!= Symbol()) continue;
               double Pip=Point*10,TSTP=TrailingStart*Pip,Trail=TrailingStop*Pip,SL=StopLoss*Pip;               
                  if(OrderType()==OP_BUY){
                     if(Bid-OrderOpenPrice()>TSTP){
                        if(OrderStopLoss()<Bid-Trail){
                           if(!OrderModify(OrderTicket(),OrderOpenPrice(),Bid-Trail,OrderTakeProfit(),Orange))
                              Print("Error Buy TrailingStop: ",GetLastError());
            }
         }

Taking a look at your latest code, I  have this to say - I GIVE UP! (You refuse to follow through)!

FMIC: Taking a look at your latest code, I  have this to say - I GIVE UP! (You refuse to follow through)!	Now perhaps you understand my "hard criticism" when they waste everyone's time. While I understand "simple things" they think it is acceptable to waste everyone's time, I don't.

---

Trader3000:  I think this was the problem,...

int start(){       for(int i=OrdersTotal()-1; i>=0; i--) {          if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES))             if(OrderSymbol()!= Symbol()) continue;                double Pip=Point*10, TSTP=TrailingStart*Pip, Trail=TrailingStop*Pip, SL=StopLoss*Pip;                   if(OrderType()==OP_BUY){                      if(Bid-OrderOpenPrice()>TSTP){                         if(OrderStopLoss()<Bid-Trail){                            if(!OrderModify(OrderTicket(),OrderOpenPrice(),Bid-Trail, OrderTakeProfit(), Orange))                               Print("Error Buy TrailingStop: ",GetLastError());             }          }

Your problem is that you don't think.

int start(){    for(int i=OrdersTotal()-1; i>=0; i--) {       if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES)) if(OrderSymbol()!= Symbol()) continue; // Everything below is executed if NO order is selected or a selected order is symbol       double Pip=Point*10, TSTP=TrailingStart*Pip, Trail=TrailingStop*Pip,SL=StopLoss*Pip;       if(OrderType()==OP_BUY)       && Bid-OrderOpenPrice()>TSTP)       && OrderStopLoss()<Bid-Trail){          if(!OrderModify(OrderTicket(), OrderOpenPrice(), Bid-Trail, OrderTakeProfit(), Orange))             Print("Error Buy TrailingStop: ",GetLastError());       }    :

Yes! Unfortunately, it seems you are correct!

Thank you for the replies.So, I changed the code back to an earlier version as per below.  With it like this it seems to be working perfectly on all pairs where the stoplevel is 50 points, but not on pairs where the stoplevel is higher eg, EURAUD even if the external variables is higher than the stoplevel.  So this is bizarre

extern int TrailingStart=15;
extern int TrailingStop=5;
double stoplevel=(MarketInfo(Symbol(),MODE_STOPLEVEL))/10;
int TS=TrailingStart-TrailingStop;
//+------------------------------------------------------------------+
//|                                                                  |
//+------------------------------------------------------------------+
int init(){
   if(TS<stoplevel){
      MessageBox("Please note: Your inputs for TrailingStart and/or TrailingStop cannot"+
                 "\nbe less than the minimum levels required by your broker and the"+
                 "\nTrailingStart has been increased automatically to "+StringConcatenate(stoplevel+TrailingStop)+" pips");
     } 
   return(0);
  }
//+------------------------------------------------------------------+
//|                                                                  |
//+------------------------------------------------------------------+
int deinit(){
   return(0);
  }
//+------------------------------------------------------------------+
//|                                                                  |
//+------------------------------------------------------------------+
int start(){
   double Pip=Point*10;
   if(TS<stoplevel) TrailingStart=(int)stoplevel+TrailingStop;
           
   if(!IsTradeAllowed()){
      MessageBox("This Expert Advisor requires Auto Trading. Please reload the EA or right click on"+
                 "\nthe chart to bring up the inputs window. Under the common tab check the box to"+
                 "\nallow live trading");
      Sleep(50000);
     }
   if(!IsExpertEnabled()){
      MessageBox("This Expert Advisor requires Auto Trading. Please click the button at the top");
      Sleep(50000);
     } 
   int ticket=0,buy_ticket=0,sell_ticket=0;  
   for(int i=OrdersTotal()-1; i>=0; i--)
      if(OrderSelect(i,SELECT_BY_POS) && OrderSymbol()==Symbol()){
      ticket++;
      if(OrderType()==OP_BUY) buy_ticket=OrderTicket();
      if(OrderType()==OP_SELL) sell_ticket=OrderTicket();
      }
            if(OrderType()==OP_BUY){
               if(OrderSelect(buy_ticket,SELECT_BY_TICKET)){
                  if((Bid-OrderOpenPrice())>(TrailingStart*Pip)){
                     if(OrderStopLoss()<Bid-(TrailingStop*Pip)){
                        if(OrderModify(OrderTicket(),OrderOpenPrice(),Bid-(TrailingStop*Pip),OrderTakeProfit(),Blue))
                        Print("Buy = ",GetLastError());
                        return(0);
                        RefreshRates();
                        }     
                     }  
                  }     
               }
            if(OrderType()==OP_SELL){
               if(OrderSelect(sell_ticket,SELECT_BY_TICKET)){
                  if((OrderOpenPrice()-Ask)>(TrailingStart*Pip)){
                     if(OrderStopLoss()>Ask+(TrailingStop*Pip) || OrderStopLoss()==0){
                        if(OrderModify(OrderTicket(),OrderOpenPrice(),Ask+(TrailingStop*Pip),OrderTakeProfit(),Red))
                        Print("Sell = ",GetLastError());
                        return(0);
                        RefreshRates();
                        }
                     }
                  }  
               }
             
  return(0);
}
//+------------------------------------------------------------------+

 

  

I also found another EA that has very similar code as mine and compared the code.  I highlighted the main differences.  

1.  Increments instead of decrements

2.  Multiply everything with Point instead of Point*10

3.  He adds the Trailingstop and Trailingstep together and then subtracts one and modify the order with the Trailingstep.  If I understand this correctly, the Trailingstart will get activated after a move of 15 pips (150 points).  In my EA, the price will trail behind by 5 pips and get get stopped out if the price falls again and hit this 5 pip level.  In his EA, it will also get activated after 15 pips, but the amount of pips it trails behind is 19 (15+5-1), so the price have to fall by 19 pips to get stopped out.  As an example:  OrderOpenPrice is 1.50000.  Price goes up by 150 points to 1.50150 which activates the Trailingstop.  If the price then falls back down to 1.50100, it will be stopped out for a 5 pip profit.  In his EA, the Trailingstop is also activated at 1.50150, but the trail is at 1.49960 which I think will cause  error 130 since the stop is to close to the stoplevel.  Or am I misunderstanding this?

Could my problem be with one or more of these?

//+------------------------------------------------------------------+
//|                                                   e-Trailing.mq4 |
//|                                           Êèì Èãîðü Â. aka KimIV |
//|                                              http://www.kimiv.ru |
//|                                                                  |
//| 12.09.2005 Àâòîìàòè÷åñêèé Trailing Stop âñåõ îòêðûòûõ ïîçèöèé    |
//|            Âåøàòü òîëüêî íà îäèí ãðàôèê                          |
//+------------------------------------------------------------------+
#property copyright "Êèì Èãîðü Â. aka KimIV"
#property link      "http://www.kimiv.ru"

//------- Âíåøíèå ïàðàìåòðû ------------------------------------------
extern bool   ProfitTrailing = True;  // Òðàëèòü òîëüêî ïðîôèò
extern int    TrailingStop   = 15;     // Ôèêñèðîâàííûé ðàçìåð òðàëà
extern int    TrailingStep   = 5;     // Øàã òðàëà
extern bool   UseSound       = True;  // Èñïîëüçîâàòü çâóêîâîé ñèãíàë
extern string NameFileSound  = "expert.wav";  // Íàèìåíîâàíèå çâóêîâîãî ôàéëà

//+------------------------------------------------------------------+
//| expert start function                                            |
//+------------------------------------------------------------------+
void start() {
  for (int i=0; i<OrdersTotal(); i++) {
    if (OrderSelect(i, SELECT_BY_POS, MODE_TRADES)) {
      TrailingPositions();
    }
  }
}

//+------------------------------------------------------------------+
//| Ñîïðîâîæäåíèå ïîçèöèè ïðîñòûì òðàëîì                             |
//+------------------------------------------------------------------+
void TrailingPositions() {
  double pBid, pAsk, pp;

  pp = MarketInfo(OrderSymbol(), MODE_POINT);
  if (OrderType()==OP_BUY) {
    pBid = MarketInfo(OrderSymbol(), MODE_BID);
    if (!ProfitTrailing || (pBid-OrderOpenPrice())>TrailingStop*pp) {
      if (OrderStopLoss()<pBid-(TrailingStop+TrailingStep-1)*pp) {
        ModifyStopLoss(pBid-TrailingStop*pp);
        return;
      }
    }
  }
  if (OrderType()==OP_SELL) {
    pAsk = MarketInfo(OrderSymbol(), MODE_ASK);
    if (!ProfitTrailing || OrderOpenPrice()-pAsk>TrailingStop*pp) {
      if (OrderStopLoss()>pAsk+(TrailingStop+TrailingStep-1)*pp || OrderStopLoss()==0) {
        ModifyStopLoss(pAsk+TrailingStop*pp);
        return;
      }
    }
  }
}

//+------------------------------------------------------------------+
//| Ïåðåíîñ óðîâíÿ StopLoss                                          |
//| Ïàðàìåòðû:                                                       |
//|   ldStopLoss - óðîâåíü StopLoss                                  |
//+------------------------------------------------------------------+
void ModifyStopLoss(double ldStopLoss) {
  bool fm;

  fm=OrderModify(OrderTicket(),OrderOpenPrice(),ldStopLoss,OrderTakeProfit(),0,CLR_NONE);
  if (fm && UseSound) PlaySound(NameFileSound);
}
//+------------------------------------------------------------------+

Lets see if I get this logic of yours! You are willing to find another EA from someone else from who knows where and are willing to incorporate that code into yours, BUT YOU ARE NOT WILLING to take code that was specifically written to help you out and correct you issues!!!

WOW! How LOGICAL of you! Where is SPOCK when you need him!

SPOCK, may you Rest in Peace! We all miss you!

I finally figured it out so thank you to Everyone for the help (and criticism).  After going back to the code that you and Mike posted on the first page, I saw my mistake.  I probably overlooked this because it was so subtle, but mostly because I did not understand the logic of the code properly.  The problem was that I thought that the pip amount for the TrailingStop was the amount of pips it was trailing behind.  But the number of pips that it trails behind is actually TrailingStart-TrailingStop.  So my settings were below the stoplevel for pairs such as the gbpjpy because I input 15 and 5.  Since 5 is below the stoplevel of 8. By simply increasing the TrailingStop it now works on all pairs.  Thank you!!!