Adopt official Java code format #60
Comments
|
Are we ready to follow the above referenced conventions to the letter? Or we want just pick and choose whatever we like and ignore what we don't? For example: Section 10 recommends parenthesis in multi-condition ifs:
The standard Java classes definitely don't heed this advice. The rules about the blank lines (5.1.2 Single-Line Comments) also are not followed. |
|
It is a pretty good ancestor, and most of the big and industrially used code style are based on it. It’s still probably a good place to start as check style plugin fodder. I would like to see single statements without braces being illegal for safety reasons, which is a fairly common addition that is often added, but I am not going to enter religious war territory. The most important thing is it looks familiar to adopters, and that it can be automatically created with Alt-Ctrl-L or similar “fix my code” shortcut in an IDE.
/M
… On 19 May 2023, at 20:52, Gene Gleyzer ***@***.***> wrote:
Are we ready to follow the above referenced conventions to the letter? Or we want just pick and choose whatever we like and ignore what we don't?
For example: Section 10 recommends parenthesis in multi-condition ifs:
... you shouldn’t assume that other programmers know precedence as well as you do ...
if (a == b && c == d) // AVOID!
if ((a == b) && (c == d)) // RIGHT
The standard Java classes definitely don't heed this advice.
The rules about the blank lines (5.1.2 Single-Line Comments) also are not followed.
—
Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDMSJUDJOTKBJM53I6VF3XG66PHANCNFSM6AAAAAAYIBAS44>.
You are receiving this because you are subscribed to this thread.
|
|
The goal is to make it comfortable for people who currently use Java (etc.) to read Ecstasy code. I guess this might be worse also for some C# programmers (since Microsoft uses a similar style to old-Ecstasy, which isn't too surprising since I think we based our style design in the early 90s on what we saw working well in Windows C development at the time.) We can augment the style guide with specific points. The |
|
I am only mention things that don’t just have cosmetic value in this thread, other parts of the standard, I consider religion and usually only care about their consistency.
There are a couple other common extensions to the default standard, having to do with object finality. This was mandated with almost identical settings at my last two jobs. I have also seen this in several other projects, including subproject of the JDK, and after working with this, I became a convert and a fan.
* All parameters to a method must be final, or checkstyle fails. (Enforceable by checkstyle.xml)
* All local variables should be final unless there is a good reason they needn’t be. (Not enforceable by checkstyle without extra logic?)
This may initially add some verbosity to read it, but there is evidence it prevents creation of unnecessarily stateful code. It looks huge in Java due to verbosity, but after a while you don’t see it, and start picking up a non final local variable as “ok - that has to have been done for a reason”, which has actually contributed to fixing some issues in my team. Not sure how controversial this one is. It’s even used in various subproject of the JDK.
* More importantly: code standard should enforce final fields, unless there is an explicit reason that they shouldn’t be final. Sometimes this is combined with mutable fields having to be declared after the final fields of the same protection domain at the start of every class. (This is hard to enforce algorithmically, of course). I like the declaration order guarantee, as you very quickly understand what is stateful in a class you haven’t seen before.
This last one has obvious and real benefits. This is more of a problem than the above finality, because if missing, properties like copy on write, and the hugely nice one of immutability in collections and avoiding having different but equal objects created and leaking out onto the heap. If a class has de facto final fields, but they aren’t declared as such, it usually just takes seconds until someone has added 14 boilerplate public setters, because they are too ignorant to realise that JSON deserialisers don’t require public setters, but Jackson, for example, requires it in its default configuration to create a POJO from JSON. It’s also fairly common that people do this for cosmetic reasons, as they think a constructor with 14 parameters is incredibly hard to read. While technically true, the correct way to solve this is to use a builder pattern with chained setters for construction.
This last one is in no way religious. Once public setters have been sloppily introduced everywhere for various reasons, it is near impossible to put that abstraction leak back into the bottle. It also has the side effect that you tell the rest of the world exactly everything about your implementation, and start shipping artefacts to mavenCentral with public APIs that third party users start using, and then you are in tech debt HELL. As I feared, when I realised we had this disease in one very complex project, it made any kind of efficient object equality and immutability assumptions impossible and works against the way of Java objects and collections. More seriously - this in turn led to every heap having massive amount of duplicated objects. It was simply impossible to guarantee that we didn’t have copy field contents, as we had no way of assuming immutability when writing code. So this one is a real concern for more complex systems that persist data on the Java heap.
/M
P.S. After ransacking my brain, I found I actually have one religious piece of cosmetic fundamentalism. I insist that casts should never be followed by whitespace. Casts are a unary operator. Other unary operations in 99% of the cases enforced to be without space between operator and operand (e.g. I++), and cast is not some special case. It is the same kind of animal.
… On 19 May 2023, at 22:16, Cameron Purdy ***@***.***> wrote:
The goal is to make it comfortable for people who currently use Java (etc.) to read Ecstasy code. I guess this might be worse also for some C# programmers (since Microsoft uses a similar style to old-Ecstasy, which isn't too surprising since I think we based our style design in the early 90s on what we saw working well in Windows C development at the time.)
We can augment the style guide with specific points. The {} required one is a good choice, for example (and we always use that already).
—
Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDMSNOBNJCS4DH6DXL323XG7ILHANCNFSM6AAAAAAYIBAS44>.
You are receiving this because you commented.
|
|
TL;DR - it should always hurt a little, and feel a little uncomfortable to look at stateful entities in code. For some cases it should hurt a lot.
… On 20 May 2023, at 17:28, Marcus Lagergren ***@***.***> wrote:
t
|
|
Developed a one-time use tool for doing the code conversion of .x files: So far, applied to the lib_ecstasy and javatools_bridge libraries. |
|
Completed applying reformatting tool to all xtclang/xvm .x files (libraries and examples). Gene is converting tests. Other projects (such as platform) will need to be updated as well. @lagergren we should be able to use an existing tool to do the same reformatting job for the Java code. Any experience with any of them? |
|
Yes, I have experience with checkstyle and spotless. Typically the former checks the syntax, and the spotlistApply gradle build task applies the changes. Best practise is to continue by adding a git hook so the checks run automatically every commit so you don't have to. Typically I only use this as a verification for a finished pull request when it goes into master. Normally it's just invoked from all local builds (without applying the spotless code changes). I'd be happy to set this up in a few weeks when the language support is good enough. The advantage with these tools is that they can very easily be extended with our existing XTC grammar, to also support an XTC code standard for any XTC projects in our builds! /M |
|
Let's just make sure to completely "sew up the current patient" (the currently in-flight build changes) first before getting onto this one. |
Is your feature request related to a problem? Please describe.
The only "problem" is a potential difficulty for developers who are not already familiar with the current code formatting.
Describe the solution you'd like
Use Java's code format standards, as described in Java Code Conventions
Suggestion: Some combination of auto-reformatting tools, plus manual review.
Suggestion: Add a check to CI pipeline to check formats.
Describe alternatives you've considered
There are infinite alternatives, which is to say there are no alternatives.
Additional context
Current code style is "Tangosol". It's a frequent topic of objection from people used to the common style used by Java and other languages (to save vertical space on their teletypes).
The text was updated successfully, but these errors were encountered: