mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julien Vermillard <jvermill...@gmail.com>
Subject Re: Sonar
Date Mon, 20 May 2013 09:06:58 GMT
Hi,
comments inline
On Mon, May 20, 2013 at 10:59 AM, Emmanuel Lécharny <elecharny@gmail.com> wrote:
> Le 5/19/13 11:18 PM, Raphaël Barazzutti a écrit :
>>
>> I'm currently working fixing some issues in the delimited module of codec
>> and the serialization modules.
>>
>> Currently some lines in my codes are generating warnings in Sonar.
>
> You are not alone here :) I fixed many warnings last saturday in the train.
>>
>> I'd like to know if there is a specific Sonar configuration file
>> recommended for the MINA project.
>
> I think so, Julien ?

I think it's running the dumb default config.

>>
>> And then I'd like to know what are your recommandations for a warning I
>> currently encounter quite often while playing with RawInt32 and VarInt is
>> the "'x' is a magic number" warning (by example when doing a shift of 8
>> bits to the left/right).
>>
>> In such a situations I see some different ways of handling that warning :
>> 1) do nothing, considering this warning as excessive
> In mst of the case, yes.
>
> for instance, I don't see why we should fix things like :
>
>                 return ((input.get() & 0xff) << 24) | ((input.get() & 0xff)
<< 16) | ((input.get() & 0xff) << 8)

I think this rule apply for dumb J2E business code, we are too low
level for that :)
I'm not going to create "FF = 0xff;" constant :)

>> 2) create a constants for all that cases
>
> I some case, that could be good :
>
>                 if ((input.get(3) & 0x80) != 0) {
>
> The '3' should remain, the 0x80 *could* be replaced by a constant :
>
> public statif final int HIGH_BIT_MASK = 0x80;
>

That could be nice, but it's not socking me, I read bit flipping code
fluently ;)

> but this is just a possibility.
>> 3) ask Sonar to ignore that line with a //nosonar
> No. Sonar can be re-configured, and I don't see any added value to
> inject such things in the code.
>
> One more thing :
>
> code like
>
>                 return ((input.get() & 0xff) << 24) | ((input.get() &
> 0xff) << 16) | ((input.get() & 0xff) << 8)
>                         | ((input.get() & 0xff));
>
> where input is a ByteBuffer can be replaced with :
>
>                 return input.getInt()
>
>
> (in RawInt32)
Yes and no, if you want to have a unsigned int :

long unsignedInt = input.getInt() &0xFFFFFFFFL; no ?


Julien

Mime
View raw message