final and constructor, a Java Wart

[Roedy Green]

Lets say you noticed that your constructor was getting overweight, and
decided to split it into more manageable chunks.

All of a sudden all your finals on the instance variable become
ERRORS!!!

The is correct because in THEORY someone later could call one of your
dependent methods.

So you remove your finals.

This is NOT a good thing.

It makes it look as though these are NOT final to other programmers.

You take off your safety wheels protecting you from duplicate
assignment or inadvertent assignment.

This usually happens when you are shuffling code around. You get some
dups.

I have two ideas to ameliorate the situation.

1. an @once annotation that says this static method can be run only
once, or this instance method can be invoked only once per object
instantiation. It would be backed by a weak transparent assert that
would be turned off when running in production. The compiler would
use the annotation to allow extra finals.

Such an annotation would be a great as general documentation and as
part of as assert mechanism.

2. IDEs should have a shuffle mode/refactor mode cut/paste. The idea
is you can move code around but not destroy or duplicate it.
You can create new code. This idea needs to be fleshed out. The
problem with any manual refactoring is you randomly lose a line of
code somewhere or duplicate one.
--
Roedy Green Canadian Mind Products
http://mindprod.com

"Species evolve exactly as if they were adapting as best they could to a changing world, and not at all as if they were moving toward a set goal."
~ George Gaylord Simpson

 

[Arne Vajhøj]

Lets say you noticed that your constructor was getting overweight, and
decided to split it into more manageable chunks.

All of a sudden all your finals on the instance variable become
ERRORS!!!

The is correct because in THEORY someone later could call one of your
dependent methods.

So you remove your finals.

This is NOT a good thing.

It makes it look as though these are NOT final to other programmers.

You take off your safety wheels protecting you from duplicate
assignment or inadvertent assignment.

I have never liked the "final on everything possible" approach.

Code clutter in my opinion.

Accidentally assigning to something that should not be changed
is not a common problem.

This usually happens when you are shuffling code around. You get some
dups.

I have two ideas to ameliorate the situation.

1. an @once annotation that says this static method can be run only
once, or this instance method can be invoked only once per object
instantiation. It would be backed by a weak transparent assert that
would be turned off when running in production. The compiler would
use the annotation to allow extra finals.

Such an annotation would be a great as general documentation and as
part of as assert mechanism.

It is only practical to implement for private methods.

And it is not very useful for private methods. It should not be that
difficult to check whether it is called multiple times within the
same class.

2. IDEs should have a shuffle mode/refactor mode cut/paste. The idea
is you can move code around but not destroy or duplicate it.
You can create new code. This idea needs to be fleshed out. The
problem with any manual refactoring is you randomly lose a line of
code somewhere or duplicate one.

Given the wide varieties of IDE's and editors used, then that
does not look as a working solution.

Some issues are just the programmers responsibility.

Arne

 

[Roedy Green]

Accidentally assigning to something that should not be changed
is not a common problem

duplicate assignment is more common, usually caused by litter left
over in the process of refactoring.

If a method that is no longer supposed to be charged with assigning
the value, is still assigning it, especially in some obsolete way, you
want to know.
--
Roedy Green Canadian Mind Products
http://mindprod.com

"Species evolve exactly as if they were adapting as best they could to a changing world, and not at all as if they were moving toward a set goal."
~ George Gaylord Simpson

 

[Lew]

Wrong answer. Leave 'final' on the variables and don't use dependent methods
to assign them. Use an instance initializer.

I have never liked the "final on everything possible" approach.

Code clutter in my opinion.

I agree, but there is a strong use case for using 'final' a lot. Any instance
or static member variable that should not be changed should be 'final'. It is
not code clutter because it invokes compiler assistance to enforce
immutability, it changes the semantics of initialization in your favor and it
guarantees thread safety.

Strong benefits for what you disparagingly deem "code clutter".

Accidentally assigning to something that should not be changed
is not a common problem.

Oh, yes, it is. I have run into it many times in production code. Even
worse, failure to properly synchronize access to unchanging but non-final
variables between threads is fairly common, and inordinately disastrous when
it occurs. The result isn't multiple assignment but failure to assign, from
some thread's point of view. The mathematical expectation, likelihood times
cost, is large.

 

[Roedy Green]

Strong benefits for what you disparagingly deem "code clutter".

If we were designing Java over, I think final should have been the
default, with some terse convention to request non-final.

As time goes on, I am becoming a bigger fan of final. It good to know
there CAN'T be a redefinition hidden somewhere.
--
Roedy Green Canadian Mind Products
http://mindprod.com

"Species evolve exactly as if they were adapting as best they could to a changing world, and not at all as if they were moving toward a set goal."
~ George Gaylord Simpson

 

[Arne Vajhøj]

I agree, but there is a strong use case for using 'final' a lot. Any
instance or static member variable that should not be changed should be
'final'. It is not code clutter because it invokes compiler assistance
to enforce immutability, it changes the semantics of initialization in
your favor and it guarantees thread safety.

Strong benefits for what you disparagingly deem "code clutter".


Oh, yes, it is. I have run into it many times in production code. Even
worse, failure to properly synchronize access to unchanging but
non-final variables between threads is fairly common, and inordinately
disastrous when it occurs. The result isn't multiple assignment but
failure to assign, from some thread's point of view. The mathematical
expectation, likelihood times cost, is large.

In my opinion Java is not the right language to try and enforce that
type of immutability.

And I would also prefer some more explicit synchronization over
final to ensure thread safeness. It makes it a lot easier the day
something non final has to be added.

Arne

 

[Arne Vajhøj]

duplicate assignment is more common, usually caused by litter left
over in the process of refactoring.

If a method that is no longer supposed to be charged with assigning
the value, is still assigning it, especially in some obsolete way, you
want to know.

It is a rather big coding mistake.

And the final trick only catches a small fraction of the problems.

Arne

 

[Mark Thornton]

I have never liked the "final on everything possible" approach.

Code clutter in my opinion.

Accidentally assigning to something that should not be changed
is not a common problem.

Final also has implications for the memory model and potentially affects
the generated code (since JSR-133). If you want to construct an
immutable object which will be passed between threads, then you should
use final on all the fields. Java then guarantees that, on completion of
the constructor, all threads will see the same values for those fields.

Mark Thornton

 

[charlesbos73]

I have never liked the "final on everything possible" approach.

Code clutter in my opinion.

Two words:

- @Immutable (from JCIP fame)
- FindBugs

 

[Lew]

And I would also prefer some more explicit synchronization over
final to ensure thread safeness. It makes it a lot easier the day
something non final has to be added.

That's just not the point of final as it relates to synchronization. 'final'
doesn't replace synchronization, it obviates the need for it with respect to
the attribute so declared.

The question of synchronization has to be settled variable by variable, or
more correctly, instance by instance. If a class declares four attributes,
all 'final', then the instances to which those variables point are
thread-safe. If later modification adds a fifth, non-'final' attribute, then
that attribute will require explicit synchronization. This is true
regardless. If a class declares four non-'final' attributes, then adds a
fifth one also not 'final', then that fifth attribute will require explicit
synchronization. A implies B; not-A implies B; therefore B.

If one gets used to treating an entire object as immutable because all its
attributes are 'final', then adds a non-'final' attribute, they've got to be
responsible for all client code that used to depend on immutability. That's
the breaks.

Even though perhaps explicit synchronization from day one would make such a
refactoring easier, it remains true that use of 'final' to make an object
read-only, therefore thread safe, is easier than explicit synchronization.
You'd have the world go through more effort on immutable objects in order to
protect them against a rare, possible, erroneous refactoring effort in the future.

In the real world as it really is right now, where Java has the keyword
'final' with its associated semantics, one has the convenience of making all
an object's attributes read-only and thread-safe, with all the safety,
efficiency and convenience that guarantees, weighed against the need to be
responsible if refactoring the class to violate that promise. That's the
tradeoff now, in Java as it is.

I'll live with that.

 

[Lew]

Final also has implications for the memory model and potentially affects
the generated code (since JSR-133). If you want to construct an
immutable object which will be passed between threads, then you should
use final on all the fields. Java then guarantees that, on completion of
the constructor, all threads will see the same values for those fields.

Just to clear up something all of us, including me, are eliding, 'final' by
itself doesn't guarantee read-onliness nor thread safety in all cases.

Not thread safe:
public class Foo
{
private final List <Bar> bars = buildBars();
private List <Bar> buildBars()
{
List <Bar> ret = new ArrayList <Bar> ();
// add elements to ret
return ret;
}
...
}

Thread safe:
public class Foo
{
private final List <Bar> bars =
Collections.unmodifiableList( buildBars() );
...
}

This caveat I believe to have been understood in this discussion, but let's
make it explicit this once just to be sure. So now when we talk about 'final'
guaranteeing read-onliness and thread safety, we know that we mean we also
take care of deep immutability with it.

 

[Lew]

Two words:

- @Immutable (from JCIP fame)

As proposed, '@Immutable' does not guarantee immutability. One will still
need to use 'final' on all the class's fields.

- FindBugs

Which complains if you forget to use 'final'.

Both these things leave the programmer responsible for using 'final' on
"everything possible" (really, only on "everything appropriate"). Neither
resolves the question of whether 'final' represents "code clutter". (Of
course, it isn't code clutter at all.)

 

[coffeymex]

Just to clear up something all of us, including me, are eliding, 'final' by
itself doesn't guarantee read-onliness nor thread safety in all cases.

Not thread safe:
  public class Foo
  {
    private final List <Bar> bars = buildBars();
    private List <Bar> buildBars()
    {
      List <Bar> ret = new ArrayList <Bar> ();
      // add elements to ret
      return ret;
    }
    ...
  }

You'd presumably agree that this is safely *published*, though?
Although it
obviously doesn't guarantee read-only access, it would in practice be
safe (though bad program design) if either (a) all accessing threads
explicitly synchronized on the list, or (b) all accesses were reads.

Neil

 

[Lew]

You'd presumably agree that this is safely *published*, though?

I don't understand what you mean by this. What I would say is that only the
initial values in the List are safely published. Future changes to its
contents might not be.

Although it
obviously doesn't guarantee read-only access, it would in practice be
safe (though bad program design) if either (a) all accessing threads
explicitly synchronized on the list, or (b) all accesses were reads.

Well, yes, but that was my very point. Simply declaring a mutable object
reference as 'final' is not itself enough to guarantee thread safety; you need
further synchronization or to guarantee that all accesses are reads.

I showed the latter upthread by wrapping the List in a
'Collections.unmodifiableList()' call.

 

[Albert]

Roedy Green a écrit :

Lets say you noticed that your constructor was getting overweight, and
decided to split it into more manageable chunks.

All of a sudden all your finals on the instance variable become
ERRORS!!!

The is correct because in THEORY someone later could call one of your
dependent methods.

So you remove your finals.

This is NOT a good thing.

It makes it look as though these are NOT final to other programmers.

You take off your safety wheels protecting you from duplicate
assignment or inadvertent assignment.

This usually happens when you are shuffling code around. You get some
dups.

I have two ideas to ameliorate the situation.

1. an @once annotation that says this static method can be run only
once, or this instance method can be invoked only once per object
instantiation. It would be backed by a weak transparent assert that
would be turned off when running in production. The compiler would
use the annotation to allow extra finals.

Such an annotation would be a great as general documentation and as
part of as assert mechanism.

2. IDEs should have a shuffle mode/refactor mode cut/paste. The idea
is you can move code around but not destroy or duplicate it.
You can create new code. This idea needs to be fleshed out. The
problem with any manual refactoring is you randomly lose a line of
code somewhere or duplicate one.

No need for such features, just arrange your code to put all
intermediate calculus in some utility method wich return the value of
the final field.