Welcome, Guest. Please login or register.

Login with username, password and session length

 
Advanced search

1411574 Posts in 69386 Topics- by 58444 Members - Latest Member: darkcitien

May 04, 2024, 07:30:00 PM

Need hosting? Check out Digital Ocean
(more details in this thread)
TIGSource ForumsDeveloperTechnical (Moderator: ThemsAllTook)Coding styles
Pages: 1 [2] 3 4 ... 7
Print
Author Topic: Coding styles  (Read 18887 times)
starsrift
Level 10
*****


Apparently I am a ruiner of worlds. Ooops.


View Profile WWW
« Reply #20 on: February 27, 2010, 04:22:28 AM »

When it comes to the subject of religion, Gentleman must be prepared to agree to disagree.

As far as varnames, I never use single or double letter names except for axis-position values, as it's easy to lose track of them and what the hell they do - without looking up my comments, which could be god knows where among the thousands of lines of code. If there's some ambiguity with the variable name, I preface the varname with the letter of its type(ie iVariable, sVariable, dVariable, fVariable). I always declare classes as cWhatever{};.

I don't use globals, I toss around a big container class by reference where I need to that contains anything that would be 'global'. It's usually called cGame and declared in my main. From it, all new variables and classes flow. Sometimes I even pass two objects for ease of typing, a cGame object and a cPlayer object. Probably strongly not approved because it introduces fantastic new ways to build errors. And in that vein, I almost never use the private: section, as a solo coder - and with the greatest reluctance interwoven with haughtily sneering derision as a team member. I'm still waiting to get my comeuppance when that backfires on me. . .

/ If I never see Pascal again, it would be still too soon.
Logged

"Vigorous writing is concise." - William Strunk, Jr.
As is coding.

I take life with a grain of salt.
And a slice of lime, plus a shot of tequila.
st33d
Guest
« Reply #21 on: February 27, 2010, 04:38:15 AM »

"g" is pretty much the only obscure and global reference I use. It's for the Game object that runs the whole show.

Bad practice, yes, but I generally start out with naughty code and move it towards polite code when I'm certain it works properly.
Logged
Average Software
Level 10
*****

Fleeing all W'rkncacnter


View Profile WWW
« Reply #22 on: February 27, 2010, 09:10:05 AM »

Rectangle.h

Code:
#ifndef __AWESOME_RECTANGLE_H_
    #define __AWESOME_RECTANGLE_H_

All names containing two consecutive underscores, or beginning with an underscore followed by a capital letter are in the compiler's reserved namespace.  Doing that is an extremely bad idea.
It's just to avoid them colliding with other declarations.

The problem is that they can collide with compiler generated declarations that you never even see.  We had a project that did this once, and the programmer (who didn't know the rules) did this to fix it:

Code:
#ifndef __SOMEHEADER_H__ABUNCHOFRANDOMCRAPONTHEENDTOAVOIDCOLLISIONS

All he had to do was remove the double underscores.

The tricky thing is that the double/leading underscore is technically illegal, but the compiler can't diagnose it because it can't discern compiler symbols from user generated ones.

I really wish I knew where people get this habit from, I've been trying to figure it out for years since it's so common.  Stroustrup's book is the only one I think I've seen that makes sure to warn against it.

Bottom line, don't ever use double or leading underscores, and if the IDE generates them for you, file a bug report, because it is a bug.
Logged



What would John Carmack do?
Core Xii
Level 10
*****


the resident dissident


View Profile WWW
« Reply #23 on: February 27, 2010, 10:53:19 AM »

Oh goodness, I have a great disdain for spaces after if, for, while, etc. A coworker does this to me on purpose to annoy me to no end Cheesy. When did that syntax take root?

It distinguishes control structures from functions. Their parenthesis syntax is very different, therefore it makes sense the name part is too.

for(argument, argument, argument);
for (statement; expression; statement) {}
Logged
terloon
Level 0
**


View Profile WWW
« Reply #24 on: February 27, 2010, 11:22:11 AM »

That actually makes sense. He pretty much does it for everything, control structures and functions. I'm not one to nitpick, as long as there are good reasons and anything that improves readability is a plus for me.
Logged
Average Software
Level 10
*****

Fleeing all W'rkncacnter


View Profile WWW
« Reply #25 on: February 27, 2010, 12:02:37 PM »

General stuff:

Always put an empty line before and after block structures, unless the following statement is logically linked:

Code:
-- Empty line
if Condition then
    -- Stuff
end if;
-- Empty line

-- Case 2
-- Empty line
if Condition then
    -- Stuff
else -- No empty line, logically linked to first block
    -- Other stuff
end if;
-- Empty line

Always put a blank line before jump statements (unless it's the only statement in a block):

Jump statements are any statements that interrupt normal control flow and go somewhere else.  Examples are goto, return, throw/raise, break/continue/exit.

Code:
if Something then
    -- Do some stuff
    -- Empty line
    return;
end if;

Only use const/constant for things that are actually constant:

If you can't tell me what it's value is at compile time, don't tell me it's const.  (Pointers/references to const are a different story, those serve a different purpose)

Code:
void Funk(const int x)

My first question to someone that wrote this would be, "What is the value of x?"  The answer will of course be, "Whatever the caller passed."  So it's not really constant, is it?  The function Funk has every right to modify that data, and should be allowed to do so.

Sometimes people say, "Well, x shouldn't change during the function," and then down the road they find a reason why it would be convenient to change it, and then they either make a local copy of a local variable, wasting memory, or go back and remove the const, defeating its purpose in the first place.

typedef is for weak encapsulation, not for you being lazy and saving keystrokes:

I have a SoundManager package that identifies sounds by an unsigned int, and contains this statement:

Code:
typedef unsigned SoundID

The fact that SoundID is an unsigned is an implementation detail, and is no business of the user.  This is weak encapsulation because C typedefs just generate aliases instead of new types, Ada's similar construct actually creates a new type, and provides much stronger protections.

What I hate is stuff like this:

Code:
typedef void (*Callback)(int, int);
// Much later...
void RegisterCallback(Callback callback);

"Callback" tells me nothing about what kind of function the thing wants, and I have to go look up the stupid thing anyway, so just use the real type and save me the trouble.

If you don't need negative values, make it unsigned:

By changing the type of a piece of data, you eliminate an entire class of errors.  Similarly...

If you don't need to receive/return NULL, pass/return by reference, not by pointer:

Once again, if you don't want NULL, don't allow it to happen in the first place.  Use the type system to your advantage.

Don't allocate dynamically unless you have a damn good reason to:

When I see code like this, I get suspicious:

Code:
void Funk()
{
    Something *something = new Something;

    // Do some stuff.

    delete something;
}

Occasionally, there is a valid reason for doing this.  Most of the time you're just inviting memory leaks.  Just make it a stack object.
Logged



What would John Carmack do?
amro
Level 0
**


View Profile
« Reply #26 on: February 27, 2010, 12:59:46 PM »



Only use const/constant for things that are actually constant:

If you can't tell me what it's value is at compile time, don't tell me it's const.  (Pointers/references to const are a different story, those serve a different purpose)

Code:
void Funk(const int x)

My first question to someone that wrote this would be, "What is the value of x?"  The answer will of course be, "Whatever the caller passed."  So it's not really constant, is it?  The function Funk has every right to modify that data, and should be allowed to do so.

Sometimes people say, "Well, x shouldn't change during the function," and then down the road they find a reason why it would be convenient to change it, and then they either make a local copy of a local variable, wasting memory, or go back and remove the const, defeating its purpose in the first place.

That makes no sense. Integers are passed by value. You can do:

void f(int x) {
    x = 10;
}

And it wouldn't change the value for the caller.
Logged
increpare
Guest
« Reply #27 on: February 27, 2010, 01:19:09 PM »

Quote from: Average Software
If you can't tell me what it's value is at compile time, don't tell me it's const.  (Pointers/references to const are a different story, those serve a different purpose)

Code:
void Funk(const int x)

My first question to someone that wrote this would be, "What is the value of x?"  The answer will of course be, "Whatever the caller passed."  So it's not really constant, is it?  The function Funk has every right to modify that data, and should be allowed to do so.
(Ignoring the issue with const int Tongue )

But const means something different in this context, and you know that.  I'll accept that you have pragmatic reason for this, but it does seem to indicate that you have no time whatsoever for const correctness - this is weird for me, because it's one of the easier/effective compiler-enforced safety mechanisms in C++.

I think it's important to know what functions are modifying and what they're not modifying.  The easiest compiler-enforced (that's not to say it can't be got around) way to do this is with const-declarations.

Quote
then down the road they find a reason why it would be convenient to change it, and then they either make a local copy of a local variable, wasting memory, or go back and remove the const, defeating its purpose in the first place.
removing the const doesn't defeat the initial purpose - if the function changes its behaviour, then the situation changes, and 'const' doesn't apply.  However, this can cause headaches further upstream if functions calling that function were written assuming const behaviour so you might get things cascading out a little.  

Also: I take it that you're not a fan of the 'mutable' keyword so? Tongue
« Last Edit: February 27, 2010, 01:30:17 PM by increpare » Logged
st33d
Guest
« Reply #28 on: February 27, 2010, 01:25:05 PM »

Quote
Code:
void Funk(const int x)

This line of code strikes me as a bit bonkers. Is there an actual reason why it would make sense to do that?
Logged
dspencer
Level 3
***


View Profile WWW
« Reply #29 on: February 27, 2010, 01:31:57 PM »

If its const you might as well pass a reference rather than pass by value.
Logged

Zaphos
Guest
« Reply #30 on: February 27, 2010, 01:47:16 PM »

Quote
Code:
void Funk(const int x)

This line of code strikes me as a bit bonkers. Is there an actual reason why it would make sense to do that?
People would do this:
Code:
void Funk(const BiggerDataStructure &x)
for when they pass x by reference purely to avoid copying, but otherwise x is not something the function should change.

edit, also:
If you don't need negative values, make it unsigned:

By changing the type of a piece of data, you eliminate an entire class of errors.  Similarly...
You also introduce some possible errors; for example if you try to compare a signed int with an unsigned int, and the signed int is negative, it will give a 'wrong' result due to converting it to unsigned before doing the comparison.  Or if you try to a for loop that counts down instead of up, and do "for (i=max; i >= 0; --i)", that will not have the expected behavior if i is unsigned.
« Last Edit: February 27, 2010, 02:02:59 PM by Zaphos » Logged
Glaiel-Gamer
Guest
« Reply #31 on: February 27, 2010, 02:05:57 PM »

classes are named as such:
Code:
class ClassName;

functions are named as such:
Code:
void functionName

variables are named like such:
Code:
variable
(i avoid having 2 word variable names, and if i do I do it one of these ways:
Code:
variablename
Code:
variableName
Code:
variable_name

constants + macros are all caps
Code:
const int NUM_BITCHES = 9000;
Code:
#define BALL_H

if a method returns a bool, i usually prefix it with "is"
Code:
bool isColliding();
bool isInBounds();

if I have a method that operates on an object, I name it as a command
Code:
void Vec2D::rotate(float degrees);

if I have a function that returns a copy, I name it in past tense
Code:
Vec2D Vec2D::rotated(float degrees);

also for .normalize(), .normalized(), scale, scaled, etc.

I only bother implementing a class "correctly" (with const functions, copy constructor / assignment operator, etc) if it's a highly used utility class (Vec2D, Array2D, VBO), otherwise assignment and copy usually get thrown in private and not implemented unless I find I need them.

underscores before/after variable names are only used in situation where something will clash, usually only in function arguments
Code:
Vec2D::Vec2D(float x_, float y_):x(x_), y(y_){}

operator overloading is reserved to only cases where it makes sense (mostly math + container classes)

brackets and indenting go as such (2 spaces = indent, no tab characters):

Code:
if(something){
  do shit();
  do more shit();
  done();
}

the exception is huge constructors
Code:
Massive::Massive(int a, int b, int c, int d, int e, int f)
: flarg(a), bwfhuw(b), wuefh(c), wufjbfujef(d), kijbebhwbef_AHDHF(e), efehbfgb(f)
{
  code();
}

empty functions have the brackets on the same line
Code:
void doNothing(){}


if the brackets are left out of an if statement, it goes all on one line
Code:
if(outofbounds()) return;

if that one line is super huge though, it goes on 2 and gets brackets

I avoid doing the same with else, but if it's short it gets one line too.

line breaks go after logical blocks.

Code:
int temp = a;
a = b;
b = temp;

v.push_back(a);
v.push_back(b);
v.sort();

return v;

enums are embedded in structs
Code:
struct Types {
  enum {
    ball = 0,
    cat,
    pepper,
    bottle,
    tnt,
  };
};

or prefixed and capital
Code:
enum Controls {
  CONTROL_NONE = 0
  CONTROL_JUMP = 1,
  CONTROL_HIT = 2,
  CONTROL_DIE = 4,
  CONTROL_EAT = 8,
};

structs are used when there's no functions (except a simple constructor that doesn't allocate on the heap)
classes are used anywhere else

pass by pointer when you need to modify an object
Code:
void sortArray(Array* arr);

pass by const reference when you dont need to modify an object
Code:
void printArray(const Array& print);

I sometimes break the first rule.

if a function allocates on the heap and returns (and expects you to manually delete it), it's prefixed with "new"
Code:
Array* arr = newFibArray(20);
delete arr;
(i try to avoid doing this when possible)

Reference counting is ONLY to be used for shared resources like textures, animations, and sprites, and they will all be stored in a container somewhere anyway.
I hate shared pointers or reference counting for everything, it ends up CAUSING memory leaks when you can't find which objects still hold a pointer to something. Much better to let it crash, hit the call stack, and find out for yourself and fix it right then and there. Also means I have to be careful about the order I delete things, but I rarely run into a situation where that becomes an issue.

unsigned will ONLY be used for bitfields and "raw bytes", and the rare situation where I need an int larger than 2 billion.

pointers and reference go next to the type, not the name

Code:
int* i;
int& i;
void foo(int* i);
void bar(int& i);

Templates will be used SPARINGLY, except for STL containers.
« Last Edit: February 27, 2010, 02:09:06 PM by Glaiel-Gamer » Logged
Zaphos
Guest
« Reply #32 on: February 27, 2010, 02:15:47 PM »

if I have a method that operates on an object, I name it as a command
Code:
void Vec2D::rotate(float degrees);

if I have a function that returns a copy, I name it in past tense
Code:
Vec2D Vec2D::rotated(float degrees);

also for .normalize(), .normalized(), scale, scaled, etc.
Ah, that's a nice rule, I might adopt it.
Logged
Average Software
Level 10
*****

Fleeing all W'rkncacnter


View Profile WWW
« Reply #33 on: February 27, 2010, 02:17:23 PM »

But const means something different in this context, and you know that.  I'll accept that you have pragmatic reason for this, but it does seem to indicate that you have no time whatsoever for const correctness - this is weird for me, because it's one of the easier/effective compiler-enforced safety mechanisms in C++.

Const correctness is a wonderful thing, and I go well out my way to make sure I have it, hell, I even maintain volatile correctness sometimes.  This, however, has nothing to do with const correctness.

Const correctness is about preventing code from modifying data that it shouldn't modify.  In this situation, the function has every right to modify the value parameter without affecting any other piece of code.  Changing it harms no one, the function owns the parameter.

Quote
I think it's important to know what functions are modifying and what they're not modifying.  The easiest compiler-enforced (that's not to say it can't be got around) way to do this is with const-declarations.

What a function does or does not do with a value parameter is the function's business and nobody else's.  If you code based on what a function does with its own values internally, you're depending on things you shouldn't be depending on.

Quote
Also: I take it that you're not a fan of the 'mutable' keyword so? Tongue

I love mutable, when used correctly.

Quote
You also introduce some possible errors; for example if you try to compare a signed int with an unsigned int, and the signed int is negative, it will give a 'wrong' result due to converting it to unsigned before doing the comparison.

I get warnings for this, I bet most if not all compilers will give you one if you ask for it.

Quote
Or if you try to a for loop that counts down instead of up, and do "for (i=max; i >= 0; --i)", that will not have the expected behavior if i is unsigned.

Then you are expecting the wrong thing.  This is a case where you need a negative number, therefore you shouldn't be using an unsigned one, which was part of my original point.
Logged



What would John Carmack do?
increpare
Guest
« Reply #34 on: February 27, 2010, 02:20:01 PM »

I agree with what you said there (given that it was based around a misunderstanding), but I'm still getting some tension between what you're saying now and what you said before

Const correctness is about preventing code from modifying data that it shouldn't modify.
vs
Quote
If you can't tell me what it's value is at compile time, don't tell me it's const.  
« Last Edit: February 27, 2010, 02:34:45 PM by increpare » Logged
Zaphos
Guest
« Reply #35 on: February 27, 2010, 02:23:14 PM »

I get warnings for this, I bet most if not all compilers will give you one if you ask for it.
You can enable warnings for it but you have to be aware of it, or be attentive enough to both use -Wall (or equivalent) and attend to all the warnings.  It is surprising enough to be a possible source of bugs, especially for new programmers, I think.

Then you are expecting the wrong thing.  This is a case where you need a negative number, therefore you shouldn't be using an unsigned one, which was part of my original point.
Well it's not like i ever needs to go negative except to satisfy the stopping criteria there, so you could write the code differently and keep i unsigned; this is not a case where you 'need' a negative number unless you look at it very narrowly.  And yes, obviously "then you are expecting the wrong thing," but that is true of most cases where bugs would arise, the point is that 'unsigned' makes it easy to expect the wrong thing.
Logged
Average Software
Level 10
*****

Fleeing all W'rkncacnter


View Profile WWW
« Reply #36 on: February 27, 2010, 02:41:27 PM »

I agree with what you said there, but I'm getting some tension between what you're saying now and what you said before

Const correctness is about preventing code from modifying data that it shouldn't modify.
vs
Quote
If you can't tell me what it's value is at compile time, don't tell me it's const. 


I think what you're missing is that I'm not at all referring to pointers/references to const.  I'm talking about const values.

Const correctness boils down to: "This data doesn't belong to you.  I'm going to let you use it, but you can't change it."  This is accomplished via pointers/references to const.  I have no quibbles with this whatsoever, it's a very important part of C/C++/Ada.

When I say "shouldn't modify" this is what I'm talking about.  Somebody else is relying on that data, it's not yours, don't touch it.

A function's value parameters are effectively local variables that belong to the function.  The functions owns them, they don't affect anyone else, therefore the function should be able to do whatever the hell it wants with them.  Declaring a value parameter as const places a pointless and unhelpful restriction on the function's implementation.  It doesn't even generate any optimizer benefits.

Quote from: Zaphos
Insert Quote
Quote from: Average Software on Today at 02:17:23 PM
I get warnings for this, I bet most if not all compilers will give you one if you ask for it.
You can enable warnings for it but you have to be aware of it, or be attentive enough to both use -Wall (or equivalent) and attend to all the warnings.

Quote from: Average Software on Today at 02:17:23 PM
Then you are expecting the wrong thing.  This is a case where you need a negative number, therefore you shouldn't be using an unsigned one, which was part of my original point.
Well it's not like i ever needs to go negative except to satisfy the stopping criteria there, so you could write the code differently and keep i unsigned; this is not a case where you 'need' a negative number unless you look at it very narrowly.  And yes, obviously "then you are expecting the wrong thing," but that is true of most cases where bugs would arise, the point is that 'unsigned' makes it easy to expect the wrong thing.

I should mention that these as primarily C problems.  Most of these days, I do my coding in Ada.  Ada doesn't perform implicit conversions, so the comparison thing would actually be a compile error.

It also does for loops like this:

Code:
for X in reverse Max .. 0 loop

Which makes the second point moot as well.

I still heavily use unsigned in C++, it's just something you have to learn how do.
Logged



What would John Carmack do?
Glaiel-Gamer
Guest
« Reply #37 on: February 27, 2010, 02:45:38 PM »

oh any my coding motto is:

"Better to write the code "wrong" in 5 minutes, then fix it in the future, than to write the code "right" in a day, and have to fix it in the future anyway"
Logged
increpare
Guest
« Reply #38 on: February 27, 2010, 03:03:38 PM »

gotcha mr average   Hand Thumbs Up Right
Logged
BorisTheBrave
Level 10
*****


View Profile WWW
« Reply #39 on: February 27, 2010, 03:42:46 PM »

You guys realize
int f(const int i)
and
int f(int i)
have the same signature? Outermost consts are completely ignored by the compiler, except creating compile errors if you try to modify i in the const version.

I really liked Glaiel's system - it's close to what I already do, but better thought out. Although I don't bother with:
Code:
Vec2D::Vec2D(float x_, float y_):x(x_), y(y_){}
Since I discovered that it compiles just fine without the underscores, as there is no ambiguity for initializer lists.
Logged
Pages: 1 [2] 3 4 ... 7
Print
Jump to:  

Theme orange-lt created by panic