Boolean Function that checks for an open order

 

Here's a function that checks to see if there is an open order. I'm a rank beginner at coding in MT4 so I welcome your comments. But it does seem to work.

bool NoOpenOrdersExist() //True when no open order, False when is open order
{
    int total = OrdersTotal();
    for(int i = 0; i < total; i++)
    {
        OrderSelect(i,SELECT_BY_POS,MODE_TRADES);
        if (OrderMagicNumber() == Magic && OrderSymbol() == Symbol())
        {
            Print ("no open order, false");
            return (false);
        }
    }
    Print ("no open order, true");
    return (true);
}// End "NoOpenOrdersExist()"

 

MisterDog 2012.06.04 08:08

Here's a function that checks to see if there is an open order. I'm a rank beginner at coding in MT4 so I welcome your comments. But it does seem to work

Maybe, if you print the result correctly ... :D

if (OrderSelect(i,SELECT_BY_POS,MODE_TRADES) == True
    && OrderMagicNumber() == Magic 
    && OrderSymbol() == Symbol())
    {
    Print ("no open order, false"); // What ????
    return (false); // is it true ???
    }
 
onewithzachy:

Maybe, if you print the result correctly ... :D


logic on this thing is a little confusing. It is a "no open orders exist" function rather than an "open orders exist" function. When I print, "no open orders, false" you could translate that as, "there is an open order". And your right to return (false) really means it's true there is an open order. Do you think I should rewrite it a little more intuitively -- Call it an "open order exist" function so I could return True if there was an open order?
 
onewithzachy:

Maybe, if you print the result correctly ... :D

The result is printed correctly, it's a double negative . . . No Open Order Exists = false means an Open Order Does Exist . . . the OP should take on board what you have shown with the OrderSelect . . and should read this: What are Function return values ? How do I use them ?
 
MisterDog:

logic on this thing is a little confusing. It is a "no open orders exist" function rather than an "open orders exist" function. When I print, "no open orders, false" you could translate that as, "there is an open order". And your right to return (false) really means it's true there is an open order. Do you think I should rewrite it a little more intuitively -- Call it an "open order exist" function so I could return True if there was an open order?

   bool NoOpenOrdersExist() //True when no open order, False when is open order
{
    int total = OrdersTotal();
    for(int i = 0; i < total; i++)
    {
        OrderSelect(i,SELECT_BY_POS,MODE_TRADES);
        if (OrderMagicNumber() == Magic && OrderSymbol() == Symbol())
        {
            Print ("order found");
            return (false);      // not true  no order
        }
    }
    Print ("no order found");
    return (true);
}// End "NoOpenOrdersExist()"

LOL this is more readable

Strange way we looking mostly if OpenOrdersExist but you can use this also

 

@RaptorUK and MisterDog, the return's value is MisterDog's discretion, I just correct the OrderSelect() thing. Both of MisterDog's print() output is confusing to me :D

@DeVries, the OrderSelect() should return something ;)

:D

if (Is_There_Already_Opened_Position() == false)
   {
   OrderSend (......
     
   }

//------
bool Is_There_Already_Opened_Position() //True when there is open order, False when thre is no open order
   {
   int total = OrdersTotal();
   for(int i = 0; i < total; i++)
      {
      if (OrderSelect(i,SELECT_BY_POS,MODE_TRADES) == true
          && OrderMagicNumber() == Magic 
          && OrderSymbol() == Symbol())
         {
          Print ("There is already opened trades");
          return (true);
         }
      }
   Print ("There is NO opened trades");
   return (False);
   }
 
onewithzachy:

@RaptorUK and MisterDog, the return's value is MisterDog's discretion, I just correct the OrderSelect() thing. Both of MisterDog's print() output is confusing to me :D

@DeVries, the OrderSelect() should return something ;)

:D


According to the title of this topic then bool NoOpenOrdersExist() is not what you expect for having an "Boolean Function that checks for an open order"

I think that was also what you like to point out....

 

The OPs function will also return false if there are Pending Orders . . . perhaps the function would be clearer if it was called ThereAreOpenOrders . . . and return true when there are Buy or Sell orders

 
onewithzachy:

@RaptorUK and MisterDog, the return's value is MisterDog's discretion, I just correct the OrderSelect() thing. Both of MisterDog's print() output is confusing to me :D

@DeVries, the OrderSelect() should return something ;)

:D


Yes, I agree with you. Things even get worse when you start thinking through the logic of, " ! no open orders exist == false ". ;-)
 

So here's what I'm taking away from this exercise:

Writing code in double negative is not incorrect (little joke) it just makes things harder to understand.

The rewrites done by deVries and onewithzachy remove the double negative.

And RaptorUK points out the benefits of paying attention to return values in a great post listed above. And he points out that the function may be doing a little more than I expected as it also checks for Pending Orders.

 
  1. A more general, more usable function would be
    int GetOpenOrderCount(int op=-1){ #define OP_ALL -1
        int nOpen = 0;  string symbol = Symbol();
        for(int iPos = OrdersTotal() - 1; iPos >= 0; iPos--) if(
            OrderSelect(iPos,SELECT_BY_POS,MODE_TRADES)
        &&  OrderMagicNumber() == Magic 
        &&  OrderSymbol()      == symbol
        && (op == OP_ALL || op == OrderType())
        )   nOpen++;
        return(nOpen);
    }
    Makes the test explicit: ==0, ==1, or >0 (many) Avoids double negative logic. Allows specific order types.

  2. Always test return codes
  3. Always count down. Doesn't matter in this case, but its better to get in to the habit.
  4. You wouldn't ever say if( (2+2 == 4) == true) would you? The ==true is redundant. So is the if(OrderSelect(...) == true)