New Organized & Documented Coding Conventions & Practices?

[R41D3NN]

It has become more prevalent through Terasology's development that people are using some of their own conventions wherever they want. As such, code isn't consistent! There are even two different spellings for a handful of methods. It is not easy keeping up with that especially when there is no Javadoc attached to the code... I have started this topic in order to get a feel for what the general consensus is on conventions so please respond and I will attempt to draft some documented conventions that better suit the project at this point in time.

• Field names should never begin with underscore. IE. no _instance or _name fields! If you haven't heard of the this identifier it might be acceptable, but that shouldn't be the case, especially when it isn't even used in that manner.
• The commonplace initialize method should be named either initialize or initialise, not both!
• A comment that extends beyond a single line should use the multiline comment feature. (Its nice for people with IDEs that fold elements and it makes code more readable.)
• Chaining methods can be better read if each scope is aligned at its . character. Its okay if there is more than one method call per line but just don't be excessive. If there are arguments in a method call then keep that on its own line.
  I know this is a bunch of crap but it shows what I'm talking about:
  

  getSomething().getParent().getContainer()
                .setPosition(child.getX(), sibling.getPosition().getY())
                .getNormal();

Java Source 1.7
What is everyone's opinion of supporting 1.7?

More on this later!

 

[Cervator]

Re: New Organized & Documented Coding Conventions & Practice

Dug out this little snippet from an old wiki page - was about all we had previously + "Just match existing style please" which of course can be a little tricky, especially after the inconsistencies get more frequent

* Class names: CamelCase
* Method names: lowerCamelCase
* Instance variables: _lowerCamelCase
* Parameters: lowerCamelCase
* Local variables: lowerCamelCase
* Constant values (only values that are static and final): THIS_IS_THE_CONSTANT

One other thing that came up in the past is putting the curly brace at the end of a line rather than start of the next line. I'm personally more of a fan of next line, but figured we should stick with what's already in use for 95-99% of the existing code base, so I submitted.

Do you think the _instanceVar bit is really important enough to change everywhere? We started out with underscores, but I expect there are some places without. I know the IDE generally highlights instance vars anyway, tho I don't really mind having the extra _ to highlight even in plain text

Initialize / Initialise - yeah, I can see stuff like that happen since we're so international A good thing to tweak into either direction consistently. Agreed on comments and chaining, those look good.

One thing that popped up a little is prefixing interfaces with I and such, that's more rare than not prefixing, IIRC. I'd lean toward no prefixes, but don't have much of a dog in that hunt

Thanks for your initiative!

 

[R41D3NN]

Re: New Organized & Documented Coding Conventions & Practice

* Class names: CamelCase
* Method names: lowerCamelCase
* Instance variables: _lowerCamelCase
* Parameters: lowerCamelCase
* Local variables: lowerCamelCase
* Constant values (only values that are static and final): THIS_IS_THE_CONSTANT

I was digging around myself and couldn't find any good info on conventions. I guess there are no good keywords located in the documents... Anyways:

• class names: CamelCase - absolutely
• MethodNames: camelCase - yep
• Instance variables: _camelCase - nooo (the scope where an instance variable is created should describe it enough and if its a singleton within a class then it could simply be called instance)
• Parameters: camelCase - yep
• Local variables: camelCase - yep
• Constants: CONSTANT - honestly I have never been a fan of this style. I understand the reasoning behind it but I personally like CamelCaseConstant. But, no biggy.

Do you think the _instanceVar bit is really important enough to change everywhere? We started out with underscores, but I expect there are some places without. I know the IDE generally highlights instance vars anyway, tho I don't really mind having the extra _ to highlight even in plain text

The thing about underscores is that it is a very ugly and a next to pointless convention. As much as I respect standards, I just know that prefixing variables is not a very good way of going about self-documentation.

One thing that popped up a little is prefixing interfaces with I and such, that's more rare than not prefixing, IIRC. I'd lean toward no prefixes, but don't have much of a dog in that hunt

I don't mind the IInterface prefix so much because at times it is useful to have some class called Creature and then have a skeleton (much like the C header file) called ICreature that can clearly represent a Creature's features (hey thats fun to say) in a concise manner. Although that may be abusing the actual realization for interfaces... Oh well.

 

[R41D3NN]

Re: New Organized & Documented Coding Conventions & Practice

Instance variables ... and if its a singleton within a class then it could simply be called instance)

Whoops. My bad, if it is the only singleton.

 

[Cervator]

Re: New Organized & Documented Coding Conventions & Practice

Oh, heh, I dug out that snippet from a hidden wiki web. Never did finish digging out all the useful stuff. Will have to go over it again when the new site is ready

 

[R41D3NN]

Re: New Organized & Documented Coding Conventions & Practice

Oh, heh, I dug out that snippet from a hidden wiki web. ... Will have to go over it again when the new site is ready

Well thats real useful for the community...
Yeah, this will all be well documented and visible.

 

[overdhose]

Re: New Organized & Documented Coding Conventions & Practice

Have no grudge against underscores etc either, Camel casing is what i was used to.

Glad to see the discussion, for something of this size you need to document it, else joining the project and have people tell you "just look at the existing code" doesn't cut it as you easily encounter 2 styles along the way used equally. Just my 2 cents.

I had a teacher who hated underscores, and a sempai who talked about sexy coding practices. I'll make sure to go over the minion code and set it straight

 

[Immortius]

Re: New Organized & Documented Coding Conventions & Practice

My issue with the underscores is that it breaks IDE help in creating getters and setters, and also means you have to break the the Java Bean standards to add them.

The standard is if you have a field:

private int test;
private boolean hungry;
private String _wrong;

The getters and setters are the field name prefixed with get and set and the first letter capitalised. With the exception of booleans that use "is" instead of "get":

public int getTest() {
    return test;
}

public void setTest(int test) {
    this.test = test;
}

public boolean isHungry() {
    return hungry;
}

public void setHungry(boolean hungry) {
    this.hungry = hungry;
}

// Underscore means _wrong needs to be
public String get_wrong() {
    return _wrong;
}

public void set_wrong(String wrong) {
    this._wrong = wrong
}

This can be functionally important, as this convention is expected by some reflection based systems. And is functionally important in Components because that convention is expected by the reflection-based system used in persisting them.

So anyhow, it is my fault that the code base has been drifting away from the _field format, as I've been following the Java standards for any new class I create. The only other part of the existing standard I haven't been following was the static variables and I'm happy to fix that up.

Initialise would also be my fault, because, well, that's how it is spelt. Where I live anyhow.

Java Source 1.7

I feel it is a little too early to switch to Java 1.7, especially because the MacOSX 1.7 JRE/JDK drops an important feature rendering the game unplayable - it breaks LWJGL. Take up also seems a little slow. Maybe later this year or early next year?

 

[Cervator]

Re: New Organized & Documented Coding Conventions & Practice

Yeah I've wondered about the 1.7 quietness myself. Maybe the whole Snoracle thing sidelined that version. 1.8 is getting closer - is there even anything substantial in 1.7 we want?

On underscores - does anybody want to keep them or do we just toss em?

 

[Immortius]

Re: New Organized & Documented Coding Conventions & Practice

I think people just got used to there not being a new version of Java, since it took so long for to the political nonsense to get sorted out.

 

[ironchefpython]

Re: New Organized & Documented Coding Conventions & Practice

On underscores - does anybody want to keep them or do we just toss em?

If you're talking about instance variables that start with an underscore, remove them as the code is edited.

 

[Cervator]

Okay, underscores are out then, whenever somebody gets around to edit classes with some of them in there please zap the underscore.

New question - before I accept the pull request over at https://github.com/MovingBlocks/Terasology/pull/247 could I get some opinions on excessive curly brackets around fors and ifs? I noticed some of them getting formatted out, know that we should be good coders and always be able to catch when we do need brackets, but I have a feeling that's one of those personal opinions, and I'm not sure which side we have the most people on

I don't mind the extras for the safety it can yield, kinda like that, but I also like concise code and appreciate the overall work from which the curly brackets is just a small part. I'm also known for my ability to be amazingly indecisive. So I figured I'd ask. I'd say with R41D3NN willing to put in the work to do all this formatting we'll default to remove them unless objections are raised?

 

[Immortius]

We shouldn't remove them. Since we're using the 1TBS it is only one extra line anyway, and it prevents a class of difficult to spot errors (specifically a second line tabbed under the first, which looks like it is part of the if but isn't).

I think the easiest way for us to approach coding conventions is simply to follow http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html unless we have good reason to break from it. And then accent with any other code style standards we need that are unique to our code base.

 

[Afforess]

My 2 cents:

I agree with @Immortuis, Oracle/Sun standards are pretty much universally accepted for Java. I would stick with them, and only tweak as needed.

I'd also like to agree with the decision to stick with 1.6. Generally the rule of thumb is to support the current Java release (1.7) and one previous release, which is 1.6. It would be painful to try to support 1.5 also, or only support 1.7.

 

[Immortius]

Alright, if there is no dissent then I move for the adoption of that standard. This means:

• Follow these naming conventions
• Have braces around single-line if/for/while statements.
• The ternary statement should have brackets around the condition expression.
• Feel free to have extra (unnecessary) brackets where it adds clarity
• Braces follow 1TBS

With this standard, the IDE format code option should format things correctly for use, so there should be no need to manually mess with braces.

Proposed additions to the standard:

• I think we should stick with the existing copyright notices format for now
• Use // TODO comments to note where something needs to be done
• I've been using // JAVA7 comments to note where something could be improved in Java 7.

 

[overdhose]

one small detail about the switch statement I only noticed now: I'm a bit amazed they don't surround statements in the switch by default. I usually do. Would it be preferable to keep the switch statement as short as possible with method calls for readability?

 

[overdhose]

another small detail : do we precede every Enum with Enum like EnumBooleanMap?

 

[Immortius]

On switch statements: I think that is a good idea, having each case statement short and calling off to another method. With enums though it can be even better to just put the behavior in the enum and skip the switch statement entirely, although that isn't always possible or sensible.

I saw a suggestion somewhere (probably in an article about self-documenting code) that it can make code cleaner to read if methods are either just some decision making and calls to other methods, or do actual processing, but not both. With that in mind where you have a switch statement would be naturally for it to merely call other methods to do the actions.

On enum prefixing: No. EnumBooleanMap is not an enum by the way, it is a specialty collection that acts like a Map<T extends Enum, Boolean> but takes advantage of the nature of enums for improved speed (O(1) read and writes with no hashing involved, fixed memory usage based on the number of items in the Enum).

 

[Cervator]

Couple more bits on prefixes - no underscores in instance variables, and what about "I" and stuff like that in interfaces, no?

 

[Immortius]

My opinion - in Java (judging from the core libraries) it is convention to have interfaces with normal names, and then have implementations either with a name suggestive of the nature of the implementation (Map -> HashMap) or sometimes ending with Impl if only one implementation makes sense. This fits with the Java paradigm of having code working against interfaces and not caring about implementation - you have a Map, doesn't really matter if it is an interface or not everywhere except where you create it (potentially using Guava's Maps.newHashMap() so you don't even have to import HashMap).