You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by Santiago Gala <sg...@hisitech.com> on 2003/03/03 11:41:55 UTC

Small steps towards a long term goal (code quality)

I am concerned about the overall state of interpackage dependencies in 
Jetspeed. As I have been saying in different posts, if we don't have a 
clear knowledge of the dependencies between our own packages and with 
third party projects, it would be difficult to assess work needed for 
PortletAPI support, release or maintain code, etc.

Recently, Tom Copeland has published a report on "unused imports" across 
jakarta projects in:

http://nagoya.apache.org/eyebrowse/ReadMsg?listName=general@jakarta.apache.org&msgNo=14537

Jetspeed is, currently, one of the projects with most unused imports. I 
wonder if someone using an IDE to develop with Jetspeed could easily 
send a patch which removes unused imports (without breaking the build, 
if possible).

I will eventually do it, but I need to install the proper tools I am 
*very* scarce of time currently.

This would be a very small step, but a path is always walked step by step.

The next one would be to use "deprecation=on" in the build, and ensure 
that no deprecated methods are used, and then delete them, or at least 
move them from "public" to protected or private.

We have *way too many* public methods in Jetspeed, and this raises the 
number of dependencies unnecesarily.

I think we need to control more tightly the code base if we want to 
succeed in the next months.

Any thoughts?

Regards,
      Santiago



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


Re: Small steps towards a long term goal (code quality)

Posted by Santiago Gala <sg...@hisitech.com>.
Ender KILICOGLU wrote:
> Sure I will look for this.. 
> But I tried one (I don't remember name but was not an ant task) which
> organized all import and all order or descriptions gone
> So I developed some own classes for those jobs. 
> 

Since I'm trying to minimize the impact to legibility of code and number 
of changed files, I would prefer a tool that would no reorder imports, 
just remove those not needed.

I'll take a look to your patch, though

Regards,
      Santiago


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


RE: Small steps towards a long term goal (code quality)

Posted by Ender KILICOGLU <en...@kilicoglu.nom.tr>.
Sure I will look for this.. 
But I tried one (I don't remember name but was not an ant task) which
organized all import and all order or descriptions gone
So I developed some own classes for those jobs. 

Ender


-----Original Message-----
From: Sami Leino [mailto:sami.leino@netorek.fi] 
Sent: Monday, March 03, 2003 5:42 PM
To: Jetspeed Developers List
Subject: Re: Small steps towards a long term goal (code quality)


> Jetspeed is, currently, one of the projects with most unused imports. 
> I wonder if someone using an IDE to develop with Jetspeed could easily

> send a patch which removes unused imports (without breaking the build,

> if possible).

I took a quick look at some (free) import cleanup tools. The most
promising of them seemed to be CleanImports version 2.0.0
(http://www.euronet.nl/users/tomb/cleanImports/), which is an ANT-based
import cleanup tool. I was able to set it up in less than five minutes,
and it seemed to remove the unused imports from the Jetspeed codebase
without breaking the build. However, this required making few minor
changes to the example configuration.

Here's the configuration I used:

        <taskdef classname="com.tombrus.cleanImports.ant.CleanImports"
                 classpath="/cleanImports.jar"
                 name="cleanimps"/>

        <!-- clean the imports -->
        <cleanimps srcdir="${src.dir}" classpathref="classpath">
            <cleanformat>
                <options  collapseAbove="99999" blankLines="1"
ambiguities="on"/>
                <import   comment="Java imports" regexp ="java\.*"/>
                <import   comment="Jetspeed imports"
package="org.apache.jetspeed"/>
                <import   comment="Torque imports"
package="org.apache.torque"/>
                <import   comment="Turbine imports"
package="org.apache.turbine"/>
            </cleanformat>
        </cleanimps>

After running the task, I re-compiled the codebase without any errors.
Before running the task, checkstyle reported 804 unused imports, and
after running the task, none. Here are the pros and cons I experienced
while evaluating the component:

Pros:

* ANT task (no need to set up an IDE for the job)
* Supports RegExp's in matching import clauses
* Allows grouping imports in the way the user wants
* Inserts warnings about ambiquous class names

Cons:

* Produces extraneous empty lines occasionally
* Some problems with ambiquous class names when 'collapseAbove' set to a
small value (like '3')
* Works only with JDK 1.4 (because of the RegExp package used)

My first impression of CleanImports tool is that it seems to be a good
candidate for the job. I haven't used this particular cleaner before
though, so I would like to hear second opinions on it from other
developers.

After taking a quick look at the resulting source files, I tried to
execute all unit-tests to verify that the code had not been broken.
Unfortunately, there were quite a few testcases that I couldn't
succesfully execute in the _unmodified_ source tree, so I can't tell you
if the cleanup operation caused any new errors (for example
ClassCastExceptions). Therefore, I cannot guarantee that the cleanup
operation did not break anything. My guess is that the codebase remained
intact.

I only took a look at those tools that are free and not tied to a
specific IDE. I guess there are very good commercial tools for the task,
but this one seemed to suit my needs. Comments about this (and other)
cleanup tools are welcome.

Regards,

Sami

-- 

Sami Leino
Software Developer, Netorek Oy, Turku, Finland
Email: sami@netorek.fi
Phone: +358 44 0140499

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


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


Re: Small steps towards a long term goal (code quality)

Posted by Sami Leino <sa...@netorek.fi>.
> Jetspeed is, currently, one of the projects with most unused imports. I
> wonder if someone using an IDE to develop with Jetspeed could easily
> send a patch which removes unused imports (without breaking the build,
> if possible).

I took a quick look at some (free) import cleanup tools. The most
promising of them seemed to be CleanImports version 2.0.0
(http://www.euronet.nl/users/tomb/cleanImports/), which is an ANT-based
import cleanup tool. I was able to set it up in less than five minutes,
and it seemed to remove the unused imports from the Jetspeed codebase
without breaking the build. However, this required making few minor
changes to the example configuration.

Here's the configuration I used:

        <taskdef classname="com.tombrus.cleanImports.ant.CleanImports"
                 classpath="/cleanImports.jar"
                 name="cleanimps"/>

        <!-- clean the imports -->
        <cleanimps srcdir="${src.dir}" classpathref="classpath">
            <cleanformat>
                <options  collapseAbove="99999" blankLines="1"
ambiguities="on"/>
                <import   comment="Java imports" regexp ="java\.*"/>
                <import   comment="Jetspeed imports"
package="org.apache.jetspeed"/>
                <import   comment="Torque imports"
package="org.apache.torque"/>
                <import   comment="Turbine imports"
package="org.apache.turbine"/>
            </cleanformat>
        </cleanimps>

After running the task, I re-compiled the codebase without any errors.
Before running the task, checkstyle reported 804 unused imports, and after
running the task, none. Here are the pros and cons I experienced while
evaluating the component:

Pros:

* ANT task (no need to set up an IDE for the job)
* Supports RegExp's in matching import clauses
* Allows grouping imports in the way the user wants
* Inserts warnings about ambiquous class names

Cons:

* Produces extraneous empty lines occasionally
* Some problems with ambiquous class names when 'collapseAbove' set to a
small value (like '3')
* Works only with JDK 1.4 (because of the RegExp package used)

My first impression of CleanImports tool is that it seems to be a good
candidate for the job. I haven't used this particular cleaner before
though, so I would like to hear second opinions on it from other
developers.

After taking a quick look at the resulting source files, I tried to
execute all unit-tests to verify that the code had not been broken.
Unfortunately, there were quite a few testcases that I couldn't
succesfully execute in the _unmodified_ source tree, so I can't tell you
if the cleanup operation caused any new errors (for example
ClassCastExceptions). Therefore, I cannot guarantee that the cleanup
operation did not break anything. My guess is that the codebase remained
intact.

I only took a look at those tools that are free and not tied to a specific
IDE. I guess there are very good commercial tools for the task, but this
one seemed to suit my needs. Comments about this (and other) cleanup tools
are welcome.

Regards,

Sami

-- 

Sami Leino
Software Developer, Netorek Oy, Turku, Finland
Email: sami@netorek.fi
Phone: +358 44 0140499

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


Re: Small steps towards a long term goal (code quality)

Posted by David Sean Taylor <da...@bluesunrise.com>.
On Monday, March 3, 2003, at 02:41  AM, Santiago Gala wrote:

> I am concerned about the overall state of interpackage dependencies in  
> Jetspeed. As I have been saying in different posts, if we don't have a  
> clear knowledge of the dependencies between our own packages and with  
> third party projects, it would be difficult to assess work needed for  
> PortletAPI support, release or maintain code, etc.
>
> Recently, Tom Copeland has published a report on "unused imports"  
> across jakarta projects in:
>
> http://nagoya.apache.org/eyebrowse/ 
> ReadMsg?listName=general@jakarta.apache.org&msgNo=14537
>
> Jetspeed is, currently, one of the projects with most unused imports.  
> I wonder if someone using an IDE to develop with Jetspeed could easily  
> send a patch which removes unused imports (without breaking the build,  
> if possible).
>
> I will eventually do it, but I need to install the proper tools I am  
> *very* scarce of time currently.
>
> This would be a very small step, but a path is always walked step by  
> step.
>
> The next one would be to use "deprecation=on" in the build, and ensure  
> that no deprecated methods are used, and then delete them, or at least  
> move them from "public" to protected or private.
>
> We have *way too many* public methods in Jetspeed, and this raises the  
> number of dependencies unnecesarily.
>
> I think we need to control more tightly the code base if we want to  
> succeed in the next months.
>
> Any thoughts?
>
Look into Maven.

Im not going to get involved in this one.
I've already starting cleaning up the code in the new code base, where  
you are welcome to help out.

--
David Sean Taylor
Bluesunrise Software
david@bluesunrise.com
+01 707 773-4646




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