EA fixing SL and TP works only on long posiiton

 

I wrote an EA to automate the process of typing SL and TP (code below), however the code works only on buy side. Can some experts tell me what went wrong? The problem seems simple yet as a newbie I can't tell. T.T


//+------------------------------------------------------------------+

//| testing.mq4 |
//| kang0014 |
//| http://www.metaquotes.net |
//+------------------------------------------------------------------+
#property copyright "kang0014"
#property link "http://www.metaquotes.net"

//--- input parameters
extern int TP=100;
extern int SL=100;
extern double Lots=0.1;
//+------------------------------------------------------------------+
//| expert initialization function |
//+------------------------------------------------------------------+

//+------------------------------------------------------------------+
//| expert start function |
//+------------------------------------------------------------------+
int start()
{
//----
for (int i=0;i<=OrdersTotal();i++){
if(OrderType()==OP_SELL){
if(OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(OrderTicket(),OrderOpenPrice(),OrderOpenPrice()+SL*Point,OrderOpenPrice()-TP*Point,0);}}
else if(OrderType()==OP_BUY){
if(OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(OrderTicket(),OrderOpenPrice(),OrderOpenPrice()-SL*Point,OrderOpenPrice()+TP*Point,0);}}}

}

//----


//+------------------------------------------------------------------+
 
kang0024:

I wrote an EA to automate the process of typing SL and TP (code below), however the code works only on buy side. Can some experts tell me what went wrong? The problem seems simple yet as a newbie I can't tell. T.T

I'm amazed it works at all. There's one actual problem and one latent problem with the lines such as the following:

OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)

Firstly, you're ostensibly trying to select the ticket number, but you're using SELECT_BY_POS, not SELECT_BY_TICKET. This ought to fail, and therefore I can't see that the code ever actually runs either OrderModify() command. What you're presumably trying to do is OrderSelect(i, SELECT_BY_POS). And, incidentally, OrderSelect(OrderTicket(), SELECT_BY_TICKET) makes no sense because it means "select the ticket which is already selected". SELECT_BY_TICKET is for use in scenarios where you have already stored a ticket number somewhere, and you want to select that ticket, rather than selecting the nth order in the open or historic lists. The only time when you'd ever do OrderSelect(OrderTicket(), SELECT_BY_TICKET) is when you want to make sure that things like OrderClosePrice() are up to date on an open ticket, because the details of the selected order are only guaranteed to be refreshed at the point when you do an OrderSelect().

Secondly, you should be doing "OrderSelect(i,SELECT_BY_POS,MODE_TRADES)==true", not "OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)". And the ==true bit isn't actually necessary. It's sufficient, and more normal, just to write "if (OrderSelect(i, SELECT_BY_POS, MODE_TRADES))"

What's going to be happening here is that the third parameter for OrderSelect() is calculated as MODE_TRADES==true. This will get evaluated as "0==1" (because MODE_TRADES = 0 and true = 1), and will return false/0. By coincidence, MODE_TRADES has the value 0, and therefore MODE_TRADES==true happens to be the same as MODE_TRADES. But only by chance.

 
jjc:

I'm amazed it works at all. There's one actual problem and one latent problem with the lines such as the following:

Firstly, you're ostensibly trying to select the ticket number, but you're using SELECT_BY_POS, not SELECT_BY_TICKET. This ought to fail, and therefore I can't see that the code ever actually runs either OrderModify() command. What you're presumably trying to do is OrderSelect(i, SELECT_BY_POS). And, incidentally, OrderSelect(OrderTicket(), SELECT_BY_TICKET) makes no sense because it means "select the ticket which is already selected". SELECT_BY_TICKET is for use in scenarios where you have already stored a ticket number somewhere, and you want to select that ticket, rather than selecting the nth order in the open or historic lists. The only time when you'd ever do OrderSelect(OrderTicket(), SELECT_BY_TICKET) is when you want to make sure that things like OrderClosePrice() are up to date on an open ticket, because the details of the selected order are only guaranteed to be refreshed at the point when you do an OrderSelect().

Secondly, you should be doing "OrderSelect(i,SELECT_BY_POS,MODE_TRADES)==true", not "OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)". And the ==true bit isn't actually necessary. It's sufficient, and more normal, just to write "if (OrderSelect(i, SELECT_BY_POS, MODE_TRADES))"

What's going to be happening here is that the third parameter for OrderSelect() is calculated as MODE_TRADES==true. This will get evaluated as "0==1" (because MODE_TRADES = 0 and true = 1), and will return false/0. By coincidence, MODE_TRADES has the value 0, and therefore MODE_TRADES==true happens to be the same as MODE_TRADES. But only by chance.

Hi,

Thanks for ur reply. Does it mean, the code should be as follows:

//+------------------------------------------------------------------+

//| testing.mq4 |
//| kang0014 |
//| http://www.metaquotes.net |
//+------------------------------------------------------------------+
#property copyright "kang0014"
#property link "http://www.metaquotes.net"

//--- input parameters
extern int TP=100;
extern int SL=100;
extern double Lots=0.1;
//+------------------------------------------------------------------+
//| expert initialization function |
//+------------------------------------------------------------------+

//+------------------------------------------------------------------+
//| expert start function |
//+------------------------------------------------------------------+
int start()
{
//----
for (int i=0;i<=OrdersTotal();i++){
if(OrderType()==OP_SELL){
if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(i,OrderOpenPrice(),OrderOpenPrice()+SL*Point,OrderOpenPrice()-TP*Point,0);}}
else if(OrderType()==OP_BUY){
if(OrderSelect(i,SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(i,OrderOpenPrice(),OrderOpenPrice()-SL*Point,OrderOpenPrice()+TP*Point,0);}}}

}

//----


//+------------------------------------------------------------------+
 

Please use this to post code . . . it makes it easier to read.

Your syntax for OrderSelect is wrong . . read the documentation.

 
Seems to make sense that it would only partially work on buy orders

Your expression below started by referring to == OP_SELL and nothing was selected

//----
for (int i=0;i<=OrdersTotal();i++){
if(OrderType()==OP_SELL){
if(OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(OrderTicket(),OrderOpenPrice(),OrderOpenPrice()+SL*Point,OrderOpenPrice()-TP*Point,0);}}
else if(OrderType()==OP_BUY){
if(OrderSelect(OrderTicket(),SELECT_BY_POS,MODE_TRADES==true)){
OrderModify(OrderTicket(),OrderOpenPrice(),OrderOpenPrice()-SL*Point,OrderOpenPrice()+TP*Point,0);}}}

Shouldn't it be Order Selected First ?
OrderSelect(i, SELECT_BY_POS, MODE_TRADES);

Then put the if statements or conditions to test / expressions

And since you have selected by position your condition will be to select if(OrderType()==OP_SELL) after you select by position

and why == true ?
If it's true it will already be true, no need to add additional comparisons I think.

So in your original version above, it appears to test the condition if(OrderType) with no selection made so it moves to expression (OrderSelect)
Then it moves down to the next expression To Modify but nothing was selected yet.
Then read you code to see to the else if(OrderType() == OP_BUY)
This may only partially functional to modify at this point because it's the only one selected and the only condition met.

Now finally in the code order downward your code finally gets to a proper OrderModify which is a little late to the race and in which case it's only going to be the OP_BUY since that is all that is ever selected technically in that order of expressions from top to bottom

I hope this helps and makes sense.

I believe the main problem is the flow of code, should be OrderSelect, then OrderType, then other expressions and testing conditions.

Hope this helps and happy reading as others have suggested. aka documentation.

I find the best way to read is while you have your metaeditor opened and use the dictionary and search to see the grammar and syntax. Reading closely is the key, and sort of scratching my head works for me a little bit. har har har!

Happy coding
 
Agent86:
Seems to make sense that it would only partially work on buy orders

Your expression below started by referring to == OP_SELL and nothing was selected


Shouldn't it be Order Selected First ?

Then put the if statements or conditions to test / expressions

And since you have selected by position your condition will be to select if(OrderType()==OP_SELL) after you select by position

and why == true ?
If it's true it will already be true, no need to add additional comparisons I think.

So in your original version above, it appears to test the condition if(OrderType) with no selection made so it moves to expression (OrderSelect)
Then it moves down to the next expression To Modify but nothing was selected yet.
Then read you code to see to the else if(OrderType() == OP_BUY)
This may only partially functional to modify at this point because it's the only one selected and the only condition met.

Now finally in the code order downward your code finally gets to a proper OrderModify which is a little late to the race and in which case it's only going to be the OP_BUY since that is all that is ever selected technically in that order of expressions from top to bottom

I hope this helps and makes sense.

I believe the main problem is the flow of code, should be OrderSelect, then OrderType, then other expressions and testing conditions.

Hope this helps and happy reading as others have suggested. aka documentation.

I find the best way to read is while you have your metaeditor opened and use the dictionary and search to see the grammar and syntax. Reading closely is the key, and sort of scratching my head works for me a little bit. har har har!

Happy coding


Thank you Agent86, this is the best answer I can hope for! Really appreciate your help=)

Happy coding too!

 

Hi,

I modified my code as follows to work on buy stop and sell stop, but it doesnt work again:

//+------------------------------------------------------------------+
//|                                                      testing.mq4 |
//|                                                         kang0014 |
//|                                        http://www.metaquotes.net |
//+------------------------------------------------------------------+
#property copyright "kang0014"
#property link      "http://www.metaquotes.net"

//--- input parameters
extern int       TP=100;
extern int       SL=100;
extern double    Lots=0.1;
//+------------------------------------------------------------------+
//| expert initialization function                                   |
//+------------------------------------------------------------------+

//+------------------------------------------------------------------+
//| expert start function                                            |
//+------------------------------------------------------------------+
int start()
  {
//----
for (int i=0;i<=OrdersTotal();i++){
OrderSelect(i,SELECT_BY_POS,MODE_TRADES);
if(OrderType()==OP_SELL || OP_SELLSTOP){
OrderModify(i,OrderOpenPrice(),OrderOpenPrice()+SL*Point,OrderOpenPrice()-TP*Point,0);}
else if(OrderType()==OP_BUY || OP_BUYSTOP){
OrderModify(i,OrderOpenPrice(),OrderOpenPrice()-SL*Point,OrderOpenPrice()+TP*Point,0);}}}

//----


//+------------------------------------------------------------------+

Can anyone guess what had happened?

 

Can you explain what this does . . in plan simple English . . in your opinion.

if(OrderType()==OP_SELL || OP_SELLSTOP)
 

Yes, the code is still broken.

  1. if(OrderType()==OP_SELL || OP_SELLSTOP)

  2. for (int i=0;i<=OrdersTotal();i++){
    OrderSelect(i,SELECT_BY_POS,MODE_TRADES);
    A) orders are positions 0 to total - 1. You DON'T check your return code so the select fails and everything below that is bogus.
    for(int iPos = OrdersTotal()-1; iPos >= 0; iPos--) if (
       OrderSelect(iPos, SELECT_BY_POS) ){
    
    B) You must count down in the presence of multiple orders.
 
kang0024:

Hi WHRoeder

Thanks for your reply. But I am kind of confused. Would you mind writing a complete code for me? Thank you!


Aren't you reading.... If this is still not clear to you ..... then reading the MQL Book will be very difficult to you

If you want someone to do it for you then use Jobs https://www.mql5.com/en/job

Hope it is not needed for you but if it is I will do it there for $5 (coffee)

 
kang0024:
Thanks for your reply. But I am kind of confused. Would you mind writing a complete code for me? Thank you!
No Slaves here, learn to code or pay someone. We're not going to code it FOR you. We are willing to HELP you.
Reason: