EA Buy / Sell Code Problem - page 2

New comment
 
phi.nuts:

1. Can you attach "Center Of Gravity Oscillator" CI ?

2. Assign your iCustom to a variable, it will make your code more readable. 

Phi.nuts,


See attached Center of Gravity Oscillator; I need explicit instructions for assigning the icustom to a variable to make it more readable.Thanks for your reply.

 
resirca52009:

Phi.nuts,


See attached Center of Gravity Oscillator; I need explicit instructions for assigning the icustom to a variable to make it more readable.Thanks for your reply.


Thank you for the attachment, actually you can attach the .mq4 file which also can be opened on notepad.

Your iCustom call is wrong. Here's parameters in iCustom that we need to fill

double iCustom (string Symbol_Name, integer Symbol_TimeFrame, string Indicator_Name, All_extern_input_of_indicator, integer indicator buffer_number, integer bar_number);

So, for example your iCustom call should be like this :

//---- input parameters
extern double   length = 8;
//extern int    maxbars = 2000;
extern string   Price_Choice = "-----  Typical or Median  --------------";
extern bool     is_median = true;

double values;

values = iCustom(Symbol(), Period(), "Center of Gravity Oscillator", length, Price_Choice, is_median, 0, 1); // using buffer 0 at bar 1

Off course you can put in the extern input directly inside iCustom, like this :

values = iCustom(Symbol(), Period(), "Center of Gravity Oscillator", 8, "-----  Typical or Median  --------------", true, 0, 1); // using buffer 0 at bar 1

In your code, you only write 2 extern input, while actually, there are 3 :(, you forget the "-----  Typical or Median  --------------" one.

Second mistake you've made is, that there are 2 buffers in CGO, they are buffer 0 and 1, yet in your code your write for buffer number 2 - which is third buffer in CGO. 

Seem like you coding for position opening is the same for closing opposite position. In this case, we can write the all those codes in one place, but you have to search first if there's already opened position before closing or opening position. 

I didn't test these code in MT nor compile it, so there may be some errors :D. 

It may good idea to answer RaptorUK request for your OrderSend/Close codes

double CALL_iCGO (int buffer, int bar)
  {
  return (iCustom(Symbol(), 0, "Center Of Gravity Oscillator",8,"-----  Typical or Median  --------------",true, buffer, bar));
  }

double buffer_0_bar_0, buffer_0_bar_1, buffer_0_bar_2, buffer_0_bar_3,  
       buffer_1_bar_0, buffer_1_bar_1, buffer_1_bar_2, buffer_1_bar_3;
       
       //--- Assign iCustom return value to a variable so the code is readable. 
       
       //--- collecting data for first line of CGO
       buffer_0_bar_0 = CALL_iCGO (0,0);  //--- first line on bar 0
       buffer_0_bar_1 = CALL_iCGO (0,1);  //--- first line on bar 1
       buffer_0_bar_2 = CALL_iCGO (0,2);  //--- first line on bar 2 
       buffer_0_bar_3 = CALL_iCGO (0,3);  //--- first line on bar 3  
       
       //--- collecting data for second line of CGO
       buffer_1_bar_0 = CALL_iCGO (1,0);  //--- second line on bar 0
       buffer_1_bar_1 = CALL_iCGO (1,1);  //--- second line on bar 1
       buffer_1_bar_2 = CALL_iCGO (1,2);  //--- second line on bar 2
       buffer_1_bar_3 = CALL_iCGO (1,3);  //--- second line on bar 3 
       
  //************GO LONG EXIT SHORT

  if ( buffer_0_bar_1 > buffer_1_bar_1 &&  //--- if first line on bar 1 is bigger than second line on bar 1 ... AND
       buffer_0_bar_3 <= buffer_1_bar_3)   //--- if first line on bar 3 is smaller or equal than second line on bar 3
      {
      //--- here you have to check if there any open position.
      
      //--- exit short
      Print("Trading condition [(iCustom(Symbol(), 0, Center Of Gravity Oscillator...] is valid. Trying to close short position...");
      CloseShort(Symbol());
      
      //--- open long
      Print("Trading condition [(iCustom(Symbol(), 0, Center Of Gravity Oscillator...] is valid. Trying to open long position...");
      Buy(Symbol(), 0.01, 30, 15, 15); 
      }

  
  //************GO SHORT EXIT LONG

  if ( buffer_0_bar_1 < buffer_1_bar_1 &&  //--- if first line on bar 1 is smaller than second line on bar 1 ... AND
       buffer_0_bar_3 >= buffer_1_bar_3)   //--- if first line on bar 3 is bigger or equal than second line on bar 3
      { 
      //--- here you have to check if there any open position.
      
      //--- exit short
      Print("Trading condition [(iCustom(Symbol(), 0, Center Of Gravity Oscillator...] is valid. Trying to close long position...");
      CloseLong(Symbol());
      
      //--- open long
      Print("Trading condition [(iCustom(Symbol(), 0, Center Of Gravity Oscillator...] is valid. Trying to open long position...");
      Sell(Symbol(), 0.01, 30, 15, 15); 
      }
 
phi.nuts:

Thank you for the attachment, actually you can attach the .mq4 file which also can be opened on notepad.

Your iCustom call is wrong. Here's parameters in iCustom that we need to fill

So, for example your iCustom call should be like this :

Off course you can put in the extern input directly inside iCustom, like this :

In your code, you only write 2 extern input, while actually, there are 3 :(, you forget the "-----  Typical or Median  --------------" one.

Second mistake you've made is, that there are 2 buffers in CGO, they are buffer 0 and 1, yet in your code your write for buffer number 2 - which is third buffer in CGO. 

Seem like you coding for position opening is the same for closing opposite position. In this case, we can write the all those codes in one place, but you have to search first if there's already opened position before closing or opening position. 

I didn't test these code in MT nor compile it, so there may be some errors :D. 

It may good idea to answer RaptorUK request for your OrderSend/Close codes


phi.nuts,

Thanks for your reply; I will review today and post my findings asap. I read through the icustom section yesterday after reading your first reply but it is hard to find a solution to a problem when you are unaware of exactly what the problem is , thanks again for helping me facilitate the learning process.

 
RaptorUK:
It probably is . . .  but it is not the code that places the Orders.  If your print output to the log agrees with the trades that are being taken then the issue may not be with the code that actually places the orders . . .



Raptor,

I believe this is what you were asking for :

Files:
ordersexecutiondcode.txt  2 kb
 
resirca52009:

Raptor,

I believe this is what you were asking for :

Yes and no . . .

It shows the  Buy()and Sell() functions but your code is convoluted so we still don't get to see the actual functions that do the OrderSend().

Your ExecuteOrder() and ExecuteECNOrder() functions take exactly the same parameter so they could easily be one function whose behaviour is determined by the value of isECN, this one function would also take the place of your Buy() and Sell() functions . . . you pass the order type to ExecuteOrder() and ExecuteECNOrder() so there is no need for the Buy() and Sell() functions . . . if you want to provide the code for all these functions I'll show you what I mean.


Why does this function call another function twice . . .

int CloseLong(string symbol) {
 
        CloseAllPositions(symbol,OP_BUY,MagicNumber);
        CloseAllPositions(symbol,OP_BUY,MagicNumber);
        
        return(0);
}

 I suspect it may well be because the function CloseAllPositions() is coded incorrectly and has a loop that counts up instead of down,  this means it will not close all Orders and hence the need to call it twice . . .

If I am correct this will help:  Loops and Closing or Deleting Orders

 
resirca52009:

phi.nuts,

Thanks for your reply; I will review today and post my findings asap. I read through the icustom section yesterday after reading your first reply but it is hard to find a solution to a problem when you are unaware of exactly what the problem is , thanks again for helping me facilitate the learning process.


phi.nuts,

I have made some significant changes over the past month. I didn't code this exactly as you had indicated previously ( there are numerous ways to achieve the same outcome ). Attached is the code in its entirety along with the oscillator code. I don't believe results from the strategy tester are very accurate so I have been forward testing on a live demo account. Those results vary a great deal and do not make sense.I'd appreciate any insight you may be able to provide.

Richard

 
resirca52009:

phi.nuts,

I have made some significant changes over the past month. I didn't code this exactly as you had indicated previously ( there are numerous ways to achieve the same outcome ). Attached is the code in its entirety along with the oscillator code. I don't believe results from the strategy tester are very accurate so I have been forward testing on a live demo account. Those results vary a great deal and do not make sense.I'd appreciate any insight you may be able to provide.

Richard


phi.nuts,

Sorry, the EA code didn't make it first time....

 
resirca52009:

phi.nuts,

I have made some significant changes over the past month. I didn't code this exactly as you had indicated previously ( there are numerous ways to achieve the same outcome ). Attached is the code in its entirety along with the oscillator code. I don't believe results from the strategy tester are very accurate so I have been forward testing on a live demo account. Those results vary a great deal and do not make sense.I'd appreciate any insight you may be able to provide.

Richard


You should make your code more readable, so it's easy for everyone to study your code.

So instead using

double x=MarketInfo(Symbol(),14);

You can use

double x = MarketInfo (Symbol(), MODE_STOPLEVEL);

I have to go though MarketInfo's Standard Constant, just to find out what is 14 in MarketInfo.

Your iCustom call is also wrong, the first parameter of iCustom is string, yet you put in an integer, and other parameter of iCustom is indicator extern input. It's better to write those inputs in iCustom so you can change the indicator input right from the EA and the EA also won't be disturbed when the default input parameter in indicator is changed.

 
phi.nuts:

You should make your code more readable, so it's easy for everyone to study your code.

So instead using

You can use

I have to go though MarketInfo's Standard Constant, just to find out what is 14 in MarketInfo.

Your iCustom call is also wrong, the first parameter of iCustom is string, yet you put in an integer, and other parameter of iCustom is indicator extern input. It's better to write those inputs in iCustom so you can change the indicator input right from the EA and the EA also won't be disturbed when the default input parameter in indicator is changed.

 

phi.nuts,

Thanks for your reply and assistance; I'll review the icustom again........

12
New comment
Reason: