5 ms·
After reading through about half of those examples, I can't go on. The only one I saw that was potentially useful was the multi-catch. Half of the rest were th
by delecti 11y ago
After reading through about half of those examples, I can't go on.
The only one I saw that was potentially useful was the multi-catch. Half of the rest were things I had never even seen, and couldn't imagine letting slip by a code review (who uses control flow without brackets?), and the other half were so trivial that I wouldn't even waste the time to run the tool for them (why is the non-null case even preferable to do first? most people I know prefer the input validation step first anyway).
- ExpiredLink 11y agoYep, the examples are not convincing. Some even make to code worse.
- JnRouvignac 11y agoWhich ones?
- ExpiredLink 11y agoThe IfElseIfSample.java converts a clear if-else to a if-elseif without else. As if the programmer had forgotten the else.
- JnRouvignac 11y agoI am trying to understand your point but I fail. Why should there always be a else to an if-elseif? Secondly, in the original if-else, the if in the else clause does not have a else. Why is it not a problem for you? It seems contradictory...
- ExpiredLink 11y ago> Why should there always be a else to an if-elseif? Because that's good style! > in the original if-else, the if in the else clause does not have a else An if can live without an else. BTW, there are many books and online resources about programing style.
- wtetzner 11y agoWhat should go in the else? A comment saying "// Do nothing here"?
- ExpiredLink 11y ago> What should go in the else? What's left. What else should go in else? BTW, the original example not disimproved by AutoRefactor was okay.
- JnRouvignac 11y agoThanks, I am reading many things about good code and I have never seen such advice. "Good style", "best practice", all of this comes from an observation made by somebody who can justify it is good practice by some real evidence. If you were using if-elseif like a switch statement, then maybe there should be an else. If the if conditions are not related, then I don't see why there should necessarily be one. Please enlighten me by pointing to the relevant online resources.
- adam-f 11y agoYou've got a change under "// should not be touched" http://imgur.com/CSHnOuP http://imgur.com/CSHnOuP
- JnRouvignac 11y agoThanks for reporting. Actually the comment is wrong in that case using BigDecimal.valueOf() is better since it could reuse the immutable object, so that is better for memory. I'll rework the samples to make it more explicit.
- JnRouvignac 11y agoFixed.
- JnRouvignac 11y agoI am the author of AutoRefactor. If you only need multi-catch refactorings and have never seen the other pieces of code, then you are the luckiest Java developer I have heard off. You are probably working on a green field project with a small team of exceptionally talented people. For people not in that case, chances are they are inheriting a very large code base originally written with Java 1.4 or (shudder) even earlier. They are working with some people that never manage to get the simplest indentations right, let alone the code patterns you saw. The size and age of the code base made for numerous failed attempts at running checks type. Finally, the team is so big that no human can review all the code changes pre or post commit. Believe me, this exist and I think this is probably most of the code written. Concerning the not null case, I agree this one is very subjective. I hesitated a lot before adding it. It is not as broad as what you think it is. It is only touching if statements when the then and else clauses are similar. Say if (o == null) return null; else return o.toString(); I am arguing that the cognitive load is less important when the if statement is inverted. i.e. if (o != null) return o.toString(); else return null; Or with ternary operator: return o != null ? o.toString() : null; You can disagree and then not run this particular rule.
- delecti 11y agoAdmittedly, none of the code I work with on a regular basis is older than ~4 years, but some of that has changed hands several times (hot potato project nobody wants to own), and actually still runs on Java 1.4. I suppose I'm willing to accept that I've been lucky though, and god help anyone whose code has the sorts of problems I wrote off as being unacceptable.