Is this a correct code to close pending STOP orders
The code is inefficient because you are looping through all open orders on each tick regardless of the new-bar status. You need to check for a new bar first and then close the stop orders. Example:
#include <stdlib.mqh> void OnStart() { if (is_newbar()) if (!close_stops_on_newbar()) Print("<!!!> ", ErrorDescription(GetLastError())); } bool close_stops_on_newbar(int magic=0, int attempts=5) { int x = attempts; for (; x>0; --x) { ResetLastError(); bool no_errors = true; for (int i=OrdersTotal()-1; i>=0; --i) { if (OrderSelect(i, SELECT_BY_POS) && OrderSymbol() == _Symbol && OrderMagicNumber() == magic && (OrderType() == OP_BUYSTOP || OrderType() == OP_SELLSTOP) ){ no_errors &= OrderDelete(OrderTicket()); } } if (no_errors) { break; } } // if x is less than one then the function has exhausted all // attempts and is not able to delete the pending order. return x > 0; } bool is_newbar() { static datetime last_newbar = WRONG_VALUE; datetime curr_bar = datetime(SeriesInfoInteger( _Symbol, PERIOD_CURRENT, SERIES_LASTBAR_DATE )); if (curr_bar != last_newbar) { bool res = (last_newbar != WRONG_VALUE); last_newbar = curr_bar; return res; } return false; }
The code is inefficient because you are looping through all open orders on each tick regardless of the new-bar status. You need to check for a new bar first and then close the stop orders. Example:
Thank you for the info and code example.
Is there anything else you think missing in my code that i should consider?
Otherwise to achieve the EA not to loop each tick i should simply just move this (i believe?) :
if(trade_once_per_bar!=Time[0]) {
Up to just below:
int start()
{
Like this:
int input_magic = 999; int retry_interval = 100; int max_retries_per_tick = 2; datetime trade_once_per_bar; //+------------------------------------------------------------------+ //| | //+------------------------------------------------------------------+ int start() { if(trade_once_per_bar!=Time[0]) { for(int b=0; b<OrdersTotal();b++) { if(OrderSelect(b,SELECT_BY_POS,MODE_TRADES)==TRUE) { if(OrderSymbol()==Symbol() && OrderMagicNumber()==input_magic) { if(OrderType()==4) { double bp_totalorders = OrdersTotal(); int bp_ticket = OrderTicket(); } if(OrderType()==5) { double sp_totalorders = OrdersTotal(); int sp_ticket = OrderTicket(); } } } } //-----------------------------------------------------------------// // loop thru open orders to get data // //-----------------------------STOP--------------------------------// //--------------------------START--------------------------// // close all stop orders each new bar // //---------------------------------------------------------// int tries=0; if(bp_totalorders>0) { int delete_buy_pending; while(tries<max_retries_per_tick) { delete_buy_pending=OrderDelete(bp_ticket); if(delete_buy_pending>0) { Print("Deleted pending : ",bp_ticket); }else{ Print("Error delete pending : ",GetLastError()," ticket:",bp_ticket); return(delete_buy_pending); } if(delete_buy_pending)break; tries++; Sleep(retry_interval); } } if(sp_totalorders>0) { int delete_sell_pending; while(tries<max_retries_per_tick) { delete_sell_pending=OrderDelete(sp_ticket); if(delete_sell_pending>0) { Print("Deleted pending : ",sp_ticket); }else{ Print("Error delete pending : ",GetLastError()," ticket:",sp_ticket); return(delete_sell_pending); } if(delete_sell_pending)break; tries++; Sleep(retry_interval); } } trade_once_per_bar=Time[0]; } //---------------------------------------------------------// // close all stop orders each new bar // //---------------------------STOP--------------------------// return(0); } //+------------------------------------------------------------------+
Thank you for the info and code example.
Is there anything else you think missing in my code that i should consider?
Otherwise to achieve the EA not to loop each tick i should simply just move this (i believe?) :
Up to just below:
Like this:
Yes.
- The start() function (catch-all event handler) has been deprecated for quite some time and probably won't compile under the new builds. Instead you should be using the proper event handlers. If you don't know them then please consult the docs.
- If variables aren't initialized then they can start with any random value. The global variables need to be initialized. (*you should not use non-constant-global-variables)
- The trade_once_per_bar variable will never be equal to Time[0] when the program first launches so it will always run through the algo once on initialization which could be at any point during the bar.
- Iterating the trade-pool is always a race-condition and at any given time orders can be added/removed by either your broker or another EA. Because of this race condition you need to always iterate in reverse or use recursion. Most people just iterate in reverse.
- While you've done an excellent job with your variable naming conventions/style, you should try to keep your indention consistent. A couple places have inconsistent indentation which can cause confusion for you when you come back to this much later.
- You code is repeating a lot and any time you see repetative code that should be a red-flag that you need to refactor.
- There's nothing programmatically wrong with deeply nested expressions, however, they are very (very) difficult to manage without a code editor that does folding. You should strive to keep your expressions flat when possible.
Yes.
- The start() function (catch-all event handler) has been deprecated for quite some time and probably won't compile under the new builds. Instead you should be using the proper event handlers. If you don't know them then please consult the docs.
- If variables aren't initialized then they can start with any random value. The global variables need to be initialized. (*you should not use non-constant-global-variables)
- The trade_once_per_bar variable will never be equal to Time[0] when the program first launches so it will always run through the algo once on initialization which could be at any point during the bar.
- Iterating the trade-pool is always a race-condition and at any given time orders can be added/removed by either your broker or another EA. Because of this race condition you need to always iterate in reverse or use recursion. Most people just iterate in reverse.
- While you've done an excellent job with your variable naming conventions/style, you should try to keep your indention consistent. A couple places have inconsistent indentation which can cause confusion for you when you come back to this much later.
- You code is repeating a lot and any time you see repetative code that should be a red-flag that you need to refactor.
- There's nothing programmatically wrong with deeply nested expressions, however, they are very (very) difficult to manage without a code editor that does folding. You should strive to keep your expressions flat when possible.
Thank you for the reply.
I have modified my code again, see code below. I know i might be using you code instead. But for me it’s easier to understand and learn if I modify my original code. =)
The start() function (catch-all event handler) has been deprecated for quite some time and probably won't compile under the new builds. Instead you should be using the proper event handlers. If you don't know them then please consult the docs.
I have changed this to OnTick() now, because I didn’t get OnStart() to work, is this wrong?
If variables aren't initialized then they can start with any random value. The global variables need to be initialized. (*you should not use non-constant-global-variables).
Is it correct to move these above the Ontick() then to initialize them like below?
double bp_totalorders=0; int bp_ticket=0; double sp_totalorders=0; int sp_ticket=0; int tries=0;
The trade_once_per_bar variable will never be equal to Time[0] when the program first launches so it will always run through the algo once on initialization which could be at any point during the bar.
Ok, thank you.
Iterating the trade-pool is always a race-condition and at any given time orders can be added/removed by either your broker or another EA. Because of this race condition you need to always iterate in reverse or use recursion. Most people just iterate in reverse.
But isn’t this prevented by using magic number? I mean the broker can’t add a order with my magic no, or? And other EAs i have i will give another magic no.
While you've done an excellent job with your variable naming conventions/style, you should try to keep your indention consistent. A couple places have inconsistent indentation which can cause confusion for you when you come back to this much later.
Ok, thank you.
You code is repeating a lot and any time you see repetative code that should be a red-flag that you need to refactor. Sorry my bad understanding, but do you mean that I should use more loops and make my code shorter?
There's nothing programmatically wrong with deeply nested expressions, however, they are very (very) difficult to manage without a code editor that does folding. You should strive to keep your expressions flat when possible. Thank you for the hint, I will try to consider this in future =)
//+------------------------------------------------------------------+ //| ProjectName | //| Copyright 2012, CompanyName | //| http://www.companyname.net | //+------------------------------------------------------------------+ #include <stdlib.mqh> int input_magic = 999; int retry_interval = 100; int max_retries_per_tick = 5; double bp_totalorders=0; int bp_ticket=0; double sp_totalorders=0; int sp_ticket=0; int tries=0; //+------------------------------------------------------------------+ //| | //+------------------------------------------------------------------+ void OnTick() { if(is_newbar()) { if(!close_stops_on_newbar()) { Print("<!!!> ",ErrorDescription(GetLastError())); } } } //+------------------------------------------------------------------+ //| | //+------------------------------------------------------------------+ bool close_stops_on_newbar() { //----------------------------------START--------------------------// // loop thru open orders to get data // //-----------------------------------------------------------------// for(int b=0; b<OrdersTotal();b++) { if(OrderSelect(b,SELECT_BY_POS,MODE_TRADES)==TRUE) { if(OrderSymbol()==Symbol() && OrderMagicNumber()==input_magic) { if(OrderType()==4) { bp_totalorders = OrdersTotal(); bp_ticket = OrderTicket(); } if(OrderType()==5) { sp_totalorders = OrdersTotal(); sp_ticket = OrderTicket(); } } } } //-----------------------------------------------------------------// // loop thru open orders to get data // //-----------------------------STOP--------------------------------// //--------------------------START--------------------------// // close all stop orders each new bar // //---------------------------------------------------------// if(bp_totalorders>0) { int delete_buy_pending; while(tries<max_retries_per_tick) { delete_buy_pending=OrderDelete(bp_ticket); if(delete_buy_pending>0) { Print("Deleted pending : ",bp_ticket); break; }else{ Print("Error delete pending : ",GetLastError()," ticket:",bp_ticket); return(true); } tries++; Sleep(retry_interval); } } if(sp_totalorders>0) { int delete_sell_pending; while(tries<max_retries_per_tick) { delete_sell_pending=OrderDelete(sp_ticket); if(delete_sell_pending>0) { Print("Deleted pending : ",sp_ticket); break; }else{ Print("Error delete pending : ",GetLastError()," ticket:",sp_ticket); return(true); } tries++; Sleep(retry_interval); } } return(true); } //---------------------------------------------------------// // close all stop orders each new bar // //---------------------------STOP--------------------------// bool is_newbar() { static datetime last_newbar=WRONG_VALUE; datetime curr_bar=datetime(SeriesInfoInteger(_Symbol,PERIOD_CURRENT,SERIES_LASTBAR_DATE)); if(curr_bar!=last_newbar) { bool res=(last_newbar!=WRONG_VALUE); last_newbar=curr_bar; return res; } return false; } //+------------------------------------------------------------------+
- Free trading apps
- Over 8,000 signals for copying
- Economic news for exploring financial markets
You agree to website policy and terms of use
Hi
I want to close 2 open stop orders on each new bar.
It is:
1 BUYSTOP order
and
1 SELLSTOP order
The below code works fine in tester and on demo account, and shows no errors or warnings when i compile. But before using on live account i want to make sure i am not missing anything important.
1. Do i need to add any function?
2. Is there any wrong with my code, and if so what?