ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Bodewig <bode...@apache.org>
Subject Re: Opportunities for cohesion improvement and refatoring.
Date Fri, 21 Jul 2017 09:40:10 GMT
Hi

thank you, Gintautas.

Yes, using a tool to verify the API hasn't changed will probably
help. Over in Commons we run this as a regular part of the release
process - it is even more important for things that are meant to be
re-usable components, of course.

I'm afraid that won't be enough, though.

Let me pick a silly example

in Project we currently have

    public Project createSubProject() {
        Project subProject = null;
        try {
            subProject = (getClass().newInstance());
        } catch (final Exception e) {
            subProject = new Project();
        }
        initSubProject(subProject);
        return subProject;
    }

and initSubProject is public as well.

A subclass may override initSubProject and rely on the method being
called by createSubProject. If you refactor createSubProject in a way
that it no longer calls initSubProject you are going to break the
subclass. And I don't think the tools are going to tell you as long as
you keep all existing methods.

This is what I meant with "have to ensure you keep the existing who
calls which method semantics".

Cheers

        Stefan


On 2017-07-20, Gintautas Grigelionis wrote:

> I looked at Project proposal [1].
> I would suggest running the refactored Ant through japicmp [2] or revapi
> [3] and examining the binary incompatibilities.

> Gintas

> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=61305
> [2] http://siom79.github.io/japicmp/
> [3] http://revapi.org/

> 2017-07-20 16:41 GMT+02:00 Stefan Bodewig <bodewig@apache.org>:

>> Welcome João Paulo

>> On 2017-07-20, João Paulo Lemes Machado wrote:

>>> I was analyzing the modularization of some classes of Ant, and  I
>>> identified some opportunities for cohesion improvement in the following
>>> classes:

>>> Javac
>>> Javadoc
>>> FTPTask
>>> FileUtils
>>> AbstractFileSet

>> Similar to what I said about Project in the bugzilla issue you created
>> all these classes are part of Ant's public API and need to be treated
>> with care.

>> Ant has been around for more than fifteen years and an eco system of
>> extensions has ground around it. This is something that forces us to
>> be extra careful with refactoring.

>> Of the classes listed above Javadoc and FTPTask are unlikely to have
>> seen subclasses, Javac may have. FileUtils is unlikely to have seen
>> subclasses as a lot of its code is inside static methods or is
>> accessed via a quasi-singleton.

>> Still, when refactoring non-static public/protected methods you really
>> have to ensure you keep the existing who calls which method semantics
>> that subclasses may rely on. AbstractFileSet is an extremely dangerous
>> one, as it has certainly seen a lot of extensions outside of our
>> control.

>>> TarEntry

>> Is a special case to me. Ant's tar, zip and bzip2 packages have seeded
>> Commons Compress and from time to time I try to backport changes from
>> Compress to Ant - usually only the bugfixes. This may become more
>> difficult when the code bases start to deviate.

>> I'd be interested in hearing what kind of changes you'd like to make,
>> but please take a look at
>> https://git-wip-us.apache.org/repos/asf?p=commons-compress.
>> git;a=blob;f=src/main/java/org/apache/commons/compress/
>> archivers/tar/TarArchiveEntry.java
>> ?

>> The class has undergone several changes that haven't been reflected
>> back, maybe it is mostly an issue of backporting those changes?

>> Cheers

>>         Stefan

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



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


Mime
View raw message