[Development] Coding style guideline

[brrritssocold]

This issue is intended to discuss the draft of Coding style guideline.

[brrritssocold]

I would suggest always using curly braces for if / else blocks or even stand alone if. Not using curly bracers has a high risk of introducing bugs.

I would argue that the braces make the code more readable, which is important, code is read more often than written.

If / else blocks with multiple statements (in if) will cause a compiler error with Error: Syntax error: else is unexpected.

Produces a compiler error (Error: Syntax error: else is unexpected.):

			if (true)
				bar = 2;
				bar = 3;
			else
				bar = 4;

This will set bar to 42:

			if (false)
				bar = 4;
				bar = 42;

In addition, if more statements need to be added to a block, you do not have to find out where and then write the braces. In addition to this, IDEs can highlight code blocks using braces.

[brrritssocold]

Good practice for the else in conditionals:

Switching else in conditionals might work, but it could also make code harder to read if negation is used in an unusual way.

Wouldn't it be better to extract the expressions into a function?

Commenting out code blocks:

Just no. Don't do it. We are using version control for a reason. Remove code in a separate commit.

Naming conventions:

Naming for Interfaces, specifically no I prefix.

[Stadler76]

@brrritssocold wrote:

Commenting out code blocks:

Just no. Don't do it. We are using version control for a reason. Remove code in a separate commit.

That section was requested. Otherwise I wouldn't have added that. And about 'We are using version control for a reason': Tell that to those, that have absolutely no idea how to work with Git and produce many single file PRs, cuz they're using the sh**y GitHub Webinterface .... Another thing are these huge diffs some ppl. tend to produce messing up the file blame ...

@brrritssocold wrote:

I would suggest always using curly braces for if / else blocks or even stand alone if. Not using curly bracers has a high risk of introducing bugs.

@brrritssocold wrote:

Naming conventions:

Naming for Interfaces, specifically no I prefix.

Tell me, how I should change/improve those two sections or file a PR for that.

[brrritssocold]

Tell me, how I should change/improve those two sections or file a PR for that.

I will make the suggested edits and open a PR.

[Stadler76]

As a heads up bout the commented out code: Many ppl. which are coding for CoC only see the code itself and have absolutely no idea, what 'working with VCS' means. Probably we need to point ppl. to Git(Hub) tutorials before.

[Stadler76]

About

Commenting out code blocks:

mmh, Looking at other commented out code blocks in the codebase I guess its better to remove this section. Not because its not recommended because VC exists, more because its rather adds:

• inconsistency (most code is commented out after the indentation and not where the line(s) start
• confusion (ppl. may be confused, what is the right way to comment out code blocks)
• confusion v2 (harder to separate from actual code, see the 'bad' example below)

 			this.skin.furColor = randomChoice("black","brown");
//			trace("Minotaur Constructor!");
 			this.a = "the ";

Well, TD/DR: I shouldn't have added it to begin with ... too many cons with no real pros ...

[Oxdeception]

What would the preferred naming convention for an interface be? We have both ISerializable and TimeAwareInterface currently. If we're following those conventions, I'm partial to to the ISerializable style. Though I'd rather say neither, and have these as Serializable and TimeAware

[Stadler76]

@Oxdeception wrote:

Though I'd rather say neither, and have these as Serializable and TimeAware

I'd tend to that, too.

@brrritssocold @Kitteh6660 Your opinion to that?

[brrritssocold]

Originally went with the recommendation from Adobe, but removed the I prefix in a recent PR (#1222) as it is superfluous.

[Stadler76]

Added commit 6d248be8a6e5f1f68fc146f5bbe6d537b4c3bfac to remove the section about commenting out code blocks. Reasons stated above.

[AnonymousIncognito5]

The git documentation can be intimidating if you don't know what you are looking for, so here's some pointers:

• You can use git stash to save and temporarily revert all uncommitted changes. If you are having issues and want to try another tack, just run that command and it'll undo your changes. If you run git stash pop later (or git stash apply, see the man page), all your changes will come back. No screwing around with commit messages, no commenting out broken code, no spamming the undo and redo buttons.
• You can use git amend to fix the last commit if you want to change something. You can change the message, change the actual contents, whatever you want. See the documentation for the details. Keep in mind that this only works for the most recent commit, and you should never do this after sharing said commit with someone else. That would mean the repositories get out-of-sync, which is... messy. Other than that, Git commits are like constitutional amendments: the only way to change them is to make an entirely new amendment.