Final Method Parameters Should Generate Compiler Warnings...
Another emerging trend in "Quality Assurance Driven Development" is the overuse of the "final" keyword. Especially the use as a method parameter:
public void setCount(final long count) {
this.count = count;
}
comes with a little value. It could help you in cases when you forget "this", or try to assign something to a parameter. Both errors would be immediately visible in the first unit or integration test.
I forget sometimes the "this." keyword what immediately results in an IDE (NetBeans / IntelliJ in my case) warning. I cannot remember any case of an attempt to assign something to a parameter...
Defining final in every method declaration just increases the noise and prevents rather esoteric errors. Btw. final method parameters are sometimes absolutely required - and therefore compiler cannot warn you. The following codes does not compile without a final method parameter:
public void setCount(long count) { // final required here...
this.count = count;
new Runnable(){
@Override
public void run() {
System.out.println("" + count);
}
};
}
Did you ever had trouble with non-final method parameters?
Interesting question which I've been thinking about myself too. The Checkstyle's "designed for extension" check that forces non-empty methods to be final raises the same question too.
From a conceptual point of view I agree with both checks. Both arguments and most methods should be final because both can lead to dodgy code when used incorrectly.
"When used incorrectly" is interesting though. I can't really think of any situation where I made an accidental assignment or did a method override that I wasn't suppose to do without immediately getting an IDE warning or a failing test. My opinion is that it's not worth the extra clutter.
Posted by Paul Bakker on March 09, 2011 at 05:20 PM CET #
@Paul,
"not worth the extra clutter." -> is a perfect summary of this post.
Thanks!,
adam
Posted by adam-bien.com on March 09, 2011 at 05:29 PM CET #
I don't agree. If it comes with an advantage, how small it might be, just do it. If you are too lazy to add it everytime (like me) just let the IDE add it for you.
In the case of Eclipse:
Windows > Preferences > Java > Editor > Save actions > "add final modifier to method parameters".
Done, you never have to think about the final again. After writing, press Ctrl-S, and everything is final.
IMHO the best solution would be to have final as the new default, and adding a parameter when it is NOT final. This is how the 'new/hot' languages do it, like Scala. This will specifically show you in the code when the programmer chooses not to have the parameter as final.
Posted by Roy van Rijn on March 09, 2011 at 05:31 PM CET #
@Roy,
"If it comes with an advantage, how small it might be, just do it."
I don't agree here - it is the first step to over-engineering.
"If you are too lazy to add it everytime (like me) just let the IDE add it for you."
It is not the problem of writing, but reading. In my opinion it is just additional noise.
"MHO the best solution would be to have final as the new default, and adding a parameter when it is NOT final."
A good idea - this would be nice. Little advantage with no noise :-)
thanks for your constructive comment!,
adam
Posted by adam-bien.com on March 09, 2011 at 05:42 PM CET #
I am afraid I am not following. As you show in your example at the bottom sometimes final parameters are *required* so I think they must not cause compiler warning. Quite the contrary: It would be better if final method parameters would be the default. Scala sort of heads in that direction by enforcing immutability. A variable introduced with "def" is immutable - only "var" variables are actually changeable.
Posted by Eberhard Wolff on March 09, 2011 at 05:49 PM CET #
@Eberhard,
default final would be a perfect solution.
Did you ever encountered a problem with non-final method parameters?
As I wrote in my post - compiler cannot generate warnings, because sometimes final is required.
Sure it would be better, but I never had a problem without final.
thanks!,
adam
Posted by adam-bien.com on March 09, 2011 at 05:55 PM CET #
Having all objects immutable causes most of the problems we are having these days in software development. So I am a big fan of having most objects final/immutable.
If you have to deliberately make objects mutable you'll have to think about the implications. Such as how these objects will behave with multiple processes editing and using the object.
So the question should be:
What situations do you have when you do not want your parameters to be final?
Posted by Roy van Rijn on March 09, 2011 at 05:58 PM CET #
*having all objects MUTABLE causes most problems
Posted by Roy van Rijn on March 09, 2011 at 06:03 PM CET #
@Roy,
what can happen in worst case with non-final *method* parameter?
Immutability in general: there is no question about its value.
thanks!
adam
Posted by adam-bien.com on March 09, 2011 at 06:11 PM CET #
@Eberhard,
I just checked the Spring v3.0.5 (core, context and beans) source code. It is clean ...and I didn't found any occurrence of a final method parameter (what is good :-)).
regards!,
adam
Posted by adam-bien.com on March 09, 2011 at 06:23 PM CET #
Beginner programmers might think the parameter is the actual object, instead of pass-by-reference (or rather, pass by pointer-value). So they might do:
public void badSwap(int var1, int var2)
{
int temp = var1;
var1 = var2;
var2 = temp;
}
This will never work. And it won't happen if you add final, because people won't be able to reassign.
The example above will probably only fool some beginning programmers, but things can get messy in larger methods:
public void resetLinePoints(Point pointFrom, Point pointTo)
{
pointFrom.x = 0;
pointFrom.y = 0;
pointTo = pointFrom;
}
If called with points 10,30 and 40,50 you'll end up with 0,0 and 40,50...!
Also, for clarity sake, I want to read the method declaration and know what the input parameters are.
For example:
public void walkToParent(Node leaf) {
.... a lot of code here....
while(node != null) {
print(node);
node = node.parent;
}
.... more code here....
//I need to add something here!
}
When I'm adding my code I don't want to read all the other code. If the argument was final I could just use it, but now I have to scan the code.
Posted by Roy van Rijn on March 09, 2011 at 06:34 PM CET #
I remember a bug in a large Java code base were a final constructor parameter would have avoided a problem. I found the problem later during debugging in a pair session.
It’s long time ago, were PMD and Checkstyle are not very popular and the Java IDE does not give a warning (hint) about this error.
Typing errors are always possible in a coding session and finals sometimes cover typing errors early at compile time.
Sometimes I see coding practices like
public double multiply(double value) {
value *= value;
return value;
}
which is a bad practice (my opinion). A simple final prevents doing this coding style. I know these are simple examples, but think on complex codes were calculations break due to assignment a value to a method parameter and later using the method parameter in calculation.
Generally I prefer final in case of reference parameter, never for primitive types and wrapper. On the other hand I do not assign a value to a method parameter, never, never! In case of assigning a value I use a local variable and not the method parameter.
Murphy’s law says you can absolutely guarantee that bad code will be in the middle of a huge piece of code, and the error reports won’t directly lead you here ;-)
Posted by Jörg Rückert on March 09, 2011 at 08:08 PM CET #
@Roy,
"Beginner programmers might think the parameter is the actual object"
This was even fixed in PHP :-) Seriously you learn that in second day in the old Sun SL-275 course.
"but things can get messy in larger methods:"
Yes, but in the role of QA department I would rather insist on writing small and simple methods and not make all parameters final.
thanks for your examples!,
adam
Posted by adam-bien.com on March 09, 2011 at 08:19 PM CET #
@Jörg,
In the role of QA department I would insist on writing tests for non-trivial methods. You will find a problem very quickly then.
Recently someone tried to introduce the hungarian notation for parameters - to make the code more readable and prevent the empty self-assignment,
thanks!,
adam
Posted by adam-bien.com on March 09, 2011 at 08:21 PM CET #
I've never get chance to use "final" in the parameters or instruct my fellow programmers to use unless the compiler mandates it.
There were couple of instances when I found such "overriding-code" during the code reviews.
But that is easy enough to correct via the reviews itself, rather than adding noise to the method signature.
eg: method(final..., final..., final...)
More over one good thing is, primary datatype doesn't require this. ;-)
Posted by Mahesh Subramaniya on March 09, 2011 at 10:03 PM CET #
@Adam
Writing tests is not longer a best practice, it is meanwhile mandatory...the time for using hungarian notation is over since IDE support for naming (e.g. camel case).
Using final for pass-by-reference parameter is only a marker to say I know what I do: I pass a reference which could be a risk.
It still prevents not to pass a reference parameter and call after method returned a setter method on the passed reference.
Following Joshua Bloch the solution is defensive coding, which prevents errors in context of mutable objects.
One comment termed immutable objects which are a good coding practice, but not usable for all business cases. Mutable objects are necessary to avoid the overhead of creating large amount of instances.
but I agree overusing final is to noisy...
Posted by Jörg Rückert on March 09, 2011 at 10:30 PM CET #
@Jörg,
yes - everything is a trade-off. Noise vs. prevention of unlikely errors. The best solution would be having final as default Java setting in method parameters.
Posted by adam-bien.com on March 10, 2011 at 12:27 AM CET #
I agree that final is safer, but anyway you must test your code instead of just tie the hands of the programmers.
I like to use final in parameters but I don't think it must be a default or generate warnings if not used.
A case that can lead to errors is when you pass an array as parameter and change the values of the array inside the method. The final keyword will not save you in this case and in many other cases.
And sometimes is good to just reassign the parameter like this:
001..public void myMethod(String s) {
002......s = s.trim();
003......if (s.isEmpty()) {
004..........// do something
005......} else {
006..........// do other stuff
007......}
008..}
instead of
001..public void myMethod(final String s) {
002......String x = s.trim();
003......if (x.isEmpty()) {
004..........// do something
005......} else {
006..........// do other stuff
007......}
008..}
( the format of the code above was generated by codepaster: http://code.google.com/p/codepaster/ )
Posted by Carlos Eduardo klock on March 10, 2011 at 07:41 AM CET #
@Adam
The time is over to give final for method parameter a special meaning which says: pass an entire object (pass-by-value) and not only the reference (pass-by-reference). Note, a copy of a reference is still a reference in this case. Backward compatibility is not guaranteed by changing the final behavior for method parameters. Programs would collapse changing the meaning of final for method parameters. I assume some programs could work better after the change (software which uses not the defensive copy strategy when necessary). I don’t know actually whether to book my holiday trip before or after the change (-:
Final as default for method parameter could be possible to reduce the noise and prevent the errors due to reassign a method parameter but not a possible error using the reference after passing it to a method to call a setter which changes an attribute. On the other hand don’t wake a sleeping dog, so let the final behavior for method parameters as it is, could be the best solution still trusting the skills of the programmer.
Posted by Jörg Rückert on March 10, 2011 at 09:39 AM CET #
@Jörg,
"I don’t know actually whether to book my holiday trip before or after the change (-:"
If you have fun with Java - you don't need vacations :-)
adam
Posted by adam-bien.com on March 10, 2011 at 10:56 AM CET #
@Carlos,
your first example is absolutely o.k!
thanks!,
adam
Posted by adam-bien.com on March 10, 2011 at 12:05 PM CET #
I can only speak for myself: I'm one of those who are "overusing final" as you might say, because It helps me reading source code, as whenever I see a non-final reference, it immediately raises my attention, giving me a better focus on which values are actually changing.
When I see a non-final parameter among 2 final parameters: this is the one I've got to pay extra attention to.
Even though the simple getter/setter example is a good example where final may not add to clarity in your view, doing it anyway keeps the concept of "non-finals raise my attention" consistent throughout the code in mine, not working against honing this way of viewing code.
There is something else I love about using final parameters: It boosts the code-smell of introducing too many method parameters.
I do think that it was a huge mistake to introduce "final" instead or "var" into the Java language. Each variable should have been implicitly final. Unfortunately it is way too late to correct it.
I have the impression that language designers love to ignore the fact that most variables are just narrow scoped constants and since Scala was mentioned: I was unpleasantly surprised with their solution: What a horrible choice it was to use two equally looking mnemonics, like "var" and "val" for separating variables from constant values! I don't care if French programmers made jokes about it, but "con" instead of "val" would have been a better choice, maybe even "def".
kind regards,
Posted by Wanja on March 10, 2011 at 12:07 PM CET #
@Wanja,
I like your constructive comment. If you are designing an application from scratch and carefully using "final" it is an interesting approach.
Using final as a vehicle to reduce the number of parameters would be the first true benefit of using final in general. This is actually a good idea.
I don't like the generic approach like "generate final for method parameters and don't care". It just introduces additional noise with no benefit.
really liked your comment!,
adam
Posted by adam-bien.com on March 10, 2011 at 12:48 PM CET #
I like to turn parameters final when I'm refactoring Java legacy code that was ported from other languages. Other than that, I don't use final parameters as common practice. But default final parameters would be the best solution IMO.
Posted by Sérgio Siegrist on March 10, 2011 at 03:12 PM CET #
@Sergio,
+1,
thanks!,
adam
Posted by adam-bien.com on March 10, 2011 at 03:18 PM CET #
@Wanja
„There is something else I love about using final parameters: It boosts the code-smell of introducing too many method parameters.”
There are three strategies (excluding varags and collections) to reduce method parameters (up to 4):
(1) break method in multiple methods
(2) create a helper class to hold parameter groups
(3) use the builder pattern
How comes final to play (I assume you don’t think of the horizontal justification)?!
Posted by Jörg Rückert on March 10, 2011 at 07:08 PM CET #
@Jörg:
You assume wrong: It really is nothing more than the horizontal justification:
If you have a "final" infront of each parameter, each parameter takes more space. Where 5 or 6 parameters may still look tolerable without a "final" infront, they will most certainly look darn ugly when each one is preceded with a "final" keyword. As an effect you will be sooner tempted to take one of the strategies you noticed, to shorten your parameter list.
If you prefer vertically aligned parameters this won't help you, sure. Fortunately that's never been a preferred Java code formatting anywhere where I have worked so far.
Having said that, I'm afraid that if the JSR303 method parameter verification proposal will ever be realized and widely used, vertical parameter lists are the last way to save us from getting caught in a jungle of Metadata cluttering up our method signatures.
Posted by Wanja on March 10, 2011 at 09:34 PM CET #
@Wanja
I like your explanation, it sounds confident (-:
I'll take two words from your text to go in a different direction, just to move the pointer to a problem we could have in the future of Java EE.
"Metadata cluttering" that could really lead to a problem in the future of Java EE...jumping from the framework hell direct to the metadata hell.
Improved configuration by exception also known as convention over configuration could be our rescue.
Recently I read a short note about Java EE 7 and the new JMS 2.0 API. It's the right time for a brand new JMS API but I'am afraid we will strict follow the annotation approach (-:
Posted by Jörg Rückert on March 10, 2011 at 10:19 PM CET #
@Jörg,
"..."Metadata cluttering" that could really lead to a problem in the future of Java EE...jumping from the framework hell direct to the metadata hell...."
Don't worry. In Java EE 6 you need about 6 annotations to build a CRUD:
http://www.adam-bien.com/roller/abien/entry/six_essential_java_ee_6
Java EE 7 should make that even leaner. Simplification and cross-integration is the common theme.
But: please just participate in the mailing lists and process and bring concrete suggestions. The guys behind JCP are rather pragmatic...
Posted by adam-bien.com on March 11, 2011 at 11:23 AM CET #
@Adam
„But: please just participate in the mailing lists and process and bring concrete suggestions. The guys behind JCP are rather pragmatic...“
I trust the guys behind the JCP, regarding to Java EE 5 they have done a good job and Java EE 6 is even better.
The alternative to the annotation approach are deployment descriptors, which are too far from the code. We had that in the past, were a wrong configuration in the deployment descriptor could lead to a damaged application.
The implementation of interfaces which were necessary in the past that the container were able to handle component states (e.g. initialized, destroyed, passivated) and the client were able to lookup a component were also not the best solutions.
I am afraid I have no better solution like to implement as much convention over configuration (simply defaults) compared with annotations.
I see in the context of EJB not really problems, even the decoration of components starting in the JSF layer down to the JPA layer is not cluttering. That’s all ok. But in case of web services it is too much. A simple SOAP based annotated web service is with its bindings, parameter styles and returns over decorated. It is a jungle of annotations hiding the implementation. This should not happen for JMS 2.0.
I consider the registration but first I have to consult my time table (-:
Posted by Jörg Rückert on March 11, 2011 at 01:16 PM CET #
@Jörg,
"...But in case of web services it is too much. A simple SOAP based annotated web service is with its bindings, parameter styles and returns over decorated..."
deprecation of the whole JAX-WS and SOAP stack in Java EE 7 would solve the problem :-)
thanks!,
adam
Posted by adam-bien.com on March 11, 2011 at 11:46 PM CET #
@Adam
“deprecation of the whole JAX-WS and SOAP stack in Java EE 7 would solve the problem :-)”
I still like JAX-WS and also SOAP although it’s dusty, finally this combination is interoperable to the .NET platform and the rest of the world. By using the Provider interface it’s possible to develop RESTful services without JAX-RS. Dynamic dispatching with the Dispatch API is flexible and ignores the WSDL. Even Java SE 6 supports JAX-WS native with the Endpoint.publish(…) functionality.
JAXB 2.0 is really an improvement and the binding of XML and POJOs were never easier before. All that comes along with metadata (-: metadata are now everywhere…since JAX-RS we have even more metadata and the possibility to use the web browser as the gateway to the database. Do we really need that? Surely, that sounds good like to fly away in a dream but could also swap to be a nightmare in case of the web service is not properly secured. Nothing is for free with the exception of metadata (-:
Posted by Jörg Rückert on March 12, 2011 at 06:15 AM CET #
Hello, I absolutely agree to put final as default. Compiler should fire warnings when it finds a parameter assignment but as @Carlos wrote, sometimes it makes "sense". A simple warning suppression would tell others that it was my choice to do that and you don't have anything to worry about.
// I know what I'm doing!
@SuppressWarning("param-assigment")
public void foo(String s) {
....s = s.trim();
....if(s ....)
....
}
Bye!
Posted by Crick on March 12, 2011 at 04:45 PM CET #
Great post. The only use I found so far for final on method parameters is overly long brownfield-ish methods (think several hundred lines) where putting final on everything helps restore _some_ sanity (for it makes reasoning about the code easier). Of course, that can only be a temporary "solution".
- and yes, I've seen people mutate input parameters often. And I've not seen a single occurrence where it made sense.
Posted by Andreas Flierl on August 18, 2012 at 01:28 PM CEST #