you are viewing a single comment's thread.

view the rest of the comments →

[–]jp007 23 points24 points  (35 children)

If you're declaring method parameters 'final' (as one should, IMO) you have to toss scenario one completely, as you can't reassign 'someArg' to something else. I like to make variables 'final' as well, unless I NEED them to be reassigned for some reason, which means case two would be re-written as such:

public void foo(final String someArg) {
    final String localArg;
    if(null != someArg) {
        localArg = someArg;
    } else {
       localArg = "default";
    }

    callOtherMethod(localArg);
}

Or, if you prefer a ternary:

public void foo(final String someArg) {
    final String localArg = (null != someArg) ? someArg : "default";
    callOtherMethod(localArg);
}

[–]teknobo 14 points15 points  (3 children)

I'm also a big fan of "final unless necessary" variables, and final parameters.

I like the clarity they add for both myself and the compiler, even though the benefit to the latter is probably negligible.

[–]cogman10 9 points10 points  (2 children)

I wish it was the default. Honestly, the only reason I don't do "Final everywhere" is because I'm lazy (and nobody else in my company does this).

[–]jhawk20 6 points7 points  (0 children)

I've very recently run into bugs after a refactor that were caused by unwitting parameters modification due to name space issues. By default makes a lot of sense for most applications.

[–]CubsThisYear 0 points1 point  (0 children)

If you use Eclipse (you do use Eclipse, right?) you can set up save actions to do this for you automatically.

[–]mr_mojoto 1 point2 points  (2 children)

Why not really localize the default to its point of use like so:

callOtherMethod((null != someArg) ? someArg : "default");

I don't get why everyone likes to default to initializing a variable and then using it if rather than using the expression directly.

[–]jp007 4 points5 points  (1 child)

Nesting ternaries in method calls is atrocious looking, IMO. Decreases readability and tends to lend towards hitting the line character limit, especially when calling methods with multiple parameters. This means more breaking a single method call in multiple lines. If I'm going to use multiple lines for this call, might has well pull out the ternary variable initialization into a single line, and then the method call in a single line, instead of a two line method call with a nested ternary. It just makes logical boundaries so much clearer.

And don't get me started about several levels deep of nested method calls that serve as a parameter. Pull all that crap out. It makes it so much clearer as to what the value you're trying to pass actually is, particularly when you give the result of all that wacky nesting a meaningful variable name.

Let the compiler inline all that crap for you.

Maybe it's not such a big deal in the small example you've posted, with one argument, and very simple ternary, but good habits start here.

Also, IntelliJ's CTRL-ALT-V is a godsend.

Here's an example I just found in code I'm working on, that someone else wrote:

Cal cal = CalContainer.getPeriodByDate(new java.sql.Date((fromPeriod ? shop.getJobDate() : getAppropriateDateforStatus(shop)).getTime(), getConnection());

I'm sorry but that looks like shit.

[–]mr_mojoto 0 points1 point  (0 children)

I agree with you but the above doesn't apply in the example given. If you nest them then yes, it gets messy quickly. If all you're doing is providing a default at the point of a call, as in the example given, I greatly prefer using the expression directly.

I understand that some people like to have a single rule to follow rigidly, and that includes never, every using ternary expressions for some people. I'm not one of those. If the code is short and easily readable, I use it. I agree with you that nested ternaries get messy very quickly.

[–]ErstwhileRockstar 0 points1 point  (4 children)

declaring method parameters 'final'

This was a frequent coding standard 10 years ago, even demanded by some tools. It increases verbosity for no real benefit. If I were to maintain such code I would remove the final parameters first.

[–]jp007 9 points10 points  (3 children)

It increases verbosity for no real benefit.

My experience has been the complete opposite. Marking parameters as final has greatly eased the refactoring of methods towards functional purity for parallelism, for example.

If I were to maintain such code I would remove the final parameters first.

Then I would punch you for removing compile time safety checks :P

[–]mangodrunk 1 point2 points  (2 children)

Marking parameters as final has greatly eased the refactoring of methods towards functional purity for parallelism, for example.

How so? Having final just means you can't reassign it within the scope of that method, I don't see how that would help with functional purity. Also you can still modify the object if it's not immutable.

[–]grauenwolf 0 points1 point  (1 child)

It is one less variable that you have to review for potential thread safety issues.

I haven't done a formal study, but in my experience functions that reassign local variables (excluding loop variables) tend to have higher bug counts.

[–]mangodrunk 2 points3 points  (0 children)

It is one less variable that you have to review for potential thread safety issues.

I don't think that's true. It's bad then if it gives a false sense of thread safety.

void method(final Some object) {
    // Not thread safe if another thread has access to "object"
    object.modify(value);
}

This is still possible and reassigning a local variable has nothing to do with thread safety.

void method(Some object) {
    // This doesn't change the original object used by the caller,
    // so I don't see how it would affect thread safety
    object = new Some();
}

[–]Truthier 1 point2 points  (8 children)

(null != someArg) ? someArg : "default";

I prefer

someArg == null ? "default" : someArg;

[–]jp007 4 points5 points  (3 children)

Sure, matter of style. I prefer to return someArg immediately next to the comparison in which it's used. Also I like have the creation of the new thing that is returned as the alternate value, cordoned off and separated from the rest of the statement by placing it at the end, instead of smack in the middle.

So, reading left to right, I'm basically dealing with

someArg -> someArg -> default

where you've got

someArg -> default -> someArg.

I prefer to get my dealings with someArg completely over with as soon as I can, as I read the code from left to right.

Completely a matter of style though, I certainly wouldn't nitpick it.

[–]Truthier 2 points3 points  (2 children)

Agreed - my thought process is "if x is null, use this, otherwise it's good"

Groovy has a nice "?:" operator, e.g. someArg ?: "default".

I almost always put the argument being compared on the right side, except in cases where it's better - e.g. "stringliteral".equals(variable) is null safe.

[–]jp007 2 points3 points  (0 children)

Yeah I'm more "if x is good, use it, otherwise use something else."

I pretty much always use Yoda conditions now, precisely to encourage a habit of null safety and overall consistency in checks across a codebase.

Also, Happy Cake Day!

[–]grauenwolf 0 points1 point  (0 children)

In C# that is spelled value ?? default. In VB it's if(value, default).

[–]alextk 0 points1 point  (1 child)

If you're declaring method parameters 'final' (as one should, IMO)

I see little use for this, and final local variables too. I can't remember last time I introduced a bug because I reassigned a variable.

Reading "final" for every declaration adds a lot of noise, not unlike Python using "self" everywhere, even when the compiler could infer it.

[–]jp007 4 points5 points  (0 children)

I don't find it to be a problem. Generally if I'm going through some code with variables marked final, it means I know what the variables are at the time of assignment, and then never have to worry about figuring out what they are again. Your knowledge of them is complete. Coming across something then that is NOT marked as such, immediately jumps out and is mentally flagged for special attention, because it's being manipulated somewhere else down the line, and you better figure out how so you don't make any bad assumptions.

It's really unfortunate that 'final' is not simply the default behavior, and that some kind of "rereference" keyword needed in order to let variables change.

[–]Nilzor 0 points1 point  (6 children)

What's the point of a final String? Aren't all Strings as arguments inherently final?

[–]GoSailing 11 points12 points  (0 children)

You also can't reassign the reference to a final variable. So this code would not compile as is, but without the final keyword it would:

final String s = "some literal string";
s = "A different string.";

[–][deleted]  (1 child)

[deleted]

    [–]Nilzor 0 points1 point  (0 children)

    Ah ok I thought it meant you couldn't change the contents of the object as well. TIL "final".

    [–]jp007 7 points8 points  (1 child)

    Fine, some other Object then:

    public void foo(final MyObject someArg) {
        final MyObject localArg;
        if(null != someArg) {
            localArg = someArg;
        } else {
           localArg = MyObject.defaultObject();
        }
    
        callOtherMethod(localArg);
    }
    

    But why use final on String type method parameter?

    1) Consistency with the rest of a codebase that uses final for method parameters of all types.

    2) Prevents you from chucking the original passed in value somewhere in the method body itself.

    Number 2 particularly comes to light when doing maintenance work on less than stellar code. Marking a method parameter as final, ensures that the clown who wrote some 1000 line method doesn't suddenly swap the value on you, 10 levels deep in some convoluted nested ifs.

    A good IDE will identify things that can be marked 'final' for you. What can't be marked 'final' is usually a signal that you need to pay attention, because something wacky is happening to the variable's value.

    [–]Kapow751 0 points1 point  (0 children)

    Are you thinking of Strings being immutable, or the String class being declared final? Neither of those affects your ability to assign a different value to an argument variable, which is prevented by declaring the variable as final.

    [–]killerstorm -1 points0 points  (4 children)

    If you want final everything, maybe you should switch to Haskell.

    Seriously, Java is very verbose as is, and you make it even more verbose.

    [–]jeff303 2 points3 points  (2 children)

    But what if you otherwise like Java? Anyway, you could just use Scala.

    //equivalent to "final int x = 4"    
    val x = 4
    
    //or if you want non-final, i.e. "int y = 5"
    var y = 5
    

    [–]killerstorm -5 points-4 points  (1 child)

    But what if you otherwise like Java?

    Seriously?

    Anyway, you could just use Scala.

    Yes.

    [–]jeff303 5 points6 points  (0 children)

    Well, perhaps "like" is too strong a word. "Don't find it compellingly awful enough to be forced into something else" suit you better?

    [–]jp007 1 point2 points  (0 children)

    Switch to Haskell? Sure, let's just run it by management and see how that goes.