[pmr_]

    if( masterList[z].list2 != NULL && masterList[z].list2.length() > 0 )
    {
        for( Integer y = 0; y < masterList[z].list2.length(); y++ )

The if-statement is a good summary of what is wrong with Java. The author doesn't even notice that the second argument of && is redundant and keeps it in the "refactored" version as well...

[Alupis]

> The author doesn't even notice that the second argument of && is redundant and keeps it in the "refactored" version as well..

That's false. A list object may be initialized but be empty (making it's length zero). Or it may not be initialized (making it null).

Both conditions may happen independently of one another. He checks for the null first so that checking the length does not throw a NPE.

> The if-statement is a good summary of what is wrong with Java.

Frankly, I see nothing wrong here.

[pmr_]

He then proceeds to iterate over the list using zero based indexing. The loop would not execute once if the list had length zero, making this check just plain wrong.

I know an even lower level language (C++, C) that doesn't have the problem of things which make no sense to be NULL (list elements). The problem is that Java did away completely with value types and made everything pointer only. That has been recognized by later languages (C#) and fixed.

The whole code consists of problems: the first is the language, the second is the programmer, the third is the missing for-each construct.

[philbarr]

No, you don't need the the second argument since the for loop just won't execute at all if the list length is zero.

[zo1]

You can't always rely on for-loop mechanics to do branching.

Also, having an implied "conditional" that must be extracted by reading the head-portion of a loop is very very unreadable. I'd reject your submission on a code-review.

Instead, you should do something like below. I assume that your example is contrived, so for the sake of argument, assume I have all sorts of business cruft around mine. I.e. should_process exists because there are business rules for processing/not processing the master_list.

    def should_process(master_list):
        return master_list is not None and len(master_list) > 0

    def main_doing_of_stuff_foo():
        if not should_process(masterList):
            return  # Yes, this thing should be in its own method so you can do an early exit.

        for item in masterList:  # Yes, you should be using an iterator as well, not indexing.
            print("Body goes here".)

[pmr_]

I can't follow. The code I showed is already processing masterlist and decides if it should process masterlist[z].list2 for every z: 0..masterlist.length.

I fully agree that the code in my post is very bad, I took it from the article.

[BinaryIdiot]

> The author doesn't even notice that the second argument of && is redundant

Doesn't it check if a list isn't null and then if the list has at least one item in it? Or are you simply saying the for loop takes care of the situation where there are 0 items in the list?

[philbarr]

I actually don't think he should remove the second argument until the entire thing has been verified in tests. What if it was actually:

  masterList[a].list2.length()

..but he misread it as

  masterList[z].list2.length()

and so removed it..?

[pmr_]

You are saying that you actually need to read and understand code you are refactoring. Hard to disagree.