You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@camel.apache.org by bvahdat <ba...@swissonline.ch> on 2011/12/13 15:26:06 UTC

[DISCUSS] Trunk-Code-Cleanup

[DISCUSS] Trunk-Cleanup

Hi Devs,

As Claus has already once stated [1] thanks to my own professionalism I
intend to take over the trunk code-clean-up activity :-)

This task (at least to the main extend) can be of course achieved in an
(almost) automated manner (*even* eclipse can do that!), which is through
the "Clean Up..." pop-up-menu. But before digging into it I want to get your
"taste" about what YOU expect to be in there and what not, so here my
proposal:

1- Add of the missing Annotations for @Deprecated & @Override:
As per Java Tiger, other than Javadoc @deprecated tag there's also the
annotation @Deprecated, good to be used because of the reasons mentioned in
[2].Also as per Java-Mustang, the semantics of the @Override annotation has
been extended to not only mean "implements" but also "overrides", so that 

new Processor(){
    public void process(Exchange exchange) {
    ...
    }
}

becomes:

new Processor(){
    @Override
    public void process(Exchange exchange) {
    ...
    }
}

or:
return new RouteBuilder() {
    public void configure() {
    ...
    }
};

Becomes:
return new RouteBuilder() {
    @Override
    public void configure() {
    ...
    }
};

Do you think this is an overkill?

2- Add of serialVersionUID with default to 1L by all those classes which
claim to be Serializable (like CamelAuthorizationException which has not
serialVersionUID), as per spec. letting the JVM to calculate the value at
runtime is not the best practice (for good reasons). Also by all other camel
serializable classes which already have that serialVersionUID change the
value to 1L. Any API/Field/Method serialization-breaking-change a Commiter
does on those classes should also include a count-up of that field (1L -> 2L
or 7L->8L).
 
Example:

from:
public class CamelAuthorizationException extends CamelExchangeException {
    private final String policyId;
...

to:
public class CamelAuthorizationException extends CamelExchangeException {
    private static final long serialVersionUID = 1L;
    private final String policyId;
...

3- Remove unused imports: I think all of us already agree on this.

4- Remove unnecessary casts
from:
List<MBeanServer> servers =
(List<MBeanServer>)MBeanServerFactory.findMBeanServer(null);

to:
List<MBeanServer> servers = MBeanServerFactory.findMBeanServer(null);

Even here, I assume we all do agree on this.

5- Remove trailing whitespace on *all* lines, even on the empty ones! Would
we maybe want to also extend the checkstyle rules to bawl on that as well?

6- Remove of all unused *private* members: Types, Constructors, Fields and
Methods! We maybe want to be careful about this when the Reflection-API
usage in Camel comes into the play.

7- Remove unnecessary $NON-NLS$ tags: just hit 2 of them in [2] & [3] see
also [4] for it's semantics in eclipse

Applying the clean-up rules I mentioned above on my workspace, I see
currently around 343 out-going-source-changes.

I would of course run a full install including all unit-test-run &
checkstyle-checks to garantie no side effect. BTW, my intention is *not* to
force this into the 2.9.0 final release as this would be already too late
for that sun-shine-release.

Another *important* clean-up activity would be of course the generics-stuff
which is completely another story I may also get some time to take care of
that in the future (for example look at the ProcessorDefinition class).
However there I don't see any automatable approach which I could apply :-(
So maybe I have to dig into [5] again to get an idea about all those already
well-known generics magics ;-))

[1]
http://camel.465427.n5.nabble.com/HEADS-UP-Adjustments-to-ExecutorServiceManager-on-trunk-tp4693698p4715407.html
[2]
http://docs.oracle.com/javase/1.5.0/docs/guide/javadoc/deprecation/deprecation.html
[3]
https://svn.apache.org/repos/asf/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/osgi/tracker/AbstractTracked.java
[4]
https://svn.apache.org/repos/asf/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/osgi/tracker/BundleTracker.java
[5] http://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5071594.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Babak Vahdat <ba...@swissonline.ch>.

Thanks a lot beforehand for the hint! I assume you mean the lines like:

// must do this ugly cast to avoid compiler error on AIX/HP-UX
ProcessorDefinition<?> defn = (ProcessorDefinition<?>) this;

which I carefully tried to skip by all the provided patches, however IMHO
*if* we reach the level of an appropriate usage of the java generics then
those casts will become obsolete and there will be no need more to help
compilers understand what we actually intend to do.

I would of course test any such changes along those lines on AIX/HP-UX as
well *before* applying.

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5158096.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Claus Ibsen <cl...@gmail.com>.

On Thu, Jan 19, 2012 at 4:43 PM, Babak Vahdat
<ba...@swissonline.ch> wrote:
> Hi,
>
> Through [1] & [2] the issues being mentioned by this thread have been
> already resolved. Before beginning of this activity
> I used to see 1991 compiler warnings, however right now only 310 are left,
> mostly because of the raw type references to the
> generic type ProcessorDefinition whose fixing would be not that trivial but
> hope to fix it as well at sometimes later. There're
> also two remaining tasks which we should better postpone, for which I've
> opened [3] to track by a Camel 3.x release.
>

You should be really careful changing generics in the model package
(eg the xxxDefinition) stuff.

As compilers on AIX, HP-UX, Solairs and other such platforms may not
compile the source code.
A few spots we got some special type casts to work around those platforms.

However their JDK may get better over time. But we may risk issues for
these people.



> That would be great if we could keep on that quality as otherwise in just
> couple of months we would be again there where we began
> with this! My thanks also to Claus & Hadrian for assisting me on the JIRA
> tickets at the time I couldn't touch the code by myself.
>
> [1] https://issues.apache.org/jira/browse/CAMEL-4796
> [2] https://issues.apache.org/jira/browse/CAMEL-4899
> [3] https://issues.apache.org/jira/browse/CAMEL-4921
>
> Babak
>
> --
> View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5158015.html
> Sent from the Camel Development mailing list archive at Nabble.com.



-- 
Claus Ibsen
-----------------
FuseSource
Email: cibsen@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Babak Vahdat <ba...@swissonline.ch>.

Hi,

Through [1] & [2] the issues being mentioned by this thread have been
already resolved. Before beginning of this activity
I used to see 1991 compiler warnings, however right now only 310 are left,
mostly because of the raw type references to the
generic type ProcessorDefinition whose fixing would be not that trivial but
hope to fix it as well at sometimes later. There're
also two remaining tasks which we should better postpone, for which I've
opened [3] to track by a Camel 3.x release.

That would be great if we could keep on that quality as otherwise in just
couple of months we would be again there where we began
with this! My thanks also to Claus & Hadrian for assisting me on the JIRA
tickets at the time I couldn't touch the code by myself.

[1] https://issues.apache.org/jira/browse/CAMEL-4796
[2] https://issues.apache.org/jira/browse/CAMEL-4899
[3] https://issues.apache.org/jira/browse/CAMEL-4921

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5158015.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Bilgin Ibryam <bi...@gmail.com>.

On 12 January 2012 15:57, Hadrian Zbarcea <hz...@gmail.com> wrote:
> I realized and changed my comment. In my defense (I know I have none) both
> your names start with a 'B' are roughly the same lenght (5-6 chars) and you
> Babak threw so many small patches at us lately that you made my head spin.
> I'll get on the rawtypes patch now.
>
> Bilgin, I don't know how I can correct my mistake, but given the chance I
> will. Maybe supply the beer when we'll meet f2f?

I can accept f2f beers :)

>
> Sorry again,
Nothing to worry about.

Bilgin

> Hadrian
>
>
> On 01/12/2012 10:34 AM, Babak Vahdat wrote:
>>
>> Hi Hadrian,
>>
>> as you've already realized the patch [1] was provided by Bilgin and not me
>> (you welcome :-)).
>>
>> However I would have also something to be applied into the trunk as well
>> [2]
>> just in case you've got some spare time for verification and applying it.
>>
>> Thanks!
>>
>> [1] http://svn.apache.org/viewvc?view=revision&revision=1230571
>> [2] https://issues.apache.org/jira/browse/CAMEL-4796
>>
>> Babak
>>
>>
>> --
>> View this message in context:
>> http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5140218.html
>> Sent from the Camel Development mailing list archive at Nabble.com.
>
>
> --
> Hadrian Zbarcea
> Principal Software Architect
> Talend, Inc
> http://coders.talend.com/
> http://camelbot.blogspot.com/

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Hadrian Zbarcea <hz...@gmail.com>.

I realized and changed my comment. In my defense (I know I have none) 
both your names start with a 'B' are roughly the same lenght (5-6 chars) 
and you Babak threw so many small patches at us lately that you made my 
head spin. I'll get on the rawtypes patch now.

Bilgin, I don't know how I can correct my mistake, but given the chance 
I will. Maybe supply the beer when we'll meet f2f?

Sorry again,
Hadrian

On 01/12/2012 10:34 AM, Babak Vahdat wrote:
> Hi Hadrian,
>
> as you've already realized the patch [1] was provided by Bilgin and not me
> (you welcome :-)).
>
> However I would have also something to be applied into the trunk as well [2]
> just in case you've got some spare time for verification and applying it.
>
> Thanks!
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=1230571
> [2] https://issues.apache.org/jira/browse/CAMEL-4796
>
> Babak
>
>
> --
> View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5140218.html
> Sent from the Camel Development mailing list archive at Nabble.com.

-- 
Hadrian Zbarcea
Principal Software Architect
Talend, Inc
http://coders.talend.com/
http://camelbot.blogspot.com/

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Babak Vahdat <ba...@swissonline.ch>.

Hi Hadrian,

as you've already realized the patch [1] was provided by Bilgin and not me
(you welcome :-)).

However I would have also something to be applied into the trunk as well [2]
just in case you've got some spare time for verification and applying it.

Thanks!

[1] http://svn.apache.org/viewvc?view=revision&revision=1230571
[2] https://issues.apache.org/jira/browse/CAMEL-4796

Babak


--
View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5140218.html
Sent from the Camel Development mailing list archive at Nabble.com.

Re: [DISCUSS] Trunk-Code-Cleanup

Posted by Babak Vahdat <ba...@swissonline.ch>.

Hi

FYI I've extended the scope of [1] to also include:

- Avoidance of the raw type declarations by the generified classes *as much
as possible*

This will be a *manual* Refactoring step as I'm not aware of any automated
way for doing it.

[1] https://issues.apache.org/jira/browse/CAMEL-4796

Babak

--
View this message in context: http://camel.465427.n5.nabble.com/DISCUSS-Trunk-Code-Cleanup-tp5071594p5123173.html
Sent from the Camel Development mailing list archive at Nabble.com.