jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
Subject Re: Code Style Guidelines
Date Sun, 12 Feb 2017 20:46:54 GMT


Am 12. Februar 2017 21:32:35 MEZ schrieb Graham Russell <graham@ham1.co.uk>:
>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

+1

>
>>
>>> * 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.

I believe the tomcat project has coding guidelines which we could use. And they have docs
on how to setup your ide.


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

There would be a lot of warnings. Nobody would look at them after a while.

We could look at the categories with the least amount of warnings, get them to zero and then
enforce them. But I believe that would not work with the boycott rule alone.

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

Great.

One rule I really like is to separate formatting from code changes.

Felix

>
>>
>>>
>>> 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