"for (bool flag = true; flag; flag = false) { ... }" - is this normal?

Question

I've never seen a for loop initialized this way and don't understand why it would be written this way?

I'm doing some research into connecting to an IMAP server in .NET and started looking at code from a library named ImapX. I found the for loop in a method that writes data to a NetworkStream and then appears to read the response within the funky for loop. I don't want to copy and paste someone else's code verbatim, but here's the gist:

public bool SendData(string data)
{
  try
  {
    this.imapStreamWriter.Write(data);

    for (bool flag = true; flag; flag = false)
    {
      var s = this.imapStreamReader.ReadLine();
    }
  }
  catch (Exception)
  {
    return false;
  }

  return true;
}

Again, this isn't the exact code, but it's the general idea. That's all the method does, it doesn't use the server response, it just returns true if no exception was thrown. I just don't understand how or why the for loop is being used this way; can anyone explain what advantages initializing this offers, if any?

It's pointless, that's for sure - this only executes once before being set to `false`. If it was intended to pause on `ReadLine()`, you get that for free... — Yuck, Feb 15 '12 at 18:03
I'm guessing you got the gist of the code wrong. A loop written like that will only execute a single time. — Justin Niessner, Feb 15 '12 at 18:04
Are you *sure* you coped the for line correctly, and `flag` isn't being adjusted in the `for` body? The way it's written right now it'll execute once and stop (start at true - continue while true - change to false on first reloop - stop now !true) — Rudu, Feb 15 '12 at 18:05
It would make a lot more sense if the loop increment and conditional parts are flipped.. Is it just a typo? — Mark Smith, Feb 15 '12 at 18:06
I suspect that either as @Rudu suggests, `flag` is written to in the body of the for, or else it *used to be* but someone edited that out, or else the code was modified from an example where `flag` was used, but the programmer didn't entirely understand it, and so neglected to remove the loop when removing the inner use of `flag`. — phoog, Feb 15 '12 at 18:10
I once knew someone who claimed (and I have no reason to disbelieve him) that by experimentation he had determined that the JIT is much more willing to inline method calls inside a loop body, because it thinks it's likely to get more benefit from it. This might be a dubious attempt to trick the JIT into compiling "faster" code, although I certainly wouldn't recommend it. — Weeble, Feb 15 '12 at 18:13
I assume that it's some kind of reminder that they want to call `SendData` later again in case of an exception. But a simple [TODO-Comment](http://www.dotnetperls.com/todo) would have been better then. — Tim Schmelter, Feb 15 '12 at 18:16
@JustinNiessner I got the gist of the code right which is why I don't understand the need for the `for` loop at all. You're right, it technically isn't even a loop, just a very creative way of executing a line of code once. — sellmeadog, Feb 15 '12 at 18:16
@Rudu unless Reflector is hiding code from me, the only part of the "loop" body I left out was a DEBUG test that writes the string to the console, nothing more. — sellmeadog, Feb 15 '12 at 18:17
@MarkSmith nope, no typo; I did copy and paste the `for` loop initialization from the code — sellmeadog, Feb 15 '12 at 18:19

score 5 · Answer 1 · answered Feb 15 '12 at 18:05

It's a horrible way of executing a loop once. If you change the flag initalizer to something which isn't always true, it may have slightly more sense, but not a lot. I've entirely-unseriously suggested this code before now:

Animal animal = ...;
for (Dog dog = animal as Dog; dog != null; dog = null)
{
    // Use dog...
}

... as a way of using an as operator without "polluting" the outer scope. But it's language silliness, and not something I'd ever really use.

score 2 · Accepted Answer · answered Feb 16 '12 at 09:27

I had previously assumed you were looking at original source code, but from your comments it sounds like you're looking at Reflector's reconstituted C# based on the MSIL. It's important to understand that this does not necessarily bear a close resemblance to the original code as written.

At the MSIL level, there is no such thing as a for loop or a while loop. There are just conditional branch instructions. (See here: http://weblogs.asp.net/kennykerr/archive/2004/09/23/introduction-to-msil-part-6-common-language-constructs.aspx ) Any tool trying to reconstitute C# must make a number of guesses for how the code was originally structured. It seems probable that this wasn't originally written as a for loop, but that Reflector detected that the IL could have been generated by a for loop, and its heuristics made a bad guess that it should be a for loop and not something else.

If I look at the same code in ILSpy, it is rendered as a while loop. It's still redundant, but it looks a lot less weird. Furthermore, it's possible that the original code actually did something that has been optimized out, perhaps calls to [Conditional] methods or code marked with #if directives. Then again, perhaps the original code used to do something else but parts were commented out - the comments are not kept in the IL. Or maybe there was more code there in the past and it was just plain deleted.

In short, there are many ways what you see in Reflector can be very different from what was originally written. You should consider it a prettier presentation of IL, not an example of C# as it is written by humans.

Thank you for your answer. I suspected as much, that what Reflector is showing me may not be what was written by the original programmer. I really wanted to know what reason, if any, someone would choose to write a loop that way and the general consensus (and my own intuition) from all other answers is that there is no reason to. Again, thank you for your detailed answer, it makes a lot of sense. — sellmeadog, Feb 16 '12 at 15:15

score 1 · Answer 3 · answered Feb 15 '12 at 18:05

I had this as a comment, but I suppose it's the answer as well.

The "loop" is pointless. It initializes flag to true and is declared to execute only while the flag is still true. However, flag is explicitly being set to false after the first iteration. So in this case it's guaranteed to run once and only once.

I suspect the author was trying to be sure that control flow would pause on ReadLine() - but it does that anyway until user input is received.

score 0 · Answer 4 · answered Feb 15 '12 at 18:32

The for-loop is pretty much a no-op, as noted by others.

It does do one thing, though, that may or may not matter: it introduces a namespace scope. The scope of the variable s is the body of the for-loop. s goes out of scope once the for-loop is exited. You could get the same effect by simply creating a block, thus:

{
  var s = this.imapStreamReader.ReadLine();
}

Which is still pretty silly.

No idea what the original author was trying to accomplish with this -- perhaps trying to ensure that his s was destroyed/garbage-collected/disposed -- not that this technique would work (it wouldn't).

"for (bool flag = true; flag; flag = false) { ... }" - is this normal?

4 Answers4