Any reason why this doesn't work?

 

Here's the situation. I have a Buy Order Opened. Once conditions are met, I want to close the Buy and open a Sell. Everything works up to the close of the Buy, but the Sell doesn't open. Infact, the Alert shows that the Sell is opened with the same ticket# as the Buy and GetLastError() returns 0. The log file (server) shows the open of the Buy Order and the close of the Buy Order buy no open of the Sell. I've tried a Sleep command to try and create a pause between the close of the Buy and the Open of the Sell, but makes no difference.

TakePS = (Ask-45*Point);

Double Ticket = OrderTicket();

//------------------------------------ CLOSE BUY --------------------------


{
{
Alert("Closing Buy Ticket#",Ticket);

OrderSelect(0,SELECT_BY_POS, MODE_TRADES);

{
while(Order == false)
{RefreshRates();
Order = OrderClose(Ticket,Lot,Bid,2);}
}

Alert ("Closed - Buy Order # ",Ticket);

Alert (GetLastError());

}

Sleep(3000);


//------------------------------------ OPEN SELL --------------------------


{

Alert("Opening Sell.... Please Wait...");

RefreshRates();

{
while(Order == false)
{RefreshRates();
Order = OrderSend(Symbol(),OP_SELL,Lot,Bid,2,0,TakePS);}
}

Alert ("Opened - Sell Order ",Ticket);

Alert (GetLastError());

}
}

 

I'll comment the changes and color them in red.

TakePS = (Ask-45*Point);

//First of all, this is no good. For OrderTicket() to work, you need to have selected one with OrderSelect prior to using OrderTicket.

//Double Ticket = OrderTicket();

int Ticket;

//------------------------------------ CLOSE BUY --------------------------
//This is all wrong.. You need to get OrdersTotal and loop all of them to get specific tickets and do whatever you want to them.

//You will need to rewrite this whole function
{
{
Alert("Closing Buy Ticket#",Ticket);

OrderSelect(0,SELECT_BY_POS, MODE_TRADES);

{
while(Order == false)
{RefreshRates();
Order = OrderClose(Ticket,Lot,Bid,2);}
}

Alert ("Closed - Buy Order # ",Ticket);

Alert (GetLastError());

}

Sleep(3000);


//------------------------------------ OPEN SELL --------------------------


{

Alert("Opening Sell.... Please Wait...");

RefreshRates();

{

//OrderSend returns an int. -1 if an error occurred otherwise it's the ticket number.
//while(Order == false)

Order = 0;

while(Order == -1)

{RefreshRates();
Order = OrderSend(Symbol(),OP_SELL,Lot,Bid,2,0,TakePS);}
}

Alert ("Opened - Sell Order ",Ticket);

//Don't need to getlasterror unless an error occurred -- you will know this is Order is still -1.

if(Order == -1){
Alert (GetLastError());

}

}
}

.

I did this really fast to give you an idea so feel free to post back after you made some changes. There are much better ways to code what you were doing so once we got rid of all the errors, I'll show you a few ways of doing your loops and stuff.

.

Jon

 

//------------------------------------ CLOSE BUY --------------------------
//This is all wrong.. You need to get OrdersTotal and loop all of them to get specific tickets and do whatever you want to them.

//You will need to rewrite this whole function
{
{
Alert("Closing Buy Ticket#",Ticket);

OrderSelect(0,SELECT_BY_POS, MODE_TRADES);

{
while(Order == false)
{RefreshRates();
Order = OrderClose(Ticket,Lot,Bid,2);}
}

Alert ("Closed - Buy Order # ",Ticket);

Alert (GetLastError());

}

Sleep(3000);

I only have one order open at a time. Only allow one order a time. What will the loop accomplish? I simply want to close what is open and open another trade of the opposite direction. ( Without Hedge Error)

int Ticket is asigned to the value of OrderTicket() at beginning of EA, because Ticket is common to several parts of EA and because I can't assign values to OrderTicket(). Such as 0 to reset loop.

Thanks!

 
Yellowbeard:

I only have one order open at a time. Only allow one order a time. What will the loop accomplish? I simply want to close what is open and open another trade of the opposite direction. ( Without Hedge Error)

int Ticket is asigned to the value of OrderTicket() at beginning of EA, because Ticket is common to several parts of EA and because I can't assign values to OrderTicket(). Such as 0 to reset loop.

Thanks!


Let's imagine a programmer who intends that his EA only ever has one open order at a time.

This programmer (let's call him Captain Pugwash, just to pick a random name) likes to avoid code which he deems to be unnecessary.

One of the things he scrimps upon is the error handling around his open and close functions.

So he may end up with an order which he thought was closed but isn't.

He then opens another order (thinking its his first) and its his second.

In order to revert to the status quo of having just one open order, it would be useful to have a loop which closes all open orders (this can be done using a decrementing index SELECT_BY_POS rather than by ticket SELECT_BY_TICKET, where 0 is the most recent, 1, 2 etc..).

It would also be useful to have error handling to attempt to prevent the situation in the first place.

There are grog tokens at stake, so Pugwash should use all the tools available to him.


CB

 
cloudbreaker:

This programmer (let's call him Captain Pugwash, just to pick a random name) likes to avoid code which he deems to be unnecessary.

One of the things he scrimps upon is the error handling around his open and close functions.

So he may end up with an order which he thought was closed but isn't.

Well, yes, but I suspect that a majority of the code posted on this forum works on a basis such as the following very-pseudo-code. Or, perhaps, is intended to work on a basis such as the following:


if (at least one open order)
   for (each open order)
	if (order meets closure conditions)
		close order

else
   if (signals okay for trade entry)
	open new position

In other words, there's no error handling in the sense of looped retries if OrderSend or OrderClose fails (though there obviously ought to be some logging). Instead, the code retries operations on the next tick - assuming that the requisite conditions are still met. You can argue about things like this kind of code missing trades if the signals only briefly come into alignment and OrderSend() then fails, but it's reasonably robust.

 
jjc wrote >>

Well, yes, but I suspect that a majority of the code posted on this forum works on a basis such as the following very-pseudo-code. Or, perhaps, is intended to work on a basis such as the following:

In other words, there's no error handling in the sense of looped retries if OrderSend or OrderClose fails (though there obviously ought to be some logging). Instead, the code retries operations on the next tick - assuming that the requisite conditions are still met. You can argue about things like this kind of code missing trades if the signals only briefly come into alignment and OrderSend() then fails, but it's reasonably robust.

I have been trying to solve this one problem by stripping my EA down to just the part where the problem lies. Here are some examples of how this code is functioning:

09:41:51 '36498': instant order sell 1.00 EURUSD at 1.41236 sl: 0.00000 tp: 1.41209
09:41:52 '36498': request was accepted by server
09:41:52 '36498': request in process

09:41:55 '36498': order was opened : #8882389 sell 1.00 EURUSD at 1.41236 sl: 0.00000 tp: 1.41209


09:43:16 '36498': close order #8882389 sell 1.00 EURUSD at 1.41236 sl: 0.00000 tp: 1.41209 at price 1.41232
09:43:16 '36498': request was accepted by server
09:43:16 '36498': request in process

09:43:20 '36498': order #8882389 sell 1.00 EURUSD at 1.41236 sl: 0.00000 tp: 1.41209 closed at price 1.41232

Here, the SELL is opened and then it's closed ( No Problems ), but the BUY did not open.

12:27:58 '36498': instant order buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601
12:27:58 '36498': request was accepted by server
12:27:59 '36498': request in process

12:28:01 '36498': order was opened : #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601


12:30:24 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41576
12:30:24 '36498': request was accepted by server

12:30:24 '36498': requote 1.41570 / 1.41588 for order #8883883 buy 1.00 EURUSD closing at 1.41576

12:30:24 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41576
12:30:26 '36498': request was accepted by server

12:30:27 '36498': requote 1.41570 / 1.41588 for order #8883883 buy 1.00 EURUSD closing at 1.41576

12:30:27 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41576
12:30:27 '36498': request was accepted by server

12:30:27 '36498': requote 1.41570 / 1.41588 for order #8883883 buy 1.00 EURUSD closing at 1.41576

12:30:27 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41576
12:30:28 '36498': request was accepted by server

12:30:28 '36498': requote 1.41570 / 1.41588 for order #8883883 buy 1.00 EURUSD closing at 1.41576

12:30:28 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41576
12:30:30 '36498': request was accepted by server
12:30:30 '36498': request in process

12:30:33 '36498': requote 1.41569 / 1.41587 for order #8883883 buy 1.00 EURUSD closing at 1.41576

12:30:33 '36498': close order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 at price 1.41569
12:30:33 '36498': request was accepted by server
12:30:33 '36498': request in process

12:30:36 '36498': order #8883883 buy 1.00 EURUSD at 1.41574 sl: 0.00000 tp: 1.41601 closed at price 1.41569

Here, the BUY is opened. After several requotes, it's closed, but the SELL does not open.

I can't seem to get past opening and closing one order. I'm not sure if this is an issue with the server and it's required time to open and close orders or a looping issue on my end. I'm not getting any errors.

Also does anyone have any ideas or startegy for dealing with requotes so that a profit doesn't become a loss?

Thanks Everyone!

I appreciate all the effort and help!

 

Well, your ticket variable serves no purpose since it's given a OrderTicket() value that hadn't been selected with OrderSelect() first, yet you're using it everywhere. That's the first problem. Second, you are assuming that things work without ever changing the behaviour if errors occur -- that is baaaaaddd hehe. Third, it's not about what will the loop accomplish.. it's about what will not having a loop NOT accomplish. You simply need that loop to get the right tickets to work on. Without that, you are "assuming" that your ticket is 0 and working on that. Never assume anything in programming. Use functions designed to get your variable value, check to make sure no errors have happened when fetching that value, then use that value and make sure yet again that no errors occurred when using that value. Go read up on OrderSelect() and everything that surrounds it, you will need it for your close_buy() function.

.

Jon

 

Try increasing your slippage slightly.


Use this code to close the open trade before opening a new trade. I find that for locating and selecting trades using the magic is far more reliable than storing the ticket number.

//close short

for(j = 0; j < OrdersTotal(); j++)
{
if(OrderSelect(j,SELECT_BY_POS,MODE_TRADES))
{
if(OrderType() == OP_SELL && OrderSymbol() == Symbol() && OrderMagicNumber() == magic)
{
OrderClose(OrderTicket(),OrderLots(),Ask,slip,Green);
}
}
}

//now buy

...

...


//close long

for(j = 0; j < OrdersTotal(); j++)
{
if(OrderSelect(j,SELECT_BY_POS,MODE_TRADES))
{
if(OrderType() == OP_BUY && OrderSymbol() == Symbol() && OrderMagicNumber() == magic)
{
OrderClose(OrderTicket(),OrderLots(),Bid,slip,Green);
}
}
}

//now sell

....

 
fxcourt:

for(j = 0; j < OrdersTotal(); j++)

[...]

OrderClose(OrderTicket(),OrderLots(),Ask,slip,Green);

Cloudbreaker is not going to be at all happy with you. (Have a think what happens if there are two matching buy or sell orders and you try to close both of them using the above code.)

 
jjc wrote >>

Cloudbreaker is not going to be at all happy with you. (Have a think what happens if there are two matching buy or sell orders and you try to close both of them using the above code.)

He is checking the OrderType(). There's nothing wrong with fxcourt's code other than calling OrdersTotal() at every loop iteration.. performance would be increased by calling it once, putting it in a variable and using that variable in the "for" loop.

.

Jon

 
Archael:

There's nothing wrong with fxcourt's code other than calling OrdersTotal() at every loop iteration.. performance would be increased by calling it once, putting it in a variable and using that variable in the "for" loop.

Channelling CB... if you have two orders, A and B, then they start off in slots 0 and 1. Loop variable j starts at 0. Order A in slot 0 is selected and closed, and moves to the trade history. Order B thus moves from slot 1 to slot 0 on the open-order list. But j is incremented from zero to one, and therefore B is left open. That's why CB keeps, er, shouting at people to decrement rather than increment.


Even if you know that you're only going to be dealing with one matching order, then CB still has a point. You're leaving yourself a nasty bug to emerge from the woodwork when you change your order-entry code in a few weeks or months time.

Reason: