Style Police (a rant)

[Jan Burse]

[... snip ...]

Looks like you are preaching to the convert.
Anyway, here is some fun:

class A {

A() {
init();
}

init() {
}

}


class B {
foo = 3;

init() {
super.init();
System.out.println("foo="+foo);
}

}

What value for foo will be printed when I do
new B()?

Would it make sense to put a final on init()?

Bye

 

[Lew]

[... snip ...]

Looks like you are preaching to the convert.
Anyway, here is some fun:

class A {

A() {
init();
}

init() {
}

}


class B {
foo = 3;

init() {
super.init();
System.out.println("foo="+foo);
}

}

What value for foo will be printed when I do
new B()?

Would it make sense to put a final on init()?

Given that calling non-final methods from a constructor is a very well-known antipattern, it's a bit like asking, "Should I drive on the wrong side ofthe road?" You can get away with it for a while, perhaps, but sooner or later you're going to run into trouble.

This is an aspect of the rules I've been repeatedly citing in this thread: "Item 17: Design and document for inheritance or else prohibit it", "use of'final' on classes and methods is a semantic restriction ... frequently the right thing to do", "uch restrictions increase the safety and predictability of the code and systems built with it", "you have to be responsible for the consequences to program logic". Thanks for illustrating the points..

 

[John B. Matthews]

Enforcing good style is difficult. I wish the purveyors of the
enforcement tools would realize it's beyond their powers to do it
well.

Such tools seem particularly useful as an adjunct to, but not a
substitute for, peer review. I'm always disappointed when someone
thinks they can cut corners by putting such a tool in the build system,
instead of educating developers.

"A Comparison of Bug Finding Tools for Java," which compares several
related tools, offers a useful perspective.

<http://www.cs.umd.edu/~jfoster/papers/issre04.pdf>

Linked from here:

<http://findbugs.sourceforge.net/publications.html>

 

[Jan Burse]

Given that calling non-final methods from a constructor is a
very well-known antipattern, it's a bit like asking, ...

I didn't know that this is an anti pattern. A solution
could be also to make the method private.

Bye

 

[Lew]

I didn't know that this is an anti pattern. A solution
could be also to make the method private.

A well known antipattern, and private methods are inherently final. (So are static methods.)
http://www.javapractices.com/topic/TopicAction.do?Id=215

Even more cautions about constructor, such as not to start a thread within one or let 'this' escape:
http://www.javapractices.com/topic/TopicAction.do?Id=11
and on overridable methods:
http://www.javapractices.com/topic/TopicAction.do?Id=89

More on overridable methods from constructors:
<https://www.securecoding.cert.org/c...+constructors+do+not+call+overridable+methods>
<http://www.oracle.com/technetwork/java/seccodeguide-139067.html>
in particular
<http://www.oracle.com/technetwork/java/seccodeguide-139067.html#4-0> _et seq._

More on "practice safe construction"
<http://www.ibm.com/developerworks/java/library/j-jtp0618/index.html>

and the general antipattern of putting code in a constructor at all:
<http://www.beaconhill.com/solutions/kb/java/code-in-constructor-anti-pattern.html>

From this last link, a warning against using ignorance as an excuse:
"When I asked why they were doing this they stated that they did it all the time. Then I explained how this is typically a good habit to have they defended themselves with 'Nobody told me'."

 

[Arved Sandstrom]

Such tools seem particularly useful as an adjunct to, but not a
substitute for, peer review. I'm always disappointed when someone
thinks they can cut corners by putting such a tool in the build system,
instead of educating developers.

+1.

I've seen tools like this used as "feel-good" CI plugins more often than
not. If that's all they are used for then they're not much good. As you
suggest, if used with good code reviews, *and* the rules used by the
tools are also reviewed and agreed ahead of time, then they are
certainly helpful.

I'm not talking about purely styles rules here. Granted, there are
supposedly grey areas as to what's style and what's not. E.g. do we
always use braces with single statements associated with looping
constructs and conditionals, or not: I consider this to be a maintenance
bug and hence not style - I think it's almost always possible to
determine whether something is merely style, or more serious.

"A Comparison of Bug Finding Tools for Java," which compares several
related tools, offers a useful perspective.

<http://www.cs.umd.edu/~jfoster/papers/issre04.pdf>

Decent read. I agree with the view that if you're going to use tools
like this at all, use at least a couple and compare/collate the findings.

AHS

 

[Jan Burse]

A well known antipattern, and private methods are inherently final. (So are static methods.)
http://www.javapractices.com/topic/TopicAction.do?Id=215

I don't think private methods are inherently final
in the same sense that the final keyword works for
non-private methods.

private methods can be overshadowed. So it would be
an illusion to make a method init() private and then
expect that nobody can define init() as well in a
subclass.

But since private methods are compiled into a static
method invocation instruction, there is no problem
with this overshadowing.

A private method can even be overshadowed with a
public method, which is compiled into a dynamic
method invocation instruction.

Bye

 

[Jan Burse]

(So are static methods.)

Static methods are also not final. They can also
be overshadowed. Typical confusion are the
following code snippets:

class A {
public static main ...
}

class B extends A {
}

B does not have a main method. It is not
inherited.

class C extends A {
public static main ...
}

C has a main method. And it is different
from the one in A.

Bye

 

[Lew]

I don't think private methods are inherently final
in the same sense that the final keyword works for
non-private methods.

You need to read the Java Language Specification.
JLS §8.4.3.3 "final Methods"
"A private method and all methods declared immediately within a final class(§8.1.1.2) behave as if they are final, since it is impossible to override them."

In the context of this conversation, calling overridable methods from a constructor, this is relevant because there is no danger of an "override" of aprivate method causing untoward behavior from the constructor of the parent class.

private methods can be overshadowed. So it would be
an illusion to make a method init() private and then
expect that nobody can define init() as well in a
subclass.

So what?

But since private methods are compiled into a static

Bad choice of words. I know what you mean, but "static" has a particular meaning in Java and this isn't it.

method invocation instruction, there is no problem
with this overshadowing.

A private method can even be overshadowed with a
public method, which is compiled into a dynamic
method invocation instruction.

But still cannot be called by the superclass instance on itself.

 

[Jan Burse]

"A private method and all methods declared immediately
within a final class (§8.1.1.2) behave as if they are
final, since it is impossible to override them."

The above refers to a final class and not to a final
method.

private methods can be overshadowed. You can try the
following code by yourself:

class A {
private void init() { };
}

class B extends A {
public void init() { };
}

It does compile, and the class loader does also
accept it.

Bad choice of words. I know what you mean, but
"static" has a particular meaning in Java and this
isn't it.

Oops, I had in mind that there are instructions invokestatic
and invokedynamic, but infact the relevant instructions
are called invokespecial and invokevirtual.

The invokespecial instruction is used for private methods.
See here:
http://cs.au.dk/~mis/dOvs/jvmspec/ref--33.html

Bye

 

[Roedy Green]

Enforcing good style is difficult. I wish the purveyors of the
enforcement tools would realize it's beyond their powers to do it well.

You might look at it like this. You complain your solar calculator
keyboard is malfunctioning, and your partner says "Did you remember to
plug it in?".

It is not meant to tell you what to do, but merely to produce an
abbreviated check list.



--
Roedy Green Canadian Mind Products
http://mindprod.com
The modern conservative is engaged in one of man's oldest exercises in moral philosophy; that is,
the search for a superior moral justification for selfishness.
~ John Kenneth Galbraith (born: 1908-10-15 died: 2006-04-29 at age: 97)

 

[Tim Slattery]

In recent days I've encountered a tool called "Checkstyle," that
parses Java code and flags various departures from its baked-in rules.
Some of these are worthwhile: It will whine if you override equals()
without overriding hashCode(), it will shriek if a method's Javadoc
omits a @param or @throws, it will moan if a static final field isn't
ALL_CAPS, and so on. Some are less so: It insists that all method
parameters should be final, it forbids the ?: operator, it tut-tuts
at `x << 8' for using a "magic number."

Checkstyle checks exactly what you tell it to check in the config file
you tell it to use.

 

[Lew]

The above refers to a final class and not to a final
method.

Then why does it say "private method"?

private methods can be overshadowed. You can try the
following code by yourself:

class A {
private void init() { };
}

class B extends A {
public void init() { };
}

It does compile, and the class loader does also
accept it.

So what?

The constructor of 'A' still can't call the 'init()' of 'B', which is the key point here. In fact, nothing in class 'A' can call the 'init()' of 'B' via 'this'; that is why the JLS says flat out that 'private' methods work like 'final' ones.

 

[Robert Klemme]

The above refers to a final class and not to a final
method.

Read like this: "(A private method) and (all methods declared
immediately within a final class (§8.1.1.2)) behave as if they are
final, since it is impossible to override them." In other words

P"final"(m) OR P"declared immediately within a final class"(m) =>
P"behaves as if final"(m).

private methods can be overshadowed. You can try the
following code by yourself:

class A {
private void init() { };
}

class B extends A {
public void init() { };
}

It does compile, and the class loader does also
accept it.

But you can never invoke A.init() from methods in B or descendants.
A.init() is final because it can only ever be seen in A.

Kind regards

robert

 

[Andreas Leitgeb]

The constructor of 'A' still can't call the 'init()' of 'B',
which is the key point here.

In fact, nothing in class 'A' can call the 'init()' of 'B'
via 'this';

Umm... within a C'tor or method of A:
if (this instanceof B) { ((B)this).init(); } ?

Just nitpicking - probably wrong once again, but curious about
the "how": is my interpretation of "via" too general? Is there
a specific meaning of the idiom "calling via" in the JLS?

Btw., that doesn't question the original and main point, anyway.

 

[Lew]

Umm... within a C'tor or method of A:
if (this instanceof B) { ((B)this).init(); } ?

Good point, if bad code, and I'll include that in the SSCCE I'm planning to make.

Just nitpicking - probably wrong once again, but curious about
the "how": is my interpretation of "via" too general? Is there
a specific meaning of the idiom "calling via" in the JLS?

There is no meaning for "calling via" in the JLS of which I'm aware. I was trying to be clear only that I didn't mean via composition. Your corner case is interesting but outside of what I was trying to say.

Btw., that doesn't question the original and main point, anyway.

It's a cute corner case. Of course it's bad form to downcast 'this' from a parent class to begin with.

 

[Eric Sosman]

Checkstyle checks exactly what you tell it to check in the config file
you tell it to use.

Almost. It checks exactly what somebody tells it to check in the
config file that somebody requires me to use.

To be fair to the somebodies, I have yet to test just how slavishly
they'll insist that I bow to their chosen dictates. Maybe the code that
puts Checkstyle into such a tizzy will sail through review unscathed.
Or maybe some gatekeeper will insist it "pass" Checkstyle before even
getting as far as review; I honestly don't know yet.

I'm just ranting at the absurdity of a criterion that says Object
itself is "not designed for extension."

 

[Andreas Leitgeb]

Checkstyle checks exactly what you tell it to check in the config file
you tell it to use.

Almost. It checks exactly what somebody tells it to check in the
config file that somebody requires me to use.

[...] maybe some gatekeeper will insist it "pass" Checkstyle before even
getting as far as review; I honestly don't know yet.

My dark side would motivate me to find some really obviously gross
workaround that (while passing the imposed checkstyle-rules) would
inevitably make the reviewers frown and complain about until they
read the comment, which says that this gross workaround is only for
a stupid checkstyle-rule...

The rule itself seems to be guided by some mantra according to which
a class should either be final or abstract (except that the rule also
allows for empty implementations). That Object and lots of other
standard library classes don't abide by that mantra is then taken
as a case for the latin proverb: "quod licet Iovi, non licet bovi."
I don't follow that mantra, myself.

 

[Tim Slattery]

Almost. It checks exactly what somebody tells it to check in the
config file that somebody requires me to use.

True. And sometimes those somebodies are quite unyielding about quite
ridiculous stuff.

I'm just ranting at the absurdity of a criterion that says Object
itself is "not designed for extension."

I agree with that.

 

[Lew]

The rule itself seems to be guided by some mantra according to which
a class should either be final or abstract (except that the rule also
allows for empty implementations). That Object and lots of other
standard library classes don't abide by that mantra is then taken
as a case for the latin proverb: "quod licet Iovi, non licet bovi."
I don't follow that mantra, myself.

Four leeched cows looked at Jon bon Jovi?