Close order X candles after MAs crossover

Davide.P 2020.06.03 15:43

Hi,

I am running an exercise to close a position 20 candles after a MA Crossover.

The code is made of 3 blocks, the 1st to open the trade, the 2nd to count the bars since the last MA crossover, and the 3rd to close the position. This code doesn't work and the position is closed immediately. Initially, I thought the reason was that the 3rd block is not nested in the 2nd, but even in that case, it doesn't work.

I have attached the code below. If you could point me in the right direction, it would be much appreciated:)

void OnTick(void)
  {
int count,cnt,ticket,total,nBars;

   //1.OPEN BLOCK
   total=OrdersTotal();
   if(total<1)
      {
      double FstMa0=iMA(NULL,0,12,0,MODE_SMA,0,0);
      double SlwMa0=iMA(NULL,0,26,0,MODE_SMA,0,0);
      double FstMa1=iMA(NULL,0,12,0,MODE_SMA,0,1);
      double SlwMa1=iMA(NULL,0,26,0,MODE_SMA,0,1);
      
      if(FstMa0>SlwMa0 && FstMa1<SlwMa1)
         {
         ticket=OrderSend(Symbol(),OP_BUY,1,Ask,0,0,0,NULL,0,0,Blue);
         return;
         }
      }
   //2.COUNT CANDLES SINCE CROSSSOEVR      
   for (count=0; count<Bars; count++)
      {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      nBars = nBars + 1;    
      }
                
   //3.CLOSE BLOCK    
   for(cnt=0;cnt<total;cnt++)
      {
      if(!OrderSelect(cnt,SELECT_BY_POS,MODE_TRADES))
      continue;
      if(OrderType()==OP_BUY && OrderSymbol()==Symbol()&& nBars>=20)
      continue;
      if(!OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue))
        {
        Print("OrderClose error ",GetLastError());
        }
}}

Keith Watford 2020.06.03 21:56 #1

The first thing that you need to do is change this

//2.COUNT CANDLES SINCE CROSSSOEVR      
   for (count=0; count<Bars; count++)
      {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      nBars = nBars + 1;    
      }

You do not initialize nBars.

You do not check for any crossover. When you do re-code it to find the crossover, you should break.

In future please post in the correct section

I will move this topic to the MQL4 and Metatrader 4 section.

Need a correction hand Help needed - Signal Help coding simple indicator

Davide.P 2020.06.03 23:09 #2

Keith Watford:

The first thing that you need to do is change this

You do not initialize nBars.

You do not check for any crossover. When you do re-code it to find the crossover, you should break.

In future please post in the correct section

I will move this topic to the MQL4 and Metatrader 4 section.

Hi Keith,

Thanks for moving my post to the right section and thx for your hints.

The code works now, but for the first crossover only. It doesn't loop when the next trade BUY trade enters.

Could you please tell me what the error is?

Many thx

void OnTick(void)
  {
int count,ticket,CloseTicket,total,nBars;

   //1.OPEN BLOCK
   total=OrdersTotal();
   if(total<1)
      {
      double FstMa0=iMA(NULL,0,12,0,MODE_SMA,0,0);
      double SlwMa0=iMA(NULL,0,26,0,MODE_SMA,0,0);
      double FstMa1=iMA(NULL,0,12,0,MODE_SMA,0,1);
      double SlwMa1=iMA(NULL,0,26,0,MODE_SMA,0,1);
      
      if(FstMa0>SlwMa0 && FstMa1<SlwMa1)
         {
         ticket=OrderSend(Symbol(),OP_BUY,1,Ask,0,0,0,NULL,0,0,Blue);
         return;
         }
      }
   //2.CLOSE TRADE 20 BARS AFTER CROSSSOEVR      
   for (count=0; count<Bars; count++)
      {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      nBars = nBars + 1;    
         
         if(FastSMA0>SlowSMA0 && FastSMA1<SlowSMA1)
             {
             if(nBars>=20)
             CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);
             break;
             }
      }
 }

Indicators: WmiFor 3.5 for Set new pending order CopyFileW()/MoveFileW()/FileMove() problems..

Keith Watford 2020.06.04 03:13 #3

Davide.P:

Hi Keith,

Thanks for moving my post to the right section and thx for your hints.

The code works now, but for the first crossover only. It doesn't loop when the next trade BUY trade enters.

Could you please tell me what the error is?

Many thx

You are still not initializing nBars.

It doesn't loop when the next trade BUY trade enters.

How do you know?

CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);

You have to select an order before you can access OrderTicket().

Davide.P 2020.06.04 11:58 #4

Keith Watford:

You are still not initializing nBars.

It doesn't loop when the next trade BUY trade enters.

How do you know?

You have to select an order before you can access OrderTicket().

Keith, thx again! The code works now.

Please, see below.

void OnTick(void)
  {
int count,OpenTicket,CloseTicket,total,nBars=0,cnt;

   //1.OPEN BLOCK
   total=OrdersTotal();
   if(total<1)
      {
      double FstMa0=iMA(NULL,0,12,0,MODE_SMA,0,0);
      double SlwMa0=iMA(NULL,0,26,0,MODE_SMA,0,0);
      double FstMa1=iMA(NULL,0,12,0,MODE_SMA,0,1);
      double SlwMa1=iMA(NULL,0,26,0,MODE_SMA,0,1);
      
      if(FstMa0>SlwMa0 && FstMa1<SlwMa1)
         {
         OpenTicket=OrderSend(Symbol(),OP_BUY,1,Ask,0,0,0,NULL,0,0,Blue);
         return;
         }
      }
   //2.CLOSE TRADE 20 BARS AFTER CROSSSOEVR      
   for (count=0; count<Bars; count++)
      {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      nBars=count+1;    
         
         if(FastSMA0>SlowSMA0 && FastSMA1<SlowSMA1)
             {
             if(nBars>=10)
                 for(cnt=0;cnt<total;cnt++)
                 {
                  if(!OrderSelect(cnt,SELECT_BY_POS,MODE_TRADES))
                  continue;
                  if(OrderType()==OP_BUY && nBars>=20)
                  continue;
                  CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);
                  break;
                  
}}}}

I would like to understand a couple of things thouogh, if you could spare another minute:

FIRST: I Don't want to use a FOR cycle to close the order since OrdersTotal<1 allows only for 1 trade at the time.
I tried the piece of code below but it didn't work (123 is the magic number given to the OpenTicket). Could you tell me why pls?

SelectTicket=OrderSelect(123,SELECT_BY_POS,MODE_TRADES);
if(OrderType()==OP_BUY && nBars>=20)
{
CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);
break;
}

SECOND: nBars is now initialised. However, why the code works even if it isn't?

Many thanks

Keith Watford 2020.06.06 09:50 #5

Davide.P:
Keith, thx again! The code works now.

Please, see below.

I would like to understand a couple of things thouogh, if you could spare another minute:

FIRST: I Don't want to use a FOR cycle to close the order since OrdersTotal<1 allows only for 1 trade at the time.
I tried the piece of code below but it didn't work (123 is the magic number given to the OpenTicket). Could you tell me why pls?

SECOND: nBars is now initialised. However, why the code works even if it isn't?

Many thanks

FIRST: I Don't want to use a FOR cycle to close the order since OrdersTotal<1 allows only for 1 trade at the time.

You should really be using a magic number and also checking for that. That will allow you to run other EAs or place manual trades on the same account.

By using a loop, you know that you will have more options in future. If there is only 1 trade open, the loop will only run once.

I tried the piece of code below but it didn't work (123 is the magic number given to the OpenTicket). Could you tell me why pls?

SelectTicket=OrderSelect(123,SELECT_BY_POS,MODE_TRADES);

You are selecting an order by its position in the list. Are there at least 124 open trades? If not, the order will not be selected.

SECOND: nBars is now initialised. However, why the code works even if it isn't?

You have changed your code, you are now no longer incrementing it at every pass of the loop, but are assigning the value count +1 to it.

Other points in your code

//2.CLOSE TRADE 20 BARS AFTER CROSSSOEVR  
//
//
if(nBars>=10)

Huh??

Also you should get into the habit of using

#property strict

for (count=0; count<Bars; count++)

as you have no break in the loop you will be accessing bar[count+1] and the maximum will be the same as bar[Bars] which does not exist.

Also,do you really want to check from bar 0 as that is the current bar and not closed yet.

You need to think what your code does.

You should stop the loop once the last cross is found. Because you don't, it may find a cross 5 bars ago, but the loop continues and it may find an earlier cross 100 bars ago and close the order!

So the first thing to do is find ONLY the LAST cross.

   for (count=1; count<Bars-1; count++)
     {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      if(FastSMA0>SlowSMA0 && FastSMA1<SlowSMA1)  //Last cross found
         break;                                   //so exit the loop
     }
   if(count>=20)
     {
      //close trades
     }

Not tested.

Note: nBars is no longer required

Expert Advisor doesn`t trade problem listing my orders how do i change

Davide.P 2020.06.07 15:25 #6

Keith, many thanks for your thorough reply. It definetely took longer than a minute!

All good points and useful tips to improve my modest coding skills.

Talking about the final FOR loop. Your suggstion is more logical and it does works. However, this is true only if the variable in the first IF condition [ if(count>=19) ] is at least 1unit smaller than the variable int the second IF condition [ &&count>=20 ].

If one of the 2 condition is removed, the order doesn't close. I would have expected the second IF condition to not be necessary, since 19 candles are gone already.

   //2.CLOSE TRADE 20 BARS AFTER CROSSSOEVR      
   for (count=1; count<Bars-1; count++)
      {
      double FastSMA0 = iMA(NULL,0,12,0,MODE_SMA,0,count);
      double SlowSMA0 = iMA(NULL,0,26,0,MODE_SMA,0,count);
      double FastSMA1 = iMA(NULL,0,12,0,MODE_SMA,0,count+1);
      double SlowSMA1 = iMA(NULL,0,26,0,MODE_SMA,0,count+1);
      
      if(FastSMA0>SlowSMA0 && FastSMA1<SlowSMA1)
             {
             if(count>=19)
                 for(cnt=0;cnt<total;cnt++)
                 {
                  if(!OrderSelect(cnt,SELECT_BY_POS,MODE_TRADES))
                  continue;
                  if(OrderType()==OP_BUY && count>=20)
                  continue;
                  CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);
                  return;
                  
}}}}

Many thanks for your help

..@@ need help in Help with code for [Archive!] Any rookie question,

Keith Watford 2020.06.07 22:34 #7

Davide.P:
Talking about the final FOR loop. Your suggstion is more logical and it does works. However, this is true only if the variable in the first IF condition [ if(count>=19) ] is at least 1 unit smaller than the variable int the second IF condition [ &&count>=20 ].

If one of the 2 condition is removed, the order doesn't close. I would have expected the second IF condition to not be necessary, since 19 candles are gone already.

if(FastSMA0>SlowSMA0 && FastSMA1<SlowSMA1)
             {
             if(count>=19)
                 for(cnt=0;cnt<total;cnt++)
                 {
                  if(!OrderSelect(cnt,SELECT_BY_POS,MODE_TRADES))
                  continue;
                  if(OrderType()==OP_BUY && count>=20)
                  continue;
                  CloseTicket=OrderClose(OrderTicket(),OrderLots(),Bid,0,Blue);
                  return;
}}

Analyse your code and think about what it is doing.

What if count>=20 and the order is a Sell? What will the code do?

I have told you that you should exit the loop once you have found the last cross, you have not done that.

OrderOpenTime() problem regarding getting How do I automatically Help with this, simple

Davide.P 2020.06.11 11:10 #8

Hi Keith, apologies for my late reply.

1. This piece of code does not allow for sell orders. There is only an open BUY order at the beginning.

2. When the FOR loop is breaked after the if condition:

if(FstMa0>SlwMa0 && FstMa1<SlwMa1)
break;

The order closes after 1 candle, although not always. I invite you to test the code attached to check it yourself.

This doesn't happen when the loop is not breaked.

3. Talking about the count>=20:

The closing order loop starts only if (count>=19). Since this condition is already verified, I don't understand why it needs to be checked a second time.

Kind regards

Davide

Files:

Close_XY_candels_after_MA_Crossover-AsAdvisedByMQL4.mq4 2 kb

[Archive!] Any rookie question, Checking Candle Open How to count bar

Keith Watford 2020.06.11 23:16 #9

Davide.P:

Hi Keith, apologies for my late reply.

1. This piece of code does not allow for sell orders. There is only an open BUY order at the beginning.

2. When the FOR loop is breaked after the if condition:

The order closes after 1 candle, although not always. I invite you to test the code attached to check it yourself.

This doesn't happen when the loop is not breaked.

3. Talking about the count>=20:

The closing order loop starts only if (count>=19). Since this condition is already verified, I don't understand why it needs to be checked a second time.

Kind regards

Davide

1. This piece of code does not allow for sell orders. There is only an open BUY order at the beginning.

Then why do you bother to check if it is a buy?

You should always allow for sell orders even if the EA only opens buys - you may change the code later.

You should also check for magic number and symbol.

Twice I have told you that you should exit the loop when the last cross is found. I even gave you the code.

You have continued to ignore this so I am done here.

Confused about "Trade context Keeping track of profit EA that does not

New comment