[josm-dev] Jumbo Patch

[Gervase Markham]

Frederik Ramm wrote:
> The problem is that if I am new to the code then I have to read it
> because somewhere inside all those accessors that don't do anything
> there might be the one accessor which actually triggers an action -
> well hidden among lots of, well, bloat.

Then as I said to Ulf, clearly it was a good idea to use accessors, 
because someone's been able to add that code without breaking all the 
users of the class.

> I do not consider this "readability". In fact, those accessors that
> don't do anything seem to me like decoys, trying to keep you from
> finding the one that does ;-)

Except that it's obvious when scanning code which accessors have extra 
code in and which don't. At least, it is if you have consistent coding 
standards.

> If you try and find your way through other people's code and you find
> a
> 
> blah.nodeList.clear();
> 
> then you know what happens without looking at blah's source code. If,
> instead, you have a 
> 
> blah.clearNodeList();
> 
> then, even if "clearNodeList" is a very trivial "nodeList.clear()",
> you still have to look inside Blah to know this. So you have made the
> whole thing more complex.

Quite the opposite. You know that the nodeList has been cleared, but you 
don't need to know anything about the underlying implementation if you 
don't want to. The nodeList could be a list, an array, or a red-black 
tree. It doesn't matter - all you know is it has a method called 
clear(), which clears it. (If it doesn't, that's bad programming 
whatever, unrelated to the encapsulation.)

> I know that in an ideal world the inside workings of Blah would be of
> no concern to you because everything was properly documented but I am
> quite sure that if JOSM ever reaches that point it will be dead,
> unable to keep up with the ever-evolving project it is attached to.

But if you write it right, the code _is_ the documentation. I agree that 
English documentation always lags - which means that the code should be, 
as far as possible, self-documenting.

When I see foo.nodeList.clear(), how do I know when it's safe to do 
that? What if some other part of the code is iterating over the nodeList 
at the time? Will I cause a crash?

If I see foo.clearNodeList(), I can be certain that the job of making 
sure it's safe to clear the list is the responsibility of the author of 
the class of which foo is an instance. So I can just call it and not worry.

> I am saying all this out of bitter experience working with "properly
> structured" Java code written by others. I am familiar enough with
> JOSM that "going private" near everywhere would not cause trouble for
> me, I just think that we'd actually make it *harder* for newcomers.

And I write out of bitter experience trying to maintain software which 
used a lot of public and global variables. It's an absolute misery. We 
couldn't change anything safely.

> The same is true for Imi's often-disputed liberal use of global static
> entities. I am sure that the same people telling me off for not being
> jubilant about private members would also say that statics are mostly
> evil. But they make many things really easy, and that's a good thing
> if you want people to participate, is it not?

It depends what you are trying to make easy. If you are trying to make 
it easy to make a small change to the code without causing accidental 
and hard-to-debug side effects elsewhere (which is what most newbies 
will be trying to do, as that's how people get into projects - making 
their own small change), then global static entities are a terrible idea.

Gerv