emoose / openhotel

some node.js/socket.io javascript tile thing
Other
5 stars 6 forks source link

Code conventions #10

Open breaks opened 10 years ago

breaks commented 10 years ago

Not high priority, but opening this issue for discussion.

Tabs, spaces, or mix (spaces for alignment, tabs for indentation)? If spaces, use 4 space indentation?

Style? The code in index.html has been mixing between

if(something) {
    // stuff
}

and

if(something)
{
   // stuff
}

while app.js uses the latter throughout, make the second style standard?

emoose commented 10 years ago

I'd prefer to use 4 space indentation with no tabs, since tabs can get shown differently depending on the text editor etc.

Also empty lines should be blank with no spaces/tabs, git likes to show a big red box during git diff on any empty lines with spaces/tabs in it (at least with my environment it does) so it should be simple to spot them and delete them.

The second style is my preference, curly braces should always be on a line of their own to make code blocks easier to distinguish, maybe others would disagree but that's the way I'd like it.

The first style came from earlier code, I've been trying to change the ones I've spotted into the second style but there's probably a few I missed out, I'll be refactoring the index.html tomorrow anyway so I'll fix them then.

breaks commented 10 years ago

Alright, I'll be using the second style from now on and convert all my tabs to 4 spaces, also I think I've spotted a couple of empty lines with extra spaces/tabs so I could go through the files and clean them up

emoose commented 10 years ago

Sure feel free to. Also I forgot to mention one other convention, if you have an if statement that only executes one line of code I'd prefer it if the braces were omitted, eg

if(condition)
    execute();
else if(condition2)
    execute2();
else
    execute3();

instead of

if(condition)
{
    execute();
}
else if(condition2)
{
    execute2();
}
else
{
    execute3();
}

Since the braces are just wasted lines if there's only a single line being executed.

I might have used something like

if(condition) execute();

before too, but thinking about it having the execute() on a seperate line looks neater, I'll fix up those ones later

infoburp commented 10 years ago

I only just found this. if(something) { // stuff }

I've used this style with tab indents. I've done this even for one line //stuff, for the sake of clarity (I find code really difficult to read unless it is like this)