Is this goto expressive?
Replace the goto with a do-while, or simply a while loop if you don't want the "always run once" functionality you have right now.
var queue = new Queue<TraceItem>(this.batch);
while (this.connected)
{
byte[] buffer = null;
try
{
socket.Recv(out buffer);
}
catch
{
// ignore the exception we get when the socket is shut down from another thread
// the connected flag will be set to false and we'll break the loop
}
do {
if (buffer != null)
{
try
{
var item = TraceItemSerializer.FromBytes(buffer);
if (item != null)
{
queue.Enqueue(item);
buffer = null;
}
}
catch (Exception ex)
{
this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
this.tracer.TraceException(TraceEventType.Error, 0, ex);
}
}
} while(queue.Count < this.batch && socket.Recv(out buffer, ZMQ.NOBLOCK))
// queue processing code
}
It's so amazingly easy to rid yourself of GOTO in this situation it makes me cry:
var queue = new Queue<TraceItem>(this.batch);
while (this.connected)
{
byte[] buffer = null;
try
{
socket.Recv(out buffer);
}
catch
{
// ignore the exception we get when the socket is shut down from another thread
// the connected flag will be set to false and we'll break the loop
}
bool hasAnotherMessage = true
while(hasAnotherMessage)
{
hasAnotherMessage = false;
if (buffer != null)
{
try
{
var item = TraceItemSerializer.FromBytes(buffer);
if (item != null)
{
queue.Enqueue(item);
buffer = null;
if (queue.Count < this.batch && socket.Recv(out buffer, ZMQ.NOBLOCK))
{
hasAnotherMessage = true;
}
}
}
catch (Exception ex)
{
this.ReceiverPerformanceCounter.IncrementDiagnosticExceptions();
this.tracer.TraceException(TraceEventType.Error, 0, ex);
}
}
}
// queue processing code
}
Pretty much sums up my thoughts on "goto."
Goto is bad programming practice for many reasons. Chief among them is that there is almost never a reason for it. Someone posted a do..while
loop, use that. Use a boolean
to check if you should continue. Use a while loop. Goto's are for interpreted languages and a call back to assembler days (JMP
anyone?). You're using a high level language for a reason. So that you and everyone else doesn't look at your code and get lost.
To keep this answer somewhat current I'd like to point out that a combination of goto
and bracing errors caused a major SSL bug in iOS and OS X.