Unable to understand this piece of code

[Qæs]

while(true)
 {
  RefreshRates();
  ticket=OrderSend(...);
  if(ticket>0)                         // this can be true only if ordersend is successful?
  {
   Print("order placed successfully"); // a message is printed then
   break;                              // and loop breaks
   err=GetLastError();                 // why this...
   if(err==0) break;                   // and this...
   if(err==4                           // and...
   || err==136                         // this
   || err==137                         // is
   || err==146) Sleep(5000);           // here now?
  }
  else
  {
   Print("order failed, Error = ", err);
   break;
  }
 }

[Seng Joo Thio]

thought re-arranging this way looks more correct (i did not add or remove anything... I just reshuffled the lines):

while(true)
 {
  RefreshRates();
  ticket=OrderSend(...);
  if(ticket>0)                         // this can be true only if ordersend is successful?
  {
   Print("order placed successfully"); // a message is printed then
   break;                              // and loop breaks
  }
  else
  {
   err=GetLastError();                 // why this...
   if(err==0) break;                   // and this...
   Print("order failed, Error = ", err);
   if(err==4                           // and...
   || err==136                         // this
   || err==137                         // is
   || err==146) Sleep(5000);           // here now?
   break;
  }
 }

[Qæs]

Seng Joo Thio:

thought re-arranging this way looks more correct (i did not add or remove anything... I just reshuffled the lines):

Yes, this is the right way but the question is, does that code make sense anyway?

(Maybe in some cases (by some broker) ticket gets a value higher than zero and err also gets one of the mentioned values. is it possible?)

[lippmaje]

Looks like the original code was mangled by a newbie coder. The original idea was this:

If the order failed to open, check the return code. In some cases it is advised to wait some time and try again. Such cases would be 'trade server busy' (err=4), 'trade timeout' (128), 'off quotes' (136) etc.

So the function would have a chance by just waiting some time, refreshing quotes and trying again.

Other cases are so obviously bad that a retry wouldn't make sense, think of errors such as 'invalid stops' (130).

[Seng Joo Thio]

Qæs:

Yes, this is the right way but the question is, does that code make sense anyway?

(Maybe in some cases (by some broker) ticket gets a value higher than zero and err also gets one of the mentioned values. is it possible?)

I wouldn't code this way. The while loop is redundant because whether the OrderSend attempt succeeded or not, a break is encountered. A better way would be, as @lippmaje pointed out - for the loop to be able to repeat (after some waiting time) itself if the error was due to out-dated price data. 

As to whether it is possible for OrderSend() to return a value > zero WHILE giving an err, I doubt it. But there is no harm, as a programming safeguard, to do a GetLastError() right after OrderSend(), and check that it is indeed zero when ticket is more than zero, or otherwise.