Breaking the For Loop cycle. - page 2

New comment
 
Amir Yacoby #:

1. Your loop runs on all your orders, not on symbols (and not when a new order appears). As you posted, you have orders open on all those symbols.

2. On each tick, each EA on each chart attached, is checking all the orders (other symbols than the chart included) and prints a message on the first order not of that chart's symbol, and exits - until next tick which will print it again because it reads the orders again.

Now it depends what you want to achieve with that loop.


Exiting on the first order which is not of the chart - I don't see a reason for that. So in case I'm right, first thing to do is remove the return when the order symbol does not match - instead place a 'continue' which goes to next order in the loop.

second, remove the printed message when the symbol does not match
.
Also, you can see in the log from which ea attached to which chart symbol/timeframe the message came from - just try to locate one ea on one chart.


And state what you want to achieve with that loop.

*Maybe you think that your loop runs when a new order appears - but that's not the case as you placed it in the OnTick.

If that's the case, then you need OnTradeTransaction and use different approach

Yep, I've changed this to a continue and it's still looping crazy, this s why this is infuriating me. 


 
TheHonestPrussian #:

Yep, I've changed this to a continue and it's still looping crazy, this s why this is infuriating me. 


Still haven't stated your purpose.
If you want each ea to process only its orders, then stop printing at all, just check and if it is not an order to process use continue.

What you are doing is reading all the orders again and again, of all the charts, by each ea, every tick.
The orders are not ordered by symbol, you just have to read them all.

Another alternative is to save an array of tickets of only that symbol/timeframe/magic and work on it.

And most efficient is using OnTradeTransaction to process only new ordees.
 
Amir Yacoby #:
Still haven't stated your purpose.
If you want each ea to process only its orders, then stop printing at all, just check and if it is not an order to process use continue.

What you are doing is reading all the orders again and again, of all the charts, by each ea, every tick.
The orders are not ordered by symbol, you just have to read them all.

Another alternative is to save an array of tickets of only that symbol/timeframe/magic and work on it.

And most efficient is using OnTradeTransaction to process only new ordees.

I will try the OnTradeTransaction

My purpose is for the function to place a close button on the chart, so when I click the button it closes the position. 

The function itself works perfectly but it just wont stop looping through all the other charts with which the trade has not been placed. I know why it's going it I just can't get it to kick out when it realises the the EA doesnt apply to a particular chart. 

Infuriating. 

I am now going to walk away from this for a few hours because I can feel my blood pressure rising. I will circle back to this later. 

 
TheHonestPrussian #:

I will try the OnTradeTransaction

My purpose is for the function to place a close button on the chart, so when I click the button it closes the position. 

The function itself works perfectly but it just wont stop looping through all the other charts with which the trade has not been placed. I know why it's going it I just can't get it to kick out when it realises the the EA doesnt apply to a particular chart. 

Infuriating. 

I am now going to walk away from this for a few hours because I can feel my blood pressure rising. I will circle back to this later. 

Why do you need a loop for a close button..
 
Because the close button has a real-time profit or loss displayed as the text, which is gleamed directly from the ledger, which is updated OnTick();
 
TheHonestPrussian #:
Because the close button has a real-time profit or loss displayed as the text, which is gleamed directly from the ledger, which is updated OnTick();
Ok, you do it all wrong. 

You have many options, not using loops through all orders in system.

1. Calculate (once only when a new order is placed or removed) the average volume/price of all the open orders of your ea symbol. Each change in the open orders, like delete, tp, sl, you recalculate this average in OnTradeTransaction. When you need to show p/l,  use that average price/volume in relation to current price

2. Much worse, but still better than what you do, keep an array of tickets for each ea
 
Amir Yacoby #:
Ok, you do it all wrong. 

You have many options, not using loops through all orders in system.

1. Calculate (once only when a new order is placed or removed) the average volume/price of all the open orders of your ea symbol. Each change in the open orders, like delete, tp, sl, you recalculate this average in OnTradeTransaction. When you need to show p/l,  use that average price/volume in relation to current price

2. Much worse, but still better than what you do, keep an array of tickets for each ea

I will look into your suggestions. Thanks for your time :) 

 

Amir Yacoby #:

Each change in the open orders, like delete, tp, sl, you recalculate this average in OnTradeTransaction. When you need to show p/l,  use that average price/volume in relation to current price

MQL4 doesn't have  OnTradeTransaction does it?

 

Amir Yacoby #: And most efficient is using OnTradeTransaction to process only new ordees ... Each change in the open orders, like delete, tp, sl, you recalculate this average in OnTradeTransaction.

You are mixing MQL4 and MQL5 concepts. The OnTradeTransaction event handler is not available on MQL4, only on MQL5.

EDIT: Oops, Keith had already stated the same thing.

 
Well, I left MQL4 as well as MQL3/2.. long time ago. Sorry for that, maybe I won't reply to those posts next time. 

Anyway, then we are left with OnTrade aren't we, which is more complicated.. Or the hand made methods to identify new orders like changes to OrdersTotal() etc.. 

Anyway, the loops are clearly not the way. 
1234
New comment