On Tue, Aug 18, 2009 at 7:48 AM, sebb <seb...@gmail.com> wrote:
> On 18/08/2009, Jörg Schaible <joerg.schai...@gmx.de> wrote:
> > Hi Sebb,
> >
> > sebb wrote at Dienstag, 18. August 2009 14:48:
> >
> >
> > >> +
> > >> + /**
> > >> + * {...@inheritdoc}
> > >> + */
> > >> + public Object clone() {
> > >> + FileSystemOptions clone = new FileSystemOptions();
> > >> + clone.options = new TreeMap(options);
> > >> + return clone;
> > >> + }
> > >
> > > This clone() does not call super.clone() so won't work properly for
> > > sub-classes.
> >
> >
> > I know, but since the key of the map is a private class anyway, it does
> not
> > make too much sense to derive from FileSystemOptions anyway ... don't
> you
> > think?
>
> In that case, you could make the class final.
>
> In any case, it would be helpful to document that the clone() method
> is not written to be sub-classable.
>
>
>
I actually don't agree that FileSystemOptions shouldn't be subclassed. I
have always disliked immensely having to manipulate FileSystemOptions using
a ConfigBuilder class. From personal experience, I've found working with it
to be awkward and brittle. I would much prefer to have each provider
subclass FileSystemOptions and provide the getters and setters there. Then,
at least, you could do an instanceof on the FileSystemOptions and determine
what options are actually supposed to be there.
Ralph
