jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Russell <gra...@ham1.co.uk>
Subject Re: Code Style Guidelines
Date Sun, 12 Feb 2017 20:32:35 GMT
On 11 February 2017 at 23:14, Philippe Mouawad
<philippe.mouawad@gmail.com> wrote:
> On Sat, Feb 11, 2017 at 11:27 PM, Graham Russell <graham@ham1.co.uk> wrote:
>
>> Hi all
>>
>> Do we have any (written) guidance on code style?
>>
>> I know we have checkstyle, but this has very few rules.
>>
>> The reason for asking is that I think having a consistent style is very
>> important for readability (which helps writing new feature or fixing
>> existing code etc.).
>>
>> The main things I think contribute to this are (and which are fairly easy
>> to detect/fix):
>> * (soft) line lengths (80, 100?)
>> * spacing between elements on a line e.g. if (..., while (...,
>> methodCall(arg1, arg2), "con" + "cat"
>>
> If this is related to String concat, we should only do that in places where
> performance is not a critical point as IMU StringBuilder concat is still
> better than '+'

This was mainly about white space. However, I believe StringBuilder is
only better (performance) than "+" when inside a loop, elsewhere you
should use "+" for readability:
http://stackoverflow.com/questions/1532461/stringbuilder-vs-string-concatenation-in-tostring-in-java

>
>> * import order, spacing and not using .*
>> * line length of methods (soft limit of 50?)
>> * line length of classes (soft limit of 500?)
>>
>
> I am ok with these proposals and providing a checkstyle and Eclipse
> formatter might be very useful.
> But if possible, I'd prefer that we do not modify too much code only for
> code style as it might make it difficult to look into particular
> regressions.
> So maybe do that on new code or when changing a file for something else..

Yes, I agree that this should only be on new code or 'while you are in
the area' i.e. the boy scout rule (leave the code better than you
found it).

I use IntelliJ - I will export my current JMeter code formatting rules
and also try to create one for Eclipse which broadly aligns.

If we put these into checkstyle would this fail the build, or can they
just be warnings?

I've created a wiki page to capture the key points:
https://wiki.apache.org/jmeter/CodeStyleGuidelines

>
>>
>> From Phillippe's Java 8 email:
>>
> * Use of Optional
>>
>
> From the document, I understand Optional has a memory/performance impact so
> we need to take this into account where we decide to apply it.
> But I'm ok with it.

Certainly, this is important with performance critical parts of the code.

>
>
>> * Default/static methods on Interfaces
>>
> +1
>
>> * Lambas where possible (max ~5 lines?)
>>
> +1 provided reduced code is more readable than existing one. If it's
> reduced but we have to scratch our head to understand it I'm not sure it's
> a gain :-)

Absolutely, this is all to do with readability, if it doesn't fit in
~5 lines then it should probably be moved to a method and named
appropriately.

>
> Regarding Streams, we need to be sure of performance impact. And It appears
> Sonar has some issues analyzing such code

Yes I noticed that too, is this due to the version of Sonar we are
using or to unidentified/unfixed bugs?

>
>
>> I have found this: https://wiki.apache.org/jmeter/JMeterEclipse
>> which suggests some formatting e.g. 80 char line length and new line before
>> opening brace but most of the code doesn't conform to that - it was also
>> updated 8 years ago.
>>
>> Any other ideas/thoughts?
>>
>> Thanks
>>
>> Graham
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message