Clean Coding - Common Sense
 

Hello everyone. I'm Chris Zakuto, I script a ton in GS2. Whenever I go onto another server, I find the owners begging me to fix something, so I go in to fix it, and oh no! I see messy code, which happens to give me a headache.

I'll explain how to make code easier to read by yourself, and easier to read and edit by others aswell.

Part I -- Spaces
This is one of the simplest things to do, and it's very easy to get used to. Just add spaces! For example, let's say we're setting a variable.
That's okay, works the same, but it much cleaner if you do it like this.
All I did was add a space after the variable name (x), a space after the equals, a space after the screenwidth, a space after the division sign, etc.
Very simple and very easy to do. Try it out in your next script and see how much better it looks.
Another thing I do is put spaces before and after parenthesis "(" and ")". Check these out.
Fix it by adding spaces after the parenthesis and after a comma. (and also after the if)
Much cleaner (would look even better with formatting).

Part II - Separate Lines
No, this actually makes your script look better. It's not just some way to increase line count (which doesn't actually matter at all...)
For each brace "{" and "}" give it it's own line. This looks much better, and is a huge step closer to clean coding. For example
That's much too crowded.
More lines, but that doesn't matter. Much cleaner code, especially in longer scripts.
Another thing I hate is an event that has a single line of code and has no brackets.
Can be corrected easily
Part III - Formatting
Now that you have everything else down, this is probably the most important part, and all it takes is the tab button every once in a while. Take a look at this code.
Eww, ugly. Check this out.
That's the last time you'll ever be caught searching for where you forgot to put that end brace! All it takes is a tab. Whenever you create a new opening brace, immediatly make the closing brace. I always do that, and it always works for me. I never have too many/too few braces in my programs.

Part IV - Naming Functions
This is really just something that makes your code look nicer. I've used several examples of functions here, such as dosomethingelse(), godosomething(), and milkacow(). These are a little ugly, but are incredibly easy to fix.
What you do is take the first word of a function (I'll use the function godosomethingelse()), and then add the rest of the words with their first capitalized. For example, that godosomethingelse() can be split into:
go do something else
now change it to this
go Do Something Else
then put them together and you have
goDoSomethingElse();
Much nicer.

Hope you all enjoyed this guide. Leave comments as you wish, but please don't flame.
Chris Zakuto

Pretty much reinforcing some of what I've written in http://forums.graalonline.com/forums...ad.php?t=61805, there are a few more guidelines that I have previously written there to help keep code clean too.

Nice work, though.

Thanks, if I'd have seen that I wouldn't have posted this ... though you did post that in 2005, so unless you're searching for it you wouldn't find it.

The only problem I have with adding spacing just for personal looks is the fact I hate adding all that width and having to scroll horizontally.

If you maximize the screen there are few times you have to do this ... most things can be split up.

Ya, I do it mainly with arrays, but I hate doing it with if statements.

Real coders just do it however they like it then run their code through a beautifier when they are done ;)

see emacs/vim

Umm what?
Looks way nicer then

But that is a preference thing. I personally think what you say is better looks more obnoxious.

also, you space too much, you can use spacing to help visualize order of operations. and padding the conditional statements is a waste.

also, single line functions are okay as wrappers. Declare them at the top, looks nice.
And i am going to agree with dusty on multilining if statements. if it needs to be that obnoxious, store booleans before the comparision.

Yeah, Hell, it really is based all off preference. For me it's more of a mood thing, sometimes I think padding looks nice, but no padding is more compact and a bit cleaner.

Style like a legend:

No. Ideally you should not be omitting brackets.

I prefer my way.

Lol that's a mix of styles >_<

I follow these two simple things:

* New line for bracket ONLY at new functions
* Spacing in variables (such as: var = pie + (1*4)), and in function paramaters (such as blah(p1, p2, p3))

With functions I always use brackets, with statements I tend to omit them if there is only one command by force of habit, but I do usually indent it still and move it to the next line. If there is an else following the if however I always use brackets. Personally I just find it looks a lot nicer that way. As far as spacing goes I agree with HR.

One to their own I suppose ;)

I guess, however I promote clean styling for a reason: to promote editability, understandability and just overall cleanliness. Leaving brackets out is hardly efficient; say you want to add something later into your conditional code block, you'll then need to add brackets which you could have just done to start with!

Row count ++ :cry:

legibility ++ :)

Bleh, I code like this:

Heres a sample of my coding, I'm extremely picky about the formatting.

Your styling is practically the same as mine ^^

A general guideline is to keep the formatting as similar to the rest of the project as possible. It might be a good idea to write a general guideline that all developers abide to.
Here's how the Angband source code is styled:

:D

Note how I use temp variables. It is something I began doing only recently and it makes the script look much more organized rather than using temp. repeatedly before all the temp variable names.

I use temp variables all the time too! :D

What does do? Is there really a point to it? does it declare it or something

Yes, I use that for temp variables only, it declares the variable as a temp so it can be referenced without the use of temp.

When you have variables defined in function parameters they're declared as temp variables and you're able to reference them with or without the temp. before the variable name, this is the same idea.

I place all the temp declarations at the beginning so I know what all I'm using and can make notes for other people using it.

Declares the variable, so it's value is "" not NULL.

Um, you can't do that, when you say vars = something, you're actually assinging it to vars, not temp.vars
Yields the following:
The best idler is the (Server): TEST:
The best idler is the (Server): TEST2:2

note that temp.hello is still null. The only way to drop the temp is if the var was passed as a paremeter, it will take presedence over an already global var.

On another note, ziro, grey, and inver all have good styling imho

I was about to comment on this too:
Returns:

Ah, no wonder I had that array problem that time, oh well, it can be easily be fixed.
Returns 3 in both cases, problem solved.

Edit:
It doesn't look pretty anymore though :cry:

I'm not sure why you are trying to manage the variables anyway. The engine's meant to do that for you, and if your functions are structured sensibly, it should be obvious which variable names you are using.

Uh, I do this for the simple fact that I'm damn tired of typing temp. before every variable name, no other reason. I also like it better without temp., its more pretty like that. Also shortens the code in the long run.

I think it's more of a styling thing. So he doesn't have to write temp. all of the time.

EDIT: Ding Ding Ding!

Hm, in my opinion, it is better to always prefix variables. That way, there is no confusion as to which scope you are working in.

I try to write my scripts for people who have to work with them as well as for myself.

For myself, I only do this no-prefixing thing for temp variables, like how local variables in Java usually go without a prefix. So there is no confusion for myself. But also, all the temp variables are declared at the beginning so you know them already.

Edit:
I would like it if un-prefixed variables defaulted to temp. rather than a global if no previous temp. or this. variables were found.

My point is not so much whether it's solely clear to you, but also to other people who are reading the script who might not be as good at scripting as you or I. A lot of people have difficulty getting their heads around variable declarations, and even more people have difficulty identifying differences between scopes. If everything temporary is prefixed, there's no confusion. If you list things as temp. and then don't use temp. in the rest of the function, that seems slightly misleading. :|

There's more to styling than whether it looks pretty or not; it's about catering for other people who may wish to edit the script or read it. It's for that reason that we prefer people to style scripts before posting asking for help with them here.

You have a good point. :D
Though on Aeon, Napo and myself are the only major scripters and hes easily skilled enough to not be confused in any way.
Though I suppose when I release some of my stuff to public I will restyle it...
However until then nobody needs to be lookin' at my stuff :P

/sign for un-prefixed variables going temp and not global.

Clean coding looks prettier anyway ;)

I recently have been practicing better organization and clean coding in my scripts, heres a snippet.