Criticise my programming style!

[chemicalNova]

Hi!

See here: http://www.privatepaste.com/691PwqptMp

I'm seriously looking for a bit of advice as to my programming style.. rip it apart if you must.

I've been told I have an "okay" layout.. what do you think?

Also, I want some opinions on how I'm going about using exceptions. Would you, as a teacher of a Game Development course.. take marks off for my use of exceptions in the above code? It's used in the MenuManager's HandleInput function, which will throw either a "Play" or "Exit" exception out to the Run function, which will then rethrow it out to the calling function (GameManager's Loop function).

Its basically what I've done so far on my breakout clone. All the background stuff is what I'm setting up before any rendering is to be done.

Thoughts please.. cheers

chem

[r0kk3rz]

first thing, is comments, where im at with programming my lecturers are absolutely ANAL about commenting, you've got a handful in there, but i would put more.

aside from that its fairly nicely laid out, good use of white space, but probably a good idea to give your classes better headers, explain exactly what each of them does etc

the exceptions is a little bit hackish, dont really understand enough about it to really comment about it further. if you're going to keep that id probably want to comment exactly what its doing.

[chemicalNova]

Yeah that comes later for me.. I hate comments.. mostly because I tend to write comments like I'm talking to someone in a conversation, and its just embarassing

I fully document each function and variable later on, don't worry

chem

[shotty tomato]

You have no style chem. Give up now.

[r0kk3rz]

i generally do the exact opposite, create comment headers THEN write the function

[Anakha]

Originally Posted by shotty tomato

You have no style chem. Give up now.

^^^^

[MessiaH]

Man, I don't even know where to start with that but you are doing it all wrong.

Are you self taught, because any professional coder wouldn't do what you have done there.

I don't think I can even help you, I don't have anything to work with.






I actually know nothing about coding and wouldn't be able to decipher that code if my life depended on it. This post is only a joke lol.

[Khaless]

Hi Chem, I am at work so I cant read your code too much, but I can see you are using Exception Specifications.

Now Generally this is considered a bad idea, for one it can cause surprising runtime performance hits! (must support exceptions & runtime). Also what happens if g() has an e spec (throw(X) and it calls f() with no spec or a less restrictive one. then the spec will be violated if f throws anything but X, even though f() may be formally able to throw anything.

If you dont believe me, believe the authors of boost! They put Exception Specifications into boost before deciding to rip it all out again.

I am sure a google of 'Exception Specifications' will bring up the issue.

[Khaless]

Oh, one more thing... I will have a better look @ lunch, but use an initializer list?

you have:

GameManager()
{
GM_GameRunning = true;
}

so this could be
GameManager() : GM_GameRunning(true) { }



PPS.

Self Documenting Code

// if this brick has died.. remove it
if (BrickIt->GetHealth() < 1) {
BrickIt->SetDelete(true);
}

instead you could drop the comment and have:

if(BrickId->isDead()) {
...
}

Which also means your policy for a dead Brick is also in Brick and not a magic value wherever the hell that is. also it removes a redundant comment.

PPPS..

Yes comments are good, however THIS IS NOT
i++; // Increments i.

(i see it all the time).

[flanK]

Khaless rules.

['stEEdy]

Originally Posted by Khaless

Also what happens if g() has an e spec (throw(X) and it calls f() with no spec or a less restrictive one. then the spec will be violated if f throws anything but X, even though f() may be formally able to throw anything.

O__O

[Strik3r]

eww.. bad chem, bad boy. ignoring the potential performance ramifications of using exceptions like that, its just ... yucky :P there has to be 69 other ways to achieve the same thing.

Other than that, the code seems ok. I dont like the way you have several 'Update' and 'Render' functions. What I have always done is keep track of the current game state, and have a switch statement inside my render/update/input functions that perform the necissary tasks for each of the game states. (MENU, INGAME, PAUSE etc.)


oh one other thing. instead of

#ifndef _x.h_
#define _x.h_
#endif

just do

#pragma once

most compilers support it, and it compiles faster and looks nicer.

keep it up and let me know if you need any more help ^-^

s2 from gerv.

[Khaless]

Originally Posted by Strik3r

just do
#pragma once

GCC lists #pragma once as obsolete and recommends you dont use it
Had a pretty firey argument about this with friends at the beginning of the year.

[Strik3r]

luckily, no one develops games for linux and mac, so who cares what GCC says :P

http://gcc.gnu.org/onlinedocs/gcc-4....2donly-headers

as if that doesnt sound like its written by a bunch of fairies.

and its obvious he's using MSVS.. as long as the compiler handles it properly, its nicer than include guards.

[Khaless]

Originally Posted by Strik3r

luckily, no one develops games for linux and mac, so who cares what GCC says :P

I Do, Though I do see your point and respectfully disagree.



and besides he is using SDL, which supports....

*drumroll*

"SDL supports Linux, Windows, Windows CE, BeOS, MacOS, Mac OS X, FreeBSD, NetBSD, OpenBSD, BSD/OS, Solaris, IRIX, and QNX. The code contains support for AmigaOS, Dreamcast, Atari, AIX, OSF/Tru64, RISC OS, SymbianOS, and OS/2, but these are not officially supported."

[esjay]

I have a style guide if you want it chem, it's from C++ but i guess it basically extends to each C language.

From uni, ill post it up




1 Indentation
Code should be indented one step on entering any structure, and reduce one step at the end of a structure. The indentation step should be 4 columns. When indenting, use spaces, not tabs. This is necessary because in different viewing environments the tab size may be different. This will destroy your layout. Any good editor (including Visual Studio) allows you to set the tab length and use spaces, or at least convert from tabs to spaces.

[Strik3r]

bleh #prama once works.. and unless your using 'nix and have symbolic links all over the place, its foolproof (why you would have a symbolic link to a header file is beyond me). Almost all compilers support it (GCC does support it, but its just trying to promote its own 'optimized' handling of include guards.). Yes its non standard, but its so widely supported theres no reason not to use it. It compiles faster as you dont have to use the C preprocessor to look for #ifndef and its tidier.

my reasons for wanting to use pragma include speed and neatness. whats the reasonas for not wanting to use it ?

[chemicalNova]

Thanks guys.

This is why I posted here! I did think it looked a tad ugly.. but I wondered if it would get marks removed.

Also, Stroustrup doesn't mention any overheads in his Third Edition of the C++ Programming Language. When I read that, I had a quick scan back over exception handling, and I couldn't see anything.

I guess he might've just forgot to mention it.. I'll take your word for it and go back to the original state machine (much like what you said gerv ).

Cheers guys,

chem

[esjay]

i feel so out of place trying to fit into a conversation with khaless strik3r and chem

[Strik3r]

esjay. good kid, tries hard.

<3