[Patch] ChangeLogParser - hiding field

[Kev Jackson <ke...@it.fts-vn.com>]

removed extra LOC from javadoc whilst I was there - no warnings about 
the whole cvslib now!! (have to make teh compiler even more strict ;) )

Re: [Patch] ChangeLogParser - hiding field

[Jesse Glick <je...@sun.com>]

Kev Jackson wrote:
>> When writing a new class, start with
>>
>> static final class Foo {}
> 
> I've read many times that declaring a class as final is a farily 
> dangerous thing to do with code.  Once you've made a class final people 
> are stuck with what you decided at a particular moment was the right way 
> to do things.  Can you truly know that the class will never have to 
> change in the future?  That's a tough call to make.

Well, think of it from the other side - if you *don't* make it final, 
you're essentially inviting anyone to subclass it for any reason, and 
promising that whatever they do, any changes you make to the class will 
work with their subclass. Do you know how they're going to subclass it? 
Which methods are they going to override, exactly, in which combination? 
Are any of the overridable methods called from the constructor? If you 
use a synchronization monitor, are they going to use the same one? If 
you decide to deprecate one method in favor of a new one you add, how 
will the class behave if subclasses are still overriding the old one? Is 
the class comparable, or does it use equals(), is it clonable or 
serializable or externalizable? There's a surprisingly large number of 
ways you can get into a royal mess as APIs evolve, at least in my 
experience, and nonfinal classes are much harder to reason about.

An example: javax.swing.tree.TreePath is nonfinal. The authors of the 
class were polite enough to even provide a no-arg subclass constructor 
and things like this. So this sounds cool - you can make a special 
TreePath that is specialized to unusual JTree storages! Well, 
unfortunately, no - it turns out that it is not possible to subclass it 
usefully, since you can't get rid of some of the fields in the 
superclass, and you can't make equals() work correctly with 
non-subclassed TreePath's, and in fact the Javadoc for the class lies 
about which methods you really need to override together. But you would 
never notice this kind of thing looking at the class casually.

On the other hand, if you make a class final, and it turns out later 
that there is a good reason to subclass it, and it seems safe and 
effective, you can remove the final modifier then, without breaking 
anything.

> But here I feel like a complete novice, practically every day I'm 
> learning something new about a particular idiosyncratic aspect of Java - 
> which is great!

Oh yes... I still feel pretty clueless about a lot of Ant internals - 
e.g. macrodefs! - so I wind up posting about something I can recognize. :-)

-J.

-- 
Jesse Glick <ma...@sun.com> x22801
NetBeans, Open APIs  <http://www.netbeans.org/>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org

Re: [Patch] ChangeLogParser - hiding field

[Kev Jackson <ke...@it.fts-vn.com>]

Jesse Glick wrote:

> Just jumping in here... :-)
>
Feel free ;)

> kj wrote:
>
>>>> -class ChangeLogParser {
>>>> +public class ChangeLogParser {
>>>
>>
>> No really, just habit - company standards where I've worked in past etc.
>
>
> Did your company produce Java components with publicly visible APIs 
> they were committed to preserving indefinitely? Seriously, making and 
> keeping a decent Java API is *hard* and almost no "good style for 
> programmers" books give you workable recommendations. I've been 
> working on NetBeans core APIs for several years and often regret some 
> method that seemed harmless four years ago and was made public without 
> careful planning. Not to mention the mess that is javax.swing.** 
> Javadoc that it is too late to fix.

No, I've never worked on a true API project before.

> Generally, assume everything should be private and final unless you 
> know how it should be called or overridden. In practice, static 
> methods are usually not much of a burden to maintain public; instance 
> methods and constructors more so; and nonfinal classes are very hard 
> to maintain compatibly, since there is a huge amount of runtime 
> semantics which subclasses typically rely on and which has to be 
> documented explicitly and probably can never be changed.
>
Good rule of thumb.

>> I suppose it could expose the class more than was intended, but again
>> the only real rationale for this was that I was looking at the class and
>> it's completely automatic (for me) to specify an exact access modifier
>> for everything as shows your exact intentions.
>
>
> For a top-level class, lack of a written access modifier *is* the one 
> and only way to specify that it should be package-private, which is 
> the most restrictive mode for it. When writing a new class, start with
>
> static final class Foo {}
>

I've read many times that declaring a class as final is a farily 
dangerous thing to do with code.  Once you've made a class final people 
are stuck with what you decided at a particular moment was the right way 
to do things.  Can you truly know that the class will never have to 
change in the future?  That's a tough call to make.

> or for a nested class
>
> private static final class Foo {}
>
> and go from there. It is just unfortunate that the Java language has 
> unsafe defaults for these modifiers.
>
Thanks for the pointers.  As an aside, I'm very happy to have decided to 
get involved (no matter how little) in an open source project.  The 
amount of extra knowlegde that I'm gaining from just reading the mailing 
list and bug-reports and scanning the code for possible fixes etc is 
immense.  I'm not by any means a Java guru, but I do consider myself to 
be pretty good at Java - and within my company, I almost certainly am 
(aren't all developers ego-maniacs?)

But here I feel like a complete novice, practically every day I'm 
learning something new about a particular idiosyncratic aspect of Java - 
which is great!  I'm trying to convice one of my ex-colleagues to get 
involved on one of the jakarta projects (he's thinking of James as he 
currently uses it), and I can definitely say that one of the benefits is 
the amount that you learn (or can learn) from other developers.

So thanks for your thoughts, they've given me much to think about.
Kev

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org

Re: [Patch] ChangeLogParser - hiding field

[Jesse Glick <je...@sun.com>]

Just jumping in here... :-)

kj wrote:
>>>-class ChangeLogParser {
>>>+public class ChangeLogParser {
> 
> No really, just habit - company standards where I've worked in past etc.

Did your company produce Java components with publicly visible APIs they 
were committed to preserving indefinitely? Seriously, making and keeping 
a decent Java API is *hard* and almost no "good style for programmers" 
books give you workable recommendations. I've been working on NetBeans 
core APIs for several years and often regret some method that seemed 
harmless four years ago and was made public without careful planning. 
Not to mention the mess that is javax.swing.** Javadoc that it is too 
late to fix.

It is a bit unfortunate that the Ant team never published guidelines 
separating real APIs (intended for task authors, embedders, listener 
creators, etc.) from implementation (e.g. task classes). So now we are 
stuck with the situation that anything which is technically accessible 
in the Ant codebase is considered a full API automatically.

Generally, assume everything should be private and final unless you know 
how it should be called or overridden. In practice, static methods are 
usually not much of a burden to maintain public; instance methods and 
constructors more so; and nonfinal classes are very hard to maintain 
compatibly, since there is a huge amount of runtime semantics which 
subclasses typically rely on and which has to be documented explicitly 
and probably can never be changed.

> I suppose it could expose the class more than was intended, but again
> the only real rationale for this was that I was looking at the class and
> it's completely automatic (for me) to specify an exact access modifier
> for everything as shows your exact intentions.

For a top-level class, lack of a written access modifier *is* the one 
and only way to specify that it should be package-private, which is the 
most restrictive mode for it. When writing a new class, start with

static final class Foo {}

or for a nested class

private static final class Foo {}

and go from there. It is just unfortunate that the Java language has 
unsafe defaults for these modifiers.

-J.

-- 
Jesse Glick <ma...@sun.com> x22801
NetBeans, Open APIs  <http://www.netbeans.org/>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org

Re: [Patch] ChangeLogParser - hiding field

[Stefan Bodewig <bo...@apache.org>]

On Fri, 17 Dec 2004, kj <ke...@it.fts-vn.com> wrote:

> No really, just habit - company standards where I've worked in past
> etc.  Changing it can't break BWC as it can only make the class more
> visible not less visible.

I agree, but it adds BWC problems for the future.  I.e. one more
classes with fixed method signatures.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org

Re: [Patch] ChangeLogParser - hiding field

[kj <ke...@it.fts-vn.com>]

On Fri, 2004-12-17 at 12:29 +0100, Stefan Bodewig wrote:
> On Fri, 17 Dec 2004, Kev Jackson <ke...@it.fts-vn.com> wrote:
> 
> > -class ChangeLogParser {
> > +public class ChangeLogParser {
> 
> why?
why not? ;)

No really, just habit - company standards where I've worked in past etc.
Changing it can't break BWC as it can only make the class more visible
not less visible.

I suppose it could expose the class more than was intended, but again
the only real rationale for this was that I was looking at the class and
it's completely automatic (for me) to specify an exact access modifier
for everything as shows your exact intentions.

I can understand if there's a problem with changing it and it breaks
stuff, I won't do it again, but I basically was on autopilot for that
particular change.

Also to be fair only a few classes are defined without any modifier - I
assumed this was a historical oddity and was breaking style.

Rambling now ... never mind

Kev


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org

Re: [Patch] ChangeLogParser - hiding field

[Stefan Bodewig <bo...@apache.org>]

On Fri, 17 Dec 2004, Kev Jackson <ke...@it.fts-vn.com> wrote:

> -class ChangeLogParser {
> +public class ChangeLogParser {

why?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org