3 ms·

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

by pmr_ 11y ago

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 11y ago
> 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.
- philbarr 11y ago
  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.
- pmr_ 11y ago
  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.
  - Alupis 11y ago
    > 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) You must null check a lot of things in C, especially since there is no graceful error handling (try/catch blocks)... C certainly allows things to be null (or garbage) values. > The problem is that Java did away completely with value types and made everything pointer only. I'm not sure what you are saying here -- the very notion of pointers do not exist in Java. This decision was made while creating the language, and avoids an entire class of programming errors. Java is strictly pass by value. > the third is the missing for-each construct. I completely agree with you here. Java does have a for-each construct, and it's recommended to use whenever possible. It avoids an entire class of programming errors.
    - pmr_ 11y ago
      > You must null check a lot of things in C, especially since there is no graceful error handling (try/catch blocks)... C certainly allows things to be null (or garbage) values. You are correct that adding C as an example language was wrong. C++ on the other hand still stands. > I'm not sure what you are saying here -- the very notion of pointers do not exist in Java Java references are just C pointers without pointer arithmetic.
    - burntsushi 11y ago
      > I'm not sure what you are saying here -- the very notion of pointers do not exist in Java. This decision was made while creating the language, and avoids an entire class of programming errors. Java is strictly pass by value. To clarify: the GP is probably referring to boxed and unboxed data types. IIRC, Java has some unboxed data types ("primitive" types?), but mostly everything is boxed behind a pointer.
      - Alupis 11y ago
        > but mostly everything is boxed behind a pointer. Behind a Reference would be more accurate. It's just a reference to a spot in the heap. Other than similarly "pointing" to a place in memory, the comparison between Java References and C Pointers stops there. One cannot pass a "pointer" in Java, nor can the pointer be free-form manipulated like in pointer-arithmetic.
        burntsushi 11y ago
        My statement is perfectly accurate in the context, which is discussing the representation of Java types. Java doesn't hold a monopoly on the word "pointer." For example, Go has pointers but doesn't allow pointer arithmetic in safe code. Similarly for Rust.
  - pc86 11y ago
    My word, something can be redundant (not conceding that point) and not "just plain wrong." Your language hate and insistence that the programmer himself is one of the "problems" is why people don't do code reviews, and why people get overly defensive if you offer constructive criticism of code. Your criticism is not constructive. If you are in a position of power or mentorship I suggest you take a moment to think how your words and actions influence those around you, particularly those less experienced who may look up to you.
    - pmr_ 11y ago
      > My word, something can be redundant (not conceding that point) and not "just plain wrong." I think we can agree to disagree. I'm strictly against redundancy if it doesn't serve a well-defined purpose. > Your language hate and insistence that the programmer himself is one of the "problems" is why people don't do code reviews, and why people get overly defensive if you offer constructive criticism of code. Your criticism is not constructive. I would phrase my criticism entirely different if the recipient was someone who asked for my commentary and not someone who felt confident enough to write a blogpost on how to start refactoring code.
      - Alupis 11y ago
        > I'm strictly against redundancy if it doesn't serve a well-defined purpose. Well, a good optimizing compiler might factor redundant checks out. The JVM has one of the best optimizing compilers around... Sometimes source code clarity is better than "absolute correctness", especially when we're discussing something trivial.
      - burntsushi 11y ago
        > I would phrase my criticism entirely different if the recipient was someone who asked for my commentary and not someone who felt confident enough to write a blogpost on how to start refactoring code. Right, because we should seek out excuses to be nasty to others. How about we just try to be constructive as much as possible?
  - burntsushi 11y ago
    The problem you highlighted is just redundant case analysis. That's not a Java specific problem.
BinaryIdiot 11y ago
> 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 11y ago
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_ 11y ago
  You are saying that you actually need to read and understand code you are refactoring. Hard to disagree.
zo1 11y ago
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_ 11y ago
  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.
  - zo1 11y ago
    The main point is that you shouldn't rely on an implied rule that comes from the oddity of the length being 0 on a list. If I'm looping through a list, it's because I want to loop through it and process its items. I'm going to return/skip earlier due to whatever reason, and not rely on the "logic" for processing being built into the length of the list, or whoever populates the list.

by pmr_ 11y ago

Alupis 11y ago

> 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.

philbarr 11y ago
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.
pmr_ 11y ago
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.
- Alupis 11y ago
  > 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) You must null check a lot of things in C, especially since there is no graceful error handling (try/catch blocks)... C certainly allows things to be null (or garbage) values. > The problem is that Java did away completely with value types and made everything pointer only. I'm not sure what you are saying here -- the very notion of pointers do not exist in Java. This decision was made while creating the language, and avoids an entire class of programming errors. Java is strictly pass by value. > the third is the missing for-each construct. I completely agree with you here. Java does have a for-each construct, and it's recommended to use whenever possible. It avoids an entire class of programming errors.
  - pmr_ 11y ago
    > You must null check a lot of things in C, especially since there is no graceful error handling (try/catch blocks)... C certainly allows things to be null (or garbage) values. You are correct that adding C as an example language was wrong. C++ on the other hand still stands. > I'm not sure what you are saying here -- the very notion of pointers do not exist in Java Java references are just C pointers without pointer arithmetic.
  - burntsushi 11y ago
    > I'm not sure what you are saying here -- the very notion of pointers do not exist in Java. This decision was made while creating the language, and avoids an entire class of programming errors. Java is strictly pass by value. To clarify: the GP is probably referring to boxed and unboxed data types. IIRC, Java has some unboxed data types ("primitive" types?), but mostly everything is boxed behind a pointer.
    - Alupis 11y ago
      > but mostly everything is boxed behind a pointer. Behind a Reference would be more accurate. It's just a reference to a spot in the heap. Other than similarly "pointing" to a place in memory, the comparison between Java References and C Pointers stops there. One cannot pass a "pointer" in Java, nor can the pointer be free-form manipulated like in pointer-arithmetic.
      - burntsushi 11y ago
        My statement is perfectly accurate in the context, which is discussing the representation of Java types. Java doesn't hold a monopoly on the word "pointer." For example, Go has pointers but doesn't allow pointer arithmetic in safe code. Similarly for Rust.
- pc86 11y ago
  My word, something can be redundant (not conceding that point) and not "just plain wrong." Your language hate and insistence that the programmer himself is one of the "problems" is why people don't do code reviews, and why people get overly defensive if you offer constructive criticism of code. Your criticism is not constructive. If you are in a position of power or mentorship I suggest you take a moment to think how your words and actions influence those around you, particularly those less experienced who may look up to you.
  - pmr_ 11y ago
    > My word, something can be redundant (not conceding that point) and not "just plain wrong." I think we can agree to disagree. I'm strictly against redundancy if it doesn't serve a well-defined purpose. > Your language hate and insistence that the programmer himself is one of the "problems" is why people don't do code reviews, and why people get overly defensive if you offer constructive criticism of code. Your criticism is not constructive. I would phrase my criticism entirely different if the recipient was someone who asked for my commentary and not someone who felt confident enough to write a blogpost on how to start refactoring code.
    - Alupis 11y ago
      > I'm strictly against redundancy if it doesn't serve a well-defined purpose. Well, a good optimizing compiler might factor redundant checks out. The JVM has one of the best optimizing compilers around... Sometimes source code clarity is better than "absolute correctness", especially when we're discussing something trivial.
    - burntsushi 11y ago
      > I would phrase my criticism entirely different if the recipient was someone who asked for my commentary and not someone who felt confident enough to write a blogpost on how to start refactoring code. Right, because we should seek out excuses to be nasty to others. How about we just try to be constructive as much as possible?
- burntsushi 11y ago
  The problem you highlighted is just redundant case analysis. That's not a Java specific problem.

BinaryIdiot 11y ago

> 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 11y ago

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_ 11y ago
You are saying that you actually need to read and understand code you are refactoring. Hard to disagree.

zo1 11y ago

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_ 11y ago
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.
- zo1 11y ago
  The main point is that you shouldn't rely on an implied rule that comes from the oddity of the length being 0 on a list. If I'm looping through a list, it's because I want to loop through it and process its items. I'm going to return/skip earlier due to whatever reason, and not rely on the "logic" for processing being built into the length of the list, or whoever populates the list.