OrderClose error 4051 invalid ticket for OrderClose function

New comment
 
Im facing this error when I try to retrieve the oldest ticket in my current pool of trades using the functions below,


int EarliestOpenSellTicket()
{
    datetime EarliestSellTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestSellTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_BUY)continue;                //go to next iteration of loop if it's not a sell.
      //if (OrderSymbol()!= Symbol())continue;             //go to next iteration if it's not the correct pair.   
      if (OrderMagicNumber()!=333 || OrderMagicNumber()!=334)continue;           //go to next iteration if magic number is not the correct one.
      if (OrderOpenTime()>EarliestSellTime)continue;     //if it's a newer order go to next iteration. It's not going to be newer than the current server time.       
                                                       //if it makes it past all of these filters save the time and the ticket number.
       EarliestSellTime = OrderOpenTime();
       EarliestSellTicket = OrderTicket();
       Print(EarliestSellTicket);
    }
    //after the loop completes it's iterations thru all the open trades, return the ticket number of the order with the oldest time.
    return(EarliestSellTicket);
}

int EarliestOpenBuyTicket()
{
    datetime EarliestBuyTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestBuyTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_SELL)continue;                //go to next iteration of loop if it's not a sell.
      //if (OrderSymbol()!= Symbol())continue;             //go to next iteration if it's not the correct pair.   
      if (OrderMagicNumber()!=333 || OrderMagicNumber()!=334)continue;           //go to next iteration if magic number is not the correct one.
      if (OrderOpenTime()>EarliestBuyTime)continue;     //if it's a newer order go to next iteration. It's not going to be newer than the current server time.       
                                                       //if it makes it past all of these filters save the time and the ticket number.
       EarliestBuyTime = OrderOpenTime();
       EarliestBuyTicket = OrderTicket();

    }
    //after the loop completes it's iterations thru all the open trades, return the ticket number of the order with the oldest time.
    return(EarliestBuyTicket);
}

void latetradecloserbuy(){

   int buy_trades_number = _get_number_of_trades(OP_BUY);
   int BuyTicket = EarliestOpenBuyTicket();
   
   if(buy_trades_number==12){
   
           OrderClose(BuyTicket,OrderLots(),Bid,2,clrNONE);
           }
   

}   

void latetradeclosersell(){

   int sell_trades_number = _get_number_of_trades(OP_SELL);
   int SellTicket = EarliestOpenSellTicket();
   
   if(sell_trades_number==12){
   
           OrderClose(SellTicket,OrderLots(),Ask,2,clrNONE);
           }
   

}  

What I am trying to do here is identify the oldest buy or sell trade in my current open trades and if the total number of buy or sell trades equals 12, I start closing the respective type.

For example if I have 12 open buy trades, I close just the oldest one in the pool.


As you can see, I get an invalid ticket when trying to close the trades. This means that my first two functions that find the earliest ticket do not work. I cant see the bug, can you guys take a stab at this?
 

There are some things looking not right. I'll point it out for the first function. 

int EarliestOpenSellTicket()
{
    datetime EarliestSellTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestSellTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_SELL)continue;                //go to next iteration of loop if it's not a sell.
      //if (OrderSymbol()!= Symbol())continue;             //go to next iteration if it's not the correct pair.   
      if (OrderMagicNumber()!=333 && OrderMagicNumber()!=334)continue;           //go to next iteration if magic number is not the correct one.
      if (OrderOpenTime()>EarliestSellTime)continue;     //if it's a newer order go to next iteration. It's not going to be newer than the current server time.       
                                                       //if it makes it past all of these filters save the time and the ticket number.
       EarliestSellTime = OrderOpenTime();
       EarliestSellTicket = OrderTicket();
       Print(EarliestSellTicket);
    }
    //after the loop completes it's iterations thru all the open trades, return the ticket number of the order with the oldest time.
    return(EarliestSellTicket);
}

You'll have to check further about more spooky things, but this should give it a go.

 

Please do not start a new topic when you are basically working with the same code as in an earlier topic. I have deleted your last topic as you had no replies.

You should go through your code very carefully and check that the logic makes sense. Just slapping some code together and asking others to sort out your mistakes means that you will not learn.

A common error that coders make is copying and pasting code for a buy to use for a sell and then not carefully editing it to work properly for a sell.

int EarliestOpenSellTicket()
{
    datetime EarliestSellTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestSellTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_BUY)continue;                //go to next iteration of loop if it's not a sell.
int EarliestOpenBuyTicket()
{
    datetime EarliestBuyTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestBuyTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_SELL)continue;                //go to next iteration of loop if it's not a sell.

----------------------------------------------------------------------------------------------------------------------------------------------------------------------------

void latetradecloserbuy(){

   int buy_trades_number = _get_number_of_trades(OP_BUY);
   int BuyTicket = EarliestOpenBuyTicket();
   
   if(buy_trades_number==12){
   
           OrderClose(BuyTicket,OrderLots(),Bid,2,clrNONE);
           }

What if BuyTicket equals -1 ?

If you checked the values when the order close fails you will see when it does = -1 and then that will give you an idea of where to check.


int EarliestOpenBuyTicket()
{
    datetime EarliestBuyTime  = TimeCurrent();        //initialize it to the current server time. You will have no orders newer than that.
    int      EarliestBuyTicket = -1;                  // None open.
    for(int pos = OrdersTotal()-1; pos >= 0 ; pos--)
    {
      if (!OrderSelect(pos, SELECT_BY_POS))return (-1);  //If it can't select an order then bail out. Of course you could print something if you like.
      if (OrderType()!= OP_SELL)continue;                //go to next iteration of loop if it's not a sell.
      //if (OrderSymbol()!= Symbol())continue;             //go to next iteration if it's not the correct pair.   
      if (OrderMagicNumber()!=333 || OrderMagicNumber()!=334)continue;           //go to next iteration if magic number is not the correct one.
      if (OrderOpenTime()>EarliestBuyTime)continue;     //if it's a newer order go to next iteration. It's not going to be newer than the current server time.       
                                                       //if it makes it past all of these filters save the time and the ticket number.
       EarliestBuyTime = OrderOpenTime();
       EarliestBuyTicket = OrderTicket();

    }
    //after the loop completes it's iterations thru all the open trades, return the ticket number of the order with the oldest time.
    return(EarliestBuyTicket);
}

As already touched on, why are you continuing when the trade is not a SELL? You should be continuing if it is not a BUY.

Your check for the order magic number will always be true. Think about it. The magic number could be 333, but as it does not = 334, it will be true

 
lippmaje:

There are some things looking not right. I'll point it out for the first function.

You'll have to check further about more spooky things, but this should give it a go.

Ahhhh there is my mistake. I had the or operand when I needed the and. Wow I was pulling my hair over this, thank you!
New comment