On 2009-03-17, sebb <seb...@gmail.com> wrote:

> On 17/03/2009, Christian Grobmeier <grobme...@gmail.com> wrote:
>> Hi all,

>>  just made a quick fix for SANDBOX-284:
>>  https://issues.apache.org/jira/browse/SANDBOX-284

>>  I could not test it with a general working testcase, since this is a
>>  platform problem and i have only OSX.

I have written some rudimentary tests that pass on Windows.

>>  Before I do this, this brings me to a question of codestyle. I prefer
>>  to do quite less in the constructor. TarArchiveEntry does quite much,
>>  and i think this is necessary in this case.

I agree that it seems to be necessary.  Note that the different
constructors behaved differently when it came to modifiying the name -
the reported bug only applied to the File-arg constructor which meant

new TarArchiveEntry("C:\\")

worked and 

new TarArchiveEntry(new File("C:\\"))

did not - and using absolute path names in either constructor lead to
different results.  I've fixed this at the same time (as well as the
setName method).

> Also, any instance fields that are mutable mean that synch. is likely
> to be needed to ensure that the class is thread-safe. Immutable
> classes are always thread-safe, and easier to test because they only
> have one state.

Many or TarArchiveEntry's fields are mutable and need to be for the
different use-cases.  They could be final for entries just read from
an archive but likely need to be mutable when you construct a entry to
be written (or it needs a big constructor with many many arguments).

Still the class is a bit strange in that it uses StringBuffers to
store names that are either completely overwritten or kept immutable
(i.e. the name is never modified internally - it can be set to a
completely different name, though).

> However, if the class is not intended to be thread-safe, then of
> course the mutable fields are not a problem.

It doesn't look as if it ever was intended to be thread safe.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to