[Studio] Cleanup...

[Emmanuel Lécharny <el...@gmail.com>]

Hi guys,

yesterday, I went through the code looking for places where 'instanceof'
was used. I found two typical improvements that could be applied :

o useless check on x != null  like in : if ( ( x != null ) && ( x
instanceof X ) )
o useless check of x == null like in : if ( ( x == null ) || !( x
instanceof X ) )

instanceof is guaranteed to return false if x is null.

That's just a itch I wanted to scracth yesterday.

At this point, I think that adding checkstyle in Studio would most
certainly be a nit of a pain (I suspect that we will get thousands of
failures ;-) but this might be good to have. Stefan did a great job with
the API, and I thinkw e should benefit from what he did in the other
projects, including Studio.

The real problem with Studio is that a lot of its code is quite old
(keep in mind it started around 2005 !!!) and there is a lot of
discrepancies between older code and ore recent one.

Thoughts ?

Re: [Studio] Cleanup...

[Emmanuel Lécharny <el...@gmail.com>]

Le 28/07/15 19:54, Emmanuel Lécharny a écrit :
>
>> Beside checkstyle we should also add PMD check, you already started to
>> cleanup some issues.
> PMD is a bit a PITA. There are many so called violations that are just
> plain BS, and many others that are going to push us to rewrite the whole
> code...
>
> This wil need some serious tuning.

FTR, I ran PMD on one plugin (aciitemeditor) this morning before
breakfast, it gave me 1550 lines of violations...

Out of which we have :

288 times : Parameter 'xxx' is not assigned and could be declared final
61 times : Comment is too large: Too many lines (this is mainly because
of the AL 2.0 license header)

etc...

in any case, there will be hundreds of things to fix. Perfect when you
have long insomnias or when at in the smaller room of your appartment ;-)

Re: [Studio] Cleanup...

[Emmanuel Lécharny <el...@gmail.com>]

Le 28/07/15 18:35, Stefan Seelmann a écrit :
> On 07/28/2015 10:28 AM, Emmanuel Lécharny wrote:
>
>> At this point, I think that adding checkstyle in Studio would most
>> certainly be a nit of a pain (I suspect that we will get thousands of
>> failures ;-) but this might be good to have. Stefan did a great job with
>> the API, and I thinkw e should benefit from what he did in the other
>> projects, including Studio.
> Yes, the plan is to add the checkstyle checks also to other projects.
> But I choosed the easiest first :).

Good choice !

>
> Beside checkstyle we should also add PMD check, you already started to
> cleanup some issues.

PMD is a bit a PITA. There are many so called violations that are just
plain BS, and many others that are going to push us to rewrite the whole
code...

This wil need some serious tuning.
>
> Both, checkstyle check and PMD check run very fast during normal build
> so it doesn't slow down the build much.
>
> Findbugs is another beast as it does a 2nd compile and increases build
> time considerably, I'd not add it to the normal build, either only run
> it on Jenkins, or just rely on Sonar (my preference).

Mine, too. But with the build we have currently, this is a bit hard. The
best would be to merge the two calls to mvn into one (ie, merging the
mvn -f pom-first.xml and the mvn pom.xml into one single pom)


>
>> The real problem with Studio is that a lot of its code is quite old
>> (keep in mind it started around 2005 !!!) and there is a lot of
>> discrepancies between older code and ore recent one.
> Well, ApacheDS is even older, so no excuse. But you are right, lot of
> the code was never updated to Java5 features. Part of bad code is due to
> Eclipse APIs. Even worse is lack of unit tests.

Ahhh, unit tests in studio. I know what you are talking about, I felt
the pain when adding some tests for the OpenLDAP plugins... Not easy.

Re: [Studio] Cleanup...

[Stefan Seelmann <ma...@stefan-seelmann.de>]

On 07/28/2015 10:28 AM, Emmanuel Lécharny wrote:
> Hi guys,
> 
> yesterday, I went through the code looking for places where 'instanceof'
> was used. I found two typical improvements that could be applied :
> 
> o useless check on x != null  like in : if ( ( x != null ) && ( x
> instanceof X ) )
> o useless check of x == null like in : if ( ( x == null ) || !( x
> instanceof X ) )
> 
> instanceof is guaranteed to return false if x is null.
> 
> That's just a itch I wanted to scracth yesterday.

Thanks for that.

> At this point, I think that adding checkstyle in Studio would most
> certainly be a nit of a pain (I suspect that we will get thousands of
> failures ;-) but this might be good to have. Stefan did a great job with
> the API, and I thinkw e should benefit from what he did in the other
> projects, including Studio.

Yes, the plan is to add the checkstyle checks also to other projects.
But I choosed the easiest first :).

Beside checkstyle we should also add PMD check, you already started to
cleanup some issues.

Both, checkstyle check and PMD check run very fast during normal build
so it doesn't slow down the build much.

Findbugs is another beast as it does a 2nd compile and increases build
time considerably, I'd not add it to the normal build, either only run
it on Jenkins, or just rely on Sonar (my preference).

> The real problem with Studio is that a lot of its code is quite old
> (keep in mind it started around 2005 !!!) and there is a lot of
> discrepancies between older code and ore recent one.

Well, ApacheDS is even older, so no excuse. But you are right, lot of
the code was never updated to Java5 features. Part of bad code is due to
Eclipse APIs. Even worse is lack of unit tests.

Kind Regards,
Stefan