I apologize for the length, but in order to get somewhere in this
discussion, I thought it prudent to get into slightly less
academically short code snippets. This hasn't improved on the brevity,
unfortunately :/
Executive Summary: There is serious harm in enforcing 'final' usage in
parameters: There are many real-life situations where modifying the
parameter, especially on the first line of the method, and especially
when adding to a method written a while ago, where modifying the
parameter is the nicest possible way you could implement something.
Read on for specific examples...
In Robert's myConcat example, he inlined the entire rest of the method
body into a single expression and lumped that into a return statement
instead of modifying the parameter with its normalized form. Nice
trick, but has major downsides:
1) In other complex methods, you could get away with it, except that
the variable is used multiple times and each usage needs to be wrapped
in the transformation.
-OR- if you want to avoid it and translate only once, then:
2) If the method is a little more complex, then the expression becomes
an enormous monster which any other style guide would tell you
requires splitting up. You could do this by creating another method,
but then you're creating rather a lot of extra methods, probably
called foobar(), foobarHelper1(), foobarHelper2(), etcetera, which
really isn't an improvement.
-OR-
3) If the method is a lot more complex, you can't even compress it
into a single expression anymore, and you're forced to use helper
methods.
And now for an example that really drives the point home: Exceptions.
Let's say we have a method that will read configuration data. Our
config system stores stuff into files, but we have a rule where, if
the file just isn't there, we assume there are no special
configuration directives. (Like its an empty file). One of our config
files is rather awkwardly named and thus in a version update we change
the name from 'global' to 'main', but in order to remain backwards
compatible, we want to treat a request for the 'global' settings as
the same as a request for the 'main' settings.
If we allow modification in the pre-amble, this is easy, bordering on
the trivial (lines //V2 have been added/modified in the update):
public List<ConfigDirectives> readConfig(String configKey) {
configKey = translateOldKeyNames(configKey); //V2
try {
Logger.get(this.getClas()).log("Request for config file: " +
configKey);
return readConfigDirectivesFromDisk(new FileInputStream
("configfiles/" + configKey + ".cfg"));
} catch ( FileNotFoundException e ) {
return Collections.emptyList();
}
}
The one doing the editing really could get away with not even looking
at what the actual body of the method does. Nice.
Now lets try with your rule:
Urgh. We can't. Anything we try is going to end up being ugly. We
could (A) repeat the translate method call both in the logging action
and in the readConfigDirectives action. We could (B) move the logging
action (but why is that an improvement?) and replace the configKey
variable with a translate() call. We could (C) Create a configKey2
variable. (D) We could come up with a better name and waste time on
it, and still have an inferior solution, or (E) We could rename this
method to readConfig0 and make it private, then create a new
readConfig which just calls readConfig0 with the normalized key.
That's 5 crappy solutions.
Okay, fine, I hear you: The above example seems contrived somehow. You
tossed that logger in there out of desperation! Well, I'd argue that
multiple usage of a variable is quite common, but let's roll with it
and take out the logger statement. Now it's simple right? We just
replace 'configKey' in readConfigDirectivesFromDisk with
translateOldKeys(configKey), and we can go home!
Except that is STILL inferior! Instead of a simple, easily makable and
readable change, we now changed the guts of the method - a long line
which will be hard to see the difference on when someone else checks
the commit logs. Also, anyone that later adds the log line, or some
other line (let's say they add the config list to a cache which isn't
a crazy idea), may make the mistake of using the non-translated
version which creates a hard to find bug, that only shows up when
legacy code calls this method. Whoops! To summarize the problems with
your approach:
(A) To make the code edit, or to even understand what's happening in
regards to the translate operation, one needs to read the entire
method instead of glancing at the top.
(B) During the read-through, the code updater may miss one if he's not
careful. No style checker in the world is going to find it, and it
might easily give you a hard to find bug.
(C) The code you end up with doesn't make it obvious that the method
is not envisioned to ever use the non-normalized key again. The method
with the modification right up front makes it abundantly clear by
wiping the original from the addressable space by overwriting it.
The optimal way to solve this one in a functional view of the world is
to first create a new variable (keyNormalized or something), and then
UNDEFINE the old one. This way anyone that tries to use the original
gets an error which should quickly lead to him or her coming up with
the right solution (either use the normalized version or create a new
paramRaw variable to make it abundantly clear what's happening).
However, java doesn't support that.
Hence, my theory is: In anything but the most trivial of examples,
inlining the transformation as you did in your myConcat method, is a
bad thing more often than it being a good thing. Yes, there was a typo
in your code (double 'left'), but I found that utterly unconvincing,
because this typo is exactly as likely in your 'correct' method:
String myConcat(final String left, final String right) {
if ( left == null ) return myConcat("", left);
if ( right == null ) return myConcat(left, "");
return left + right;
}
and exactly as hard to find as your typo. When you compare things, you
have to make an effort to compare apples to apples.
On Feb 14, 1:31 am, Robert Fischer <[email protected]>
wrote:
> Reinier Zwitserloot wrote:
> > Specifically: There is considerable harm in enforcing 'final' usage.
> > The right answer is obviously: Use 'final' where appropriate, and
> > don't where it isn't. Any explicit unbreakable rule is generally a bad
> > thing. What should be the default? non-final wins, because its shorter
> > and does not require your IDE to magic in extra keywords.
>
> What's the "considerable harm in enforcing 'final' usage"? And did you see
> the rest of my post, or
> did I put you off with the flippant stuff? There's a major set of
> argumentation I got into that you
> haven't responded to, and I'd be curious to hear your thoughts, because that
> really gets at the
> issue in more depth.
>
> As for things being better because they're shorter -- if that's really such a
> concern, then why are
> getters and setters the default? Or "public"? Or descriptive method names
> vs. "mpz_cdiv_r_ui"[1]?
> Or generics? All of those are lengthier than the alternative, and
> usually used via IDE assists.
> But they all provided value that make them worth their while as defaults.
>
> BTW, I wasn't making an "explicit unbreakable rule". Just suggesting an
> alternative. We both agree
> that one should "Use 'final' where appropriate, and don't where it isn't."
> The question is how
> widespread is its appropriate use? Where is that balance?
>
> [1]http://gmplib.org/manual/Integer-Division.html#Integer-Division
>
> ~~ Robert Fischer.
> Grails Training http://GroovyMag.com/training
> Smokejumper Consultinghttp://SmokejumperIT.com
> Enfranchised Mind Bloghttp://EnfranchisedMind.com/blog
>
> Check out my book, "Grails Persistence with GORM and
> GSQL"!http://www.smokejumperit.com/redirect.html
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "The
Java Posse" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/javaposse?hl=en
-~----------~----~----~----~------~----~------~--~---
