Am 12. Februar 2017 21:32:35 MEZ schrieb Graham Russell <[email protected]>: >On 11 February 2017 at 23:14, Philippe Mouawad ><[email protected]> wrote: >> On Sat, Feb 11, 2017 at 11:27 PM, Graham Russell <[email protected]> >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.
