Should I sum profit first or close orders first?

New comment
 

 Please refer to the codes below. Is there any potential issue if I calculate order profit before closing the order?

How should I improve the codes? Thanks for help!

 

   for(int i=ArrOrderSize-1; i>=0; i--)
     {
      Chk=OrderSelect(ArrOrderIDs[i],SELECT_BY_TICKET);
      if(Chk>0 && OrderSymbol()==Symbol())
        {
         TotalProfit+=(OrderProfit()+OrderCommission()+OrderSwap());
         if(OrderType()==OP_BUY)
           {
            Chk=OrderClose(OrderTicket(),OrderLots(),Bid,0);
           }
         else if(OrderType()==OP_SELL)
           {
            Chk=OrderClose(OrderTicket(),OrderLots(),Ask,0);
           }
         .....
         .....
 
jollydragon: Is there any potential issue if I calculate order profit before closing the order?
  1. Slippage on close could change profit.
  2. No need to test OrderType, just use OrderClosePrice() instead of Bid/Ask
  3. OrderSelect returns a bool. What does true>0 or false>0 mean?
      for(int i=ArrOrderSize-1; i>=0; i--)
     {
      Chk=OrderSelect(ArrOrderIDs[i],SELECT_BY_TICKET);
      if(Chk>0 && OrderSymbol()==Symbol())
        {
    		 
      for(int i=ArrOrderSize-1; i>=0; i--) if(
     OrderSelect(ArrOrderIDs[i],SELECT_BY_TICKET)
  && OrderSymbol()==Symbol())
        {
    This is why I say: You would never write if( (2+2 == 4) == true) would you? if(2+2 == 4) is sufficient. So Don't write if(bool == true), just use if(bool) or if(!bool). Code becomes self documenting when you use meaningful variable names, like bool isLongEnabled. Long_Entry sounds like a trigger price or a ticket number and "if long entry" is an incomplete sentence.
  4. You select by ticket, what if the order has already closed OrderCloseTime != 0?
  5. What if the EA/terminal/pc is restarted, do you recover your ticket numbers?
    If the power fails, OS crashes, terminal or chart is accidentally closed, the next tick, the ticket variable will be lost. You will have an open order but don't know it, so the EA will never try to close it, trail SL, etc. How are you going to recover? Use a orderSelect loop no recovery, or persistent storage (GV/file) of ticket numbers required.
 
WHRoeder:
  1. Slippage on close could change profit.
  2. No need to test OrderType, just use OrderClosePrice() instead of Bid/Ask
  3. OrderSelect returns a bool. What does true>0 or false>0 mean?
     This is why I say: You would never write if( (2+2 == 4) == true) would you? if(2+2 == 4) is sufficient. So Don't write if(bool == true), just use if(bool) or if(!bool). Code becomes self documenting when you use meaningful variable names, like bool isLongEnabled. Long_Entry sounds like a trigger price or a ticket number and "if long entry" is an incomplete sentence.
  4. You select by ticket, what if the order has already closed OrderCloseTime != 0?
  5. What if the EA/terminal/pc is restarted, do you recover your ticket numbers?
    If the power fails, OS crashes, terminal or chart is accidentally closed, the next tick, the ticket variable will be lost. You will have an open order but don't know it, so the EA will never try to close it, trail SL, etc. How are you going to recover? Use a orderSelect loop no recovery, or persistent storage (GV/file) of ticket numbers required.

 

 Dear Sir, let me show the appreciation first for your kind reply.

1. May I double confirm if it's better to put this sentence behind OrderClose(), " TotalProfit+=(OrderProfit()+OrderCommission()+OrderSwap());"? 

2. Do you mean OrderClosePrice() is available before OrderClose()? Not Bid or Ask, then what will it be?

3. Thanks for reminding.

4. There is ArrOrderIDs[i] which controls the order is open.

5. There is InitOrders() in OnInit() to re-scan all orders. 

 

1) After the close, Reselect by ticket (to update values) then calculate profit.

2) OrderClosePrice will be either Bid or Ask depending on the OrderType.

New comment
Reason: