ControlSystemStudio / phoebus

A framework and set of tools to monitor and operate large scale control systems, such as the ones in the accelerator community.
http://phoebus.org/
Eclipse Public License 1.0
92 stars 89 forks source link

Java code conventions #894

Open shroffk opened 5 years ago

shroffk commented 5 years ago

It seems that as a collaborative project we should have some code conventions to make it easier for others to read our code.

I am sure this will prompt some strong opinions...but the current solution of "each person uses their own" makes for a very messy code base.

@kasemir @willrogers @georgweiss @tanviash

shroffk commented 5 years ago

some conventions already available:

https://www.oracle.com/technetwork/java/codeconventions-150003.pdf

https://google.github.io/styleguide/javaguide.html

https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md

I do not expect us to change all the code in one go... but that we should adopt some conventions and stick to them as we move forward and update existing code when we touch it.

I will add a few comments with proposals and justifications... people can vote/comment on each

shroffk commented 5 years ago

One declaration per line is recommended since it encourages commenting. In other words,

int level; // indentation level
int size; // size of table

is preferred over

int level, size;
shroffk commented 5 years ago

We prefer putting an opening curly brace "{" at the end of a line, and not on a line by itself.


if (condition) {
    statements;
} else if (condition) {
    statements;
} else if (condition) {
    statements;
}

A scarce resource is the number of lines that one can see on the monitor at one time, and this convention wastes that resource.

if (condition)
{
    statements;
}
else if (condition)
{
    statements;
}
else if (condition)
{
    statements;
}        
shroffk commented 5 years ago

image

kasemir commented 5 years ago

Well, I do feel strongly about having balanced indentations for the opening and closing quotes.

georgweiss commented 5 years ago

@kasemir , are you referring to constructs like:

String a = "first" +
                  "second" +
                  "third";

Regarding braces, I think

if(){
}
else{
}

is more readable than the @shroffk 's example, even though this "wastes" an additional line.

kasemir commented 5 years ago

Sorry, braces not quotes. I'm referring to https://en.wikipedia.org/wiki/Indentation_style#Allman_style, being in favor of balanced braces

int func()
{
    while (true)
    {
    }
}

over placing the opening and closing brace at different indentation levels:

int func() {
    while (true)  {
    }
}
willrogers commented 5 years ago

I think it's a very good idea to have consistent formatting. My preferred method is to have automatic formatting on save and a CI check to make sure the code conforms.

I don't really mind what the convention is.

kasemir commented 5 years ago

Not sure about automatic formatting. Just like code doesn't write itself, it doesn't necessarily format itself.

// For instance, sometimes it better to have a short
final int end = (xx > l) ? xx : l-4;
// while at other times it's better to align like this
final int end = some_larger_expression
              ? value_for_one_case
              : value_for_other_case;

// Auto-formatted is likely worst
final int end = (some expression ) ? and then breaking
         just because the formatter hit : eighty
         characters;
georgweiss commented 5 years ago

Agree with @kasemir that auto-formatting may not always work as expected. I would suggest we agree on a set of conventions and express them in formatting files for (at least) Eclipse, IntelliJ and Netbeans.

shroffk commented 5 years ago

I think we can discuss this topic in detail during the next developers meeting on Wednesday.

Like @georgweiss said...maybe the existing formatting files do not fit our needs. We should make new formatting files with balanced branches, line splits on "," and not random locations, etc...

While always keeping the option open for the developers to format code as necessary....however this should not mean each class has a different style of branches, variable names, function names, etc...

I am hoping that we can agree on a set of rules... the exact rules are not necessarily my primary concern here.

kasemir commented 4 years ago

Created PR with writeup of suggestions for formatting, logging, file locations, naming of tests, ... We can then update that as needed, if possible add formatting files for IDEs etc. In any case, the information would be right with the code base, not scattered in hangout notes or tickets.