CODEWITHSUNDEEP
javacoding-style
Chris
Chris

Reputation: 830

java code style question - for loops

Is the following code clear and easy to read?

public void createDatabase() throws SQLException, IOException {
    SQLiteDatabase database = dbStore.getDatabase();
    LineNumberReader scriptInputReader = new LineNumberReader(new InputStreamReader(getClass().getResourceAsStream(SCRIPT_CREATE)));
    for(String line; (line = scriptInputReader.readLine()) != null;) {
        database.execSQL(line);
    }
}

I write a lot of "for" loops like the one above. For me it looks clear - it shows the temporary variable ("line") used in the loop, limits it's scope and points out when the loop ends (when "readLine" returns "null"). I wonder if other programmers will hate me for those...

or this one:

    SQLiteDatabase database = dbStore.getDatabase();
    Cursor cursor = database.query("PINS", new String [] {"ID", "X", "Y"}, null, null, null, null, "ID");
    if(cursor.moveToFirst()) {
        for(; !cursor.isAfterLast(); cursor.moveToNext()) {
            (...)
        }
    }
    cursor.close();

Are things like the above just "neat" or already a Java-puzzles?

Upvotes: 1

Views: 1777

Answers (6)

Bohemian
Bohemian

Reputation: 425033

I like what you've done, but I would make one small change:

for(String line = scriptInputReader.readLine(); line != null; line = scriptInputReader.readLine()) {
    database.execSQL(line);
}

This separates the iteration action from the loop termination condition. Also, unlike the "while" version, it limits the scope of the line variable to the loop - narrowing scope as much as possible is good coding practice.

Also, code style checkers usually consider assignments nested within tests as "poor style". To be clear, your code is a bit like this:

for (int i = -1; ++i < max;) { // don't do this: increment action inside condition section
    // some code
}

Upvotes: 4

reynman
reynman

Reputation: 729

Some prefer to use a while-loop when you do not know when the loop will terminate. For example reading through a file.

Jarek Potiuk's solution works, however I prefer something like this:

String line = scriptInputReader.readLine();
while(line != null) 
{
  ... do stuff with line
  line = scriptInputReader.readLine();
}

It is a little bit more code, but I have to agree with Bohemian:

code style checkers usually consider assignments nested within tests as "poor style"

Upvotes: 0

Voo
Voo

Reputation: 30216

I think there are two different schools of thought here. The one as shown by Jarek Potiuk that prefers to use for/while loops for different purposes, ie. a for loop should be used when you know the range of your loop in advance (for (;i < arr.length(); i++)) while a while is preferred for unlimited situations.

But then there's the other line of thought that uses only one kind of loop and at that the for loop because it's more versatile. The Java SDK for example uses for loops pretty extensively in unlimited situations (eg linked lists). But then you should really write the for loop like Bohemian does - much clearer.

Upvotes: 0

SJuan76
SJuan76

Reputation: 24885

I would feel more at ease with while.

The first one is not that bad as it is because it is easy to understand what the loop does, but if you add more logic in the middle of the loop and the operation is complicated it will become more difficult (because people will think:'hey, if he wanted just to read a file he would have used a while, so there must be some trick').

The second one (the for doing at the work, and no code inside the loop) is awful and probably in a not so distant future someone will say: 'Hey, there was a loop here and the contents were removed, but they forgot to remove the loop! I can optimize that by removing the for altogether').

Upvotes: 1

Jarek Potiuk
Jarek Potiuk

Reputation: 20077

I'd opt for:

String line = null;
while((line = scriptInputReader.readLine()) != null) {
  ... do stuff with line
}

This is clear and straightforward.

Upvotes: 7

brian_d
brian_d

Reputation: 11385

I would use a while loop

String line = scriptInputReader.readLine();
while(line != null){
     //do stuff
     line = scriptInputReader.readLine();
}

Upvotes: 1

Related Questions