Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve scijava coding style #8

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maarzt
Copy link
Collaborator

@maarzt maarzt commented Feb 10, 2023

I would like to improve the scijava coding style in a similar manner as done with the imglib2 coding style.
The main change is that the code formatter now is less likely to remove manually placed new line characters.
This combined with a longer line length gives us programmers more freedom when using the coding style.

This demo PR shows the impact of the changes on the scijava-common repository.

Changes:

  • Increase line length.
  • Allow more freedom to manually break lines.
  • Don't inline if statements
  • Use only one tab for indentation in enum classes. (fix)

* Increase line length.
* Allow more freedom to manually break lines.
* Don't inline if statements
* Use only one tab for indentation in enum classes. (fix)
@maarzt maarzt requested a review from ctrueden February 10, 2023 14:42
@ctrueden
Copy link
Member

ctrueden commented Feb 15, 2023

@maarzt Thanks for working on this. I like these changes for the most part.

I'm sad to see the 80 limit go because the only true terminal size is 80x25 forever, but at the same time it's a huge pain in practice and having it longer is really better. So I accept it.

I do not like the change to inlining of if statements. I know I am in a small minority here, but I love reading code like:

String lang = ss.getLanguageByName(arg);
if (lang == null) lang = ss.getLanguageByExtension(arg);
if (lang == null) lang = doSomeOtherFallback(arg);

When the consequents are not inlined anymore, it becomes:

String lang = ss.getLanguageByName(arg);
if (lang == null)
  lang = ss.getLanguageByExtension(arg);
if (lang == null)
  lang = doSomeOtherFallback(arg);

Which I find to be ugly and a waste of space. It's even worse IMHO with the if / else if / else if / else pattern. I really just want each case on its own line, no extra newlines, no curly braces.

I'm probably willing to cave on this, since probably everyone else disagrees with me, but just wanted to throw in why the style was like this up till now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants