Hi, comments inline On Mon, May 20, 2013 at 10:59 AM, Emmanuel Lécharny <elecha...@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