Channels ▼

Bil Lewis

Dr. Dobb's Bloggers

*Four* Loops and the One I'm For

February 15, 2009

Here are three loops that all do the same thing. (Assuming I haven't
made a silly mistake.:-) A clever compiler might even compile them
down to identical instructions. So which version do you like best and
why?

I'll tell you my opinions next week. I'd like to hear yours this week.
 
 public static final String readStringDO(Reader reader) throws IOException {
    char buf[] = new char[4096];
    StringBuffer result = new StringBuffer();
    int size = 0;

    do {
       size = reader.read(buf);
       if (size != -1)
          result.append(buf, 0, size);
       } while (size != -1);

     return result.toString();
 }


 
 public static final String readStringWHILE(Reader reader) throws IOException {
    char buf[] = new char[4096];
    StringBuffer result = new StringBuffer();

    while (true) {
          int size = reader.read(buf);
          if (size == -1)
              return result.toString(); 
          result.append(buf, 0, size);
       }      
 }


 
 public static final String readStringFOR(Reader reader) throws IOException {
    char buf[] = new char[4096];
    StringBuffer result = new StringBuffer();

    for (int size = reader.read(buf); size != -1; size = reader.read(buf)) {
          result.append(buf, 0, size);
       }      

    return result.toString(); 
 }



// Written by Andrew Koenig, who didn't like any of the above
 public static final String readString(Reader reader) throws IOException {
    char buf[] = new char[4096];
    StringBuffer result = new StringBuffer();
    int size; 

    while ((size = reader.read(buf)) != -1) { 
        result.append(buf, 0, size); 
    } 

    return result.toString();
}


So... what's important for you in source code?

For me it's simple: Is it easy to read the code?

That is more important than execution speed or the number of lines or
the exact naming conventions used or anything else.

So what makes code easy to read? A combination of the number of lines
and naming conventions used, etc. :-) There are further generalities that
are more directly useful and for this example, there are two of
particular interest. 

Generality #1: Are variables appropriately scoped? Are they still
alive after their last intended use? And (god forbid!) do they get
reused for another purpose?

Generality #2: Tests should not change things. You should be able to
ask the same question multiple times and get the same answer, until
somebody changes something.

Generality #3: Don't duplicate stuff. Do you access a resource for the
same reason in two different places? Do you repeat tests?

====


The DO loop is really ugly, IMHO. It declares size to be a live
variable over the entire method, even though it's only used in the
loop. (It's pretty trivial here, but this would be a bigger deal if
the method outside the loop were larger.) So I don't like that.

It also repeats the exit test. That's not horrible, but it does make
the code a little bit harder to read. And I find reading the test at
the bottom of the loop awkward.

I suppose I could have eliminated the second test like this:

   do {
       result.append(buf, 0, size);
       size = reader.read(buf);
       } while (size != -1);

because we know size is 0 coming in, but I find that very non-evident
and I'd be constantly rechecking that initialization every time I
touched the code.

So... It violates #1 & #3.

====

The FOR loop localizes size, has only one exit test, but has two calls
to read(). The calls are obviously identical, but it takes more brain
power to recognize that they are equivalent to the other loops. Call
me lazy, but I don't want to have to think about "obvious" things.

And there's cognitive dissonance in the FOR line itself. It's
ostensively setting SIZE to some value, but it's mainly reading data
into a buffer. It just feels wrong. I think Abdul Habra was expressing
a similar sense of unease.

It violates #3 and adds discomfort.
====

Now Andrew's example is exactly what I wanted to include. It is common
in C and C++ code and I've been so focused on Java that I just spaced
it. It is clearly compact. We like short code.

It has the poorly scoped variable, size. Don't like that. It also has
a test that changes state. And that I *really* don't like. Why?

Here's our test from the while loop:

 ((size = reader.read(buf)) != -1)

Obviously, I can take it apart in my head and see what it's
doing. It's so common that's it's almost an idiom. But it's not an
idiom, because there are so many minor variations on it. (How many
versions of this can you think of?)  You cannot just glance at it, you
have to pause and think about it. And that's my real complaint.

Violates #1 & #2.

====

The WHILE loop keeps size localized to the loop. I like that. It only
tests once and it's super obvious what happens when it
terminates. It's longer than Andrew's, 342 characters vs. 295. It
separates the filling of the buffer from the testing of the result.

It also puts the result next to the test that produces it. All the
other loops have a exit-loop test and then return a value at the
end. This example puts them together.

There's just less to think about.

====

Now imagine that these are much bigger methods with more complicated
exit tests. How does that change your perception?

Let's say the test becomes:

((size == -1) || (size > 2 && (buf[0] == 'F' && buf[1] == 'i' &&
buf[2] == 'n'))

How would that look in those loops?

====

Here's how I'd rewrite the WHILE loop:

 public static final String readReaderWHILE(Reader reader) throws IOException {
    char buf[] = new char[4096];
    StringBuffer result = new StringBuffer();

    while (true) {
          int size = reader.read(buf);
          if (size == -1)
             break;
          if (size > 2) {
             if (buf[0] == 'F' && buf[1] == 'i' && buf[2] == 'n')
                 break;
          }
          result.append(buf, 0, size);
       }

    return result.toString();
 }

How does that look? Is it easier to understand than if I just left in
the long version of the test instead of breaking it out like I did?
What do you think of this:

   if ((size == -1) || ((size > 2) && ((buf[0] == 'F' && buf[1] == 'i' && buf[2] == 'n'))))
                 break;

I find it vastly more complex than the while example.

====

As for the other loops...

Would you stick the new logic into the loop test? Would you add an IF
statement like above? And have two different places you're testing to
exit the loop? I don't see any way to do it well.

The WHILE loop gets my vote.

What would you do?

-Bil

Related Reading


More Insights






Currently we allow the following HTML tags in comments:

Single tags

These tags can be used alone and don't need an ending tag.

<br> Defines a single line break

<hr> Defines a horizontal line

Matching tags

These require an ending tag - e.g. <i>italic text</i>

<a> Defines an anchor

<b> Defines bold text

<big> Defines big text

<blockquote> Defines a long quotation

<caption> Defines a table caption

<cite> Defines a citation

<code> Defines computer code text

<em> Defines emphasized text

<fieldset> Defines a border around elements in a form

<h1> This is heading 1

<h2> This is heading 2

<h3> This is heading 3

<h4> This is heading 4

<h5> This is heading 5

<h6> This is heading 6

<i> Defines italic text

<p> Defines a paragraph

<pre> Defines preformatted text

<q> Defines a short quotation

<samp> Defines sample computer code text

<small> Defines small text

<span> Defines a section in a document

<s> Defines strikethrough text

<strike> Defines strikethrough text

<strong> Defines strong text

<sub> Defines subscripted text

<sup> Defines superscripted text

<u> Defines underlined text

Dr. Dobb's encourages readers to engage in spirited, healthy debate, including taking us to task. However, Dr. Dobb's moderates all comments posted to our site, and reserves the right to modify or remove any content that it determines to be derogatory, offensive, inflammatory, vulgar, irrelevant/off-topic, racist or obvious marketing or spam. Dr. Dobb's further reserves the right to disable the profile of any commenter participating in said activities.

 
Disqus Tips To upload an avatar photo, first complete your Disqus profile. | View the list of supported HTML tags you can use to style comments. | Please read our commenting policy.
 


Video