From: Lew on
Lew wrote:
>> There's nothing wrong with writing the 'readLine()' assignment twice, since
>> that is what purchases the scope confinement for the 'line' variable.

Tom McGlynn wrote:
> For me duplication of code is almost always inelegant and even
> slightly dangerous. There's always the chance that there could be
> unintended inconsistencies between the two instances--especially when
> code gets modified. I make no claim that this is a massive issue, but
> neither is the suggested change very large.

Copy-and-paste avoids divergence of the duplicated expressions.

Since I don't have a problem with the idiom the OP wrote about, I usually do this:

for ( String line; (line = reader.readLine()) != null; )
{
...
}

This limits the scope of 'line', avoids curly-brace explosion and avoids
unnecessary duplication of code, much like your proposed 'while' syntax except
that it's legal.

--
Lew
From: Tom Anderson on
On Fri, 6 Nov 2009, Lew wrote:

> Lew wrote:
>>> There's nothing wrong with writing the 'readLine()' assignment twice,
>>> since
>>> that is what purchases the scope confinement for the 'line' variable.
>
> Tom McGlynn wrote:
>> For me duplication of code is almost always inelegant and even
>> slightly dangerous. There's always the chance that there could be
>> unintended inconsistencies between the two instances--especially when
>> code gets modified. I make no claim that this is a massive issue, but
>> neither is the suggested change very large.
>
> Copy-and-paste avoids divergence of the duplicated expressions.
>
> Since I don't have a problem with the idiom the OP wrote about, I usually do
> this:
>
> for ( String line; (line = reader.readLine()) != null; )
> {
> ...
> }
>
> This limits the scope of 'line', avoids curly-brace explosion and avoids
> unnecessary duplication of code, much like your proposed 'while' syntax
> except that it's legal.

I like that.

If you had this:

http://urchin.earth.li/~twic/Code/LineIterator.java

You could write:

import static LineIterator.lines;

for (String line: lines(reader)) {
...
}

However, you would throw away your ability to see exceptions, and there's
a bit more runtime overhead.

tom

--
Yesterday's research projects are today's utilities and tomorrow's
historical footnotes. -- Roy Smith
From: Tom McGlynn on
On Nov 7, 7:57 am, Tom Anderson <t...(a)urchin.earth.li> wrote:
> On Fri, 6 Nov 2009, Lew wrote:
> > Lew wrote:
> >>> There's nothing wrong with writing the 'readLine()' assignment twice,
> >>> since
> >>> that is what purchases the scope confinement for the 'line' variable.
>
> > Tom McGlynn wrote:
> >> For me duplication of code is almost always inelegant and even
> >> slightly dangerous. There's always the chance that there could be
> >> unintended inconsistencies between the two instances--especially when
> >> code gets modified. I make no claim that this is a massive issue, but
> >> neither is the suggested change very large.
>
> > Copy-and-paste avoids divergence of the duplicated expressions.
>
> > Since I don't have a problem with the idiom the OP wrote about, I usually do
> > this:
>
> > for ( String line; (line = reader.readLine()) != null; )
> > {
> > ...
> > }
>
> > This limits the scope of 'line', avoids curly-brace explosion and avoids
> > unnecessary duplication of code, much like your proposed 'while' syntax
> > except that it's legal.
>
> I like that.
>
> If you had this:
>
> http://urchin.earth.li/~twic/Code/LineIterator.java
>
> You could write:
>
> import static LineIterator.lines;
>
> for (String line: lines(reader)) {
> ...
>
> }
>
> However, you would throw away your ability to see exceptions, and there's
> a bit more runtime overhead.
>
> tom
>


Lew's construction may be the best of the bunch using an expression
with side effects. But putting it in a for loop doesn't alter the
fact that there is an expression doing two things as once -- which I
dislike. This situation is pretty much the only time I write code
that includes such expressions. At other times I've used the
duplicate assignment approach -- and occasionally have paid a price
where I missed updating one of them in later on, though it has usually
been quickly detected. I still don't like either approach. It sounds
like Eclipse PMD plugin shares at least some of my prejudices.

Tom's suggestion of the use of an iterator when it's available is very
nice but as he points out the handling of exceptions is an issue. For
me at least this kind of loop comes up in contexts other than I/O.
I'll spend a little more time thinking about if I can build/use an
iterator when it seems appropriate. But it is probably overkill in
most simple cases.

Tom McGlynn