Thread: Preprocessor token use and abuse - good or bad

I have a tendency to name things according to a letter count, so for example when mapping the OpenGL buffer hints I would have something like this:

Code:

void InitGfxBufEnums()
{
#define ENUM( NAME, SRC, DST ) \
	GfxBuf##NAME##Enums[SRC] = #SRC; \
	ApiBuf##NAME##Enums[SRC] = #DST; \
	ApiBuf##NAME##Types[SRC] = DST;

	ENUM( Type, E_BUFF_TYPE_NONE, 0 );
	ENUM( Type, E_BUFF_TYPE_VERTEX, GL_ARRAY_BUFFER );
	ENUM( Type, E_BUFF_TYPE_INDICE, GL_ELEMENT_ARRAY_BUFFER );

	ENUM( Hint, E_BUFF_HINT_DYNAMIC_COPY, GL_DYNAMIC_COPY );
	ENUM( Hint, E_BUFF_HINT_DYNAMIC_DRAW, GL_DYNAMIC_DRAW );
	ENUM( Hint, E_BUFF_HINT_DYNAMIC_READ, GL_DYNAMIC_READ );
	ENUM( Hint, E_BUFF_HINT_FLOWING_COPY, GL_STREAM_COPY );
	ENUM( Hint, E_BUFF_HINT_FLOWING_DRAW, GL_STREAM_DRAW );
	ENUM( Hint, E_BUFF_HINT_FLOWING_READ, GL_STREAM_READ );
	ENUM( Hint, E_BUFF_HINT_LASTING_COPY, GL_STATIC_COPY );
	ENUM( Hint, E_BUFF_HINT_LASTING_DRAW, GL_STATIC_DRAW );
	ENUM( Hint, E_BUFF_HINT_LASTING_READ, GL_STATIC_READ );
#undef ENUM
}

Today I was looking at various synonyms of volatile (which I thought would be considered a synonym for dynamic, wasn't on the site I was visiting), and saw "fickle" as an option for a 6 letter naming scheme that mostly used the original opengl names (so static & stream would remain unchanged but dynamic would be swapped out for fickle), what I was wandering is how different programmers would view such a name, incidentally for buffer functions I tend to use these types of names for various actions:

Code:

VoidBuffer - Terminate & delete the object
MakeBuffer - Create the object where information would be stored
TermBuffer - Terminate the object, this deleting the array it points to if any & clearing the element size
InitBuffer - Initialise the object with a element size and - if a count/total is given - an array, also if indicated then reserve member 0 as an invalid index and zero it
TrimBuffer - Remove elements from the end of the array
GrowBuffer - "Insert" more elements into the end of the array
CramBuffer - Insert more elements into position x, pushing any existing elements from x across to make room (so inserting "Hello " into position 0 of "World!" would create "Hello World!")
BindMember - If the member is not found in the array add it to the end, in either case give the index
OustMember - Zero the member at position x, if a state is given instead, search for the first member with said state & clear that one
MarkMember - Fill the member at position x with given state
SeekMember - Read the state of the member at position x

I'm considering renaming "Oust" to plain "Null" or "Zero" & re-using Oust for removing members from the middle of a buffer instead of just the end like TrimBuffer does

So yeah, this thread is just for opinions.

I take it by the lack of responses none of the people who did look at the thread have any particular opinion to give, which I take to mean that there wouldn't be any particular problems if an external programmer saw the names, no confusion, no serious slow downs in programming new features etc.

FWIW, I find your code excessively hard to read.

Abusing ## for no good reason like "I have a tendency to name things according to a letter count" only compounds the problem.

There you go - an opinion.

Originally Posted by Salem

FWIW, I find your code excessively hard to read.

Abusing ## for no good reason like "I have a tendency to name things according to a letter count" only compounds the problem.

There you go - an opinion.

Care to elaborate on the "abusing ##" part? I found my usage of it quite reasonable, it keeps the code clutter free & consistent in places where it should be, for example the window inputs, it's absolutely stupid to manually do all those set callback functions if all that's needed is a a quick macro and a few manual names, or the wrapped opengl set uniform functions, absolutely ridiculous to manually wrap them all if the general format of the wrapper is going to be the same. There's a place for manual scripting and those are not it.

As for the letter count I do it because it helps make my code look tidier/aesthetically pleasing, the easier on the eyes code is, the more I want to work with it. As for finding my general code hard to read, mind elaborating on that too? Not gonna improve if I don't have any specifics to even acknowledge, let alone decide if I'll try to change them.

Originally Posted by hamster_nz

The idea behind ## is to help remove complexity, not add it. With good code you shouldn't have to get into the original author's mindset to understand what they are doing.

There wasn't even a comment to explain what was going on.

The code as written meant nothing to me - I could not understand what it was doing, or why you want to do it.

So I ran it though the preprocessor and got this:

Code:

void InitGfxBufEnums()
{
    GfxBufTypeEnums[E_BUFF_TYPE_NONE] = "E_BUFF_TYPE_NONE"; ApiBufTypeEnums[E_BUFF_TYPE_NONE] = "0"; ApiBufTypeTypes[E_BUFF_TYPE_NONE] = 0;;
    GfxBufTypeEnums[E_BUFF_TYPE_VERTEX] = "E_BUFF_TYPE_VERTEX"; ApiBufTypeEnums[E_BUFF_TYPE_VERTEX] = "GL_ARRAY_BUFFER"; ApiBufTypeTypes[E_BUFF_TYPE_VERTEX] = GL_ARRAY_BUFFER;;
    GfxBufTypeEnums[E_BUFF_TYPE_INDICE] = "E_BUFF_TYPE_INDICE"; ApiBufTypeEnums[E_BUFF_TYPE_INDICE] = "GL_ELEMENT_ARRAY_BUFFER"; ApiBufTypeTypes[E_BUFF_TYPE_INDICE] = GL_ELEMENT_ARRAY_BUFFER;;


    GfxBufHintEnums[E_BUFF_HINT_DYNAMIC_COPY] = "E_BUFF_HINT_DYNAMIC_COPY"; ApiBufHintEnums[E_BUFF_HINT_DYNAMIC_COPY] = "GL_DYNAMIC_COPY"; ApiBufHintTypes[E_BUFF_HINT_DYNAMIC_COPY] = GL_DYNAMIC_COPY;;
    GfxBufHintEnums[E_BUFF_HINT_DYNAMIC_DRAW] = "E_BUFF_HINT_DYNAMIC_DRAW"; ApiBufHintEnums[E_BUFF_HINT_DYNAMIC_DRAW] = "GL_DYNAMIC_DRAW"; ApiBufHintTypes[E_BUFF_HINT_DYNAMIC_DRAW] = GL_DYNAMIC_DRAW;;
    GfxBufHintEnums[E_BUFF_HINT_DYNAMIC_READ] = "E_BUFF_HINT_DYNAMIC_READ"; ApiBufHintEnums[E_BUFF_HINT_DYNAMIC_READ] = "GL_DYNAMIC_READ"; ApiBufHintTypes[E_BUFF_HINT_DYNAMIC_READ] = GL_DYNAMIC_READ;;
    GfxBufHintEnums[E_BUFF_HINT_FLOWING_COPY] = "E_BUFF_HINT_FLOWING_COPY"; ApiBufHintEnums[E_BUFF_HINT_FLOWING_COPY] = "GL_STREAM_COPY"; ApiBufHintTypes[E_BUFF_HINT_FLOWING_COPY] = GL_STREAM_COPY;;
    GfxBufHintEnums[E_BUFF_HINT_FLOWING_DRAW] = "E_BUFF_HINT_FLOWING_DRAW"; ApiBufHintEnums[E_BUFF_HINT_FLOWING_DRAW] = "GL_STREAM_DRAW"; ApiBufHintTypes[E_BUFF_HINT_FLOWING_DRAW] = GL_STREAM_DRAW;;
    GfxBufHintEnums[E_BUFF_HINT_FLOWING_READ] = "E_BUFF_HINT_FLOWING_READ"; ApiBufHintEnums[E_BUFF_HINT_FLOWING_READ] = "GL_STREAM_READ"; ApiBufHintTypes[E_BUFF_HINT_FLOWING_READ] = GL_STREAM_READ;;
    GfxBufHintEnums[E_BUFF_HINT_LASTING_COPY] = "E_BUFF_HINT_LASTING_COPY"; ApiBufHintEnums[E_BUFF_HINT_LASTING_COPY] = "GL_STATIC_COPY"; ApiBufHintTypes[E_BUFF_HINT_LASTING_COPY] = GL_STATIC_COPY;;
    GfxBufHintEnums[E_BUFF_HINT_LASTING_DRAW] = "E_BUFF_HINT_LASTING_DRAW"; ApiBufHintEnums[E_BUFF_HINT_LASTING_DRAW] = "GL_STATIC_DRAW"; ApiBufHintTypes[E_BUFF_HINT_LASTING_DRAW] = GL_STATIC_DRAW;;
    GfxBufHintEnums[E_BUFF_HINT_LASTING_READ] = "E_BUFF_HINT_LASTING_READ"; ApiBufHintEnums[E_BUFF_HINT_LASTING_READ] = "GL_STATIC_READ"; ApiBufHintTypes[E_BUFF_HINT_LASTING_READ] = GL_STATIC_READ;;
}

(note the ";;" on the end of lines).

So then I had a look at it and refactored it:

Code:

static inline void AddTypeEnumMapping(int src, char *src_name, int dest, dest_name) {
    GfxBufTypeEnums[src] = src_name;
    ApiBufTypeEnums[src] = dest_name;
    ApiBufTypeTypes[src] = dest;
};


static inline void AddHintEnumMapping(int src, char *src_name, int dest, dest_name) {
    GfxBufHintEnums[src] = src_name;
    ApiBufHintEnums[src] = dest_name;
    ApiBufHintTypes[src] = dest;
};


void InitGfxBufEnums()
{
// Macros to stringyfy enum names to save typing
#define ADD_TYPE(src, dest)  AddTypeEnumMapping(src,#src,dest,#dest)
#define ADD_HINT(src, dest)  AddHintEnumMapping(src,#src,dest,#dest)
   ADD_TYPE(E_BUFF_TYPE_NONE,   0);
   ADD_TYPE(E_BUFF_TYPE_VERTEX, GL_ARRAY_BUFFER);
   ADD_TYPE(E_BUFF_TYPE_INDICE, GL_ELEMENT_ARRAY_BUFFER);


   ADD_HINT(E_BUFF_HINT_DYNAMIC_COPY, GL_DYNAMIC_COPY );
   ADD_HINT(E_BUFF_HINT_DYNAMIC_DRAW, GL_DYNAMIC_DRAW );
   ADD_HINT(E_BUFF_HINT_DYNAMIC_READ, GL_DYNAMIC_READ );
   ADD_HINT(E_BUFF_HINT_FLOWING_COPY, GL_STREAM_COPY );
   ADD_HINT(E_BUFF_HINT_FLOWING_DRAW, GL_STREAM_DRAW );
   ADD_HINT(E_BUFF_HINT_FLOWING_READ, GL_STREAM_READ );
   ADD_HINT(E_BUFF_HINT_LASTING_COPY, GL_STATIC_COPY );
   ADD_HINT(E_BUFF_HINT_LASTING_DRAW, GL_STATIC_DRAW );
   ADD_HINT(E_BUFF_HINT_LASTING_READ, GL_STATIC_READ );
#undef ADD_TYPE
#undef ADD_HINT
}

This is what comes out of the preprocessor:

Code:

static inline void AddTypeEnumMapping(int src, char *src_name, int dest, dest_name) {
    GfxBufTypeEnums[src] = src_name;
    ApiBufTypeEnums[src] = dest_name;
    ApiBufTypeTypes[src] = dest;
};


static inline void AddHintEnumMapping(int src, char *src_name, int dest, dest_name) {
    GfxBufHintEnums[src] = src_name;
    ApiBufHintEnums[src] = dest_name;
    ApiBufHintTypes[src] = dest;
};


void InitGfxBufEnums()
{


   AddTypeEnumMapping(E_BUFF_TYPE_NONE,"E_BUFF_TYPE_NONE",0,"0");
   AddTypeEnumMapping(E_BUFF_TYPE_VERTEX,"E_BUFF_TYPE_VERTEX",GL_ARRAY_BUFFER,"GL_ARRAY_BUFFER");
   AddTypeEnumMapping(E_BUFF_TYPE_INDICE,"E_BUFF_TYPE_INDICE",GL_ELEMENT_ARRAY_BUFFER,"GL_ELEMENT_ARRAY_BUFFER");
   AddHintEnumMapping(E_BUFF_HINT_DYNAMIC_COPY,"E_BUFF_HINT_DYNAMIC_COPY",GL_DYNAMIC_COPY,"GL_DYNAMIC_COPY");
   AddHintEnumMapping(E_BUFF_HINT_DYNAMIC_DRAW,"E_BUFF_HINT_DYNAMIC_DRAW",GL_DYNAMIC_DRAW,"GL_DYNAMIC_DRAW");
   AddHintEnumMapping(E_BUFF_HINT_DYNAMIC_READ,"E_BUFF_HINT_DYNAMIC_READ",GL_DYNAMIC_READ,"GL_DYNAMIC_READ");
   AddHintEnumMapping(E_BUFF_HINT_FLOWING_COPY,"E_BUFF_HINT_FLOWING_COPY",GL_STREAM_COPY,"GL_STREAM_COPY");
   AddHintEnumMapping(E_BUFF_HINT_FLOWING_DRAW,"E_BUFF_HINT_FLOWING_DRAW",GL_STREAM_DRAW,"GL_STREAM_DRAW");
   AddHintEnumMapping(E_BUFF_HINT_FLOWING_READ,"E_BUFF_HINT_FLOWING_READ",GL_STREAM_READ,"GL_STREAM_READ");
   AddHintEnumMapping(E_BUFF_HINT_LASTING_COPY,"E_BUFF_HINT_LASTING_COPY",GL_STATIC_COPY,"GL_STATIC_COPY");
   AddHintEnumMapping(E_BUFF_HINT_LASTING_DRAW,"E_BUFF_HINT_LASTING_DRAW",GL_STATIC_DRAW,"GL_STATIC_DRAW");
   AddHintEnumMapping(E_BUFF_HINT_LASTING_READ,"E_BUFF_HINT_LASTING_READ",GL_STATIC_READ,"GL_STATIC_READ");
}

I'll leave it up to yourself and others do discuss which is better and why, but found the original unintelligible.

So you couldn't work out that X##Y becomes Z or #X becomes "X", good to know you have no understanding of the preprocessor, I'll continue writing code that assumes the next developer has that fundamental understanding that only beginners don't have thank you very much, didn't even think of "emit" either, my 1st thought was "echo" like the command that is available on every shell, linux or otherwise, the letter limit thing is annoying I'll agree but it's not a habit I can easily kick, starting to feel a bit like ocd to me I think, I do question what the point in using monospace fonts is if you don't make use of it's main usage point, to line things up, well that variable tab width is the most annoying to me though, I eventually settled on just using a width of 8 for the little bit less tabs I use in exhange.

Originally Posted by hamster_nz

Shrug - If you don't want an opinion then why did you ask. "So yeah, this thread is just for opinions" were your exact words.

And you are right that I had to doublecheck that what I thought was going on was actually going on, because it was somewhat obscure and opaque. And we all know that Obscure and Opaque are two features that are valuable in code designed to be used and understood by others. At least I knew how I could check what was going on, and verify exactly what you were doing - a job many others would find challenging.

I pity the next person who comes along, and should the debugger stops at "ENUM( Type, E_BUFF_TYPE_INDICE, GL_ELEMENT_ARRAY_BUFFER );" is left scratching their head. Only to find out that "GfxBufTypeEnums[]" has been declared too small.

However, it more than likely that the debugger won't stop, and your code will just trash a random memory location somewhere, giving them an almost unsolvable bug that will haunt their nightmares.

If you wanted to write robust code, if it was broken out into a function you could at least do something like this:

Code:

static inline void AddTypeEnumMapping(int src, char *src_name, int dest, dest_name) {
    assert(src >= 0 &&  src < sizeof(GfxBufTypeEnums)/sizeof(char *));
    GfxBufTypeEnums[src] = src_name;
    ApiBufTypeEnums[src] = dest_name;
    ApiBufTypeTypes[src] = dest;
};

After a few decades of coding, you will come to the realization that it isn't how cunning your code is that matters most, but how simple it is. It can take a lot of skill to write cunning code too solve a tricky problem, but it takes even more skill to write simple code that does the same job.

A lot of my code to look as if it was written by a four year old, and that is the way I like it. Others appreciate me for it.

The mini function, fine I can agree with that, the forcing the full enum name when just part of it will do however I don't, if the enums share a common start/end like the ones I was using then it is UNREASONABLE to force using the whole enum, especially when you're just filling a static array of fixed size and only during initialisation of the wrapper, that's like expecting a hand drill to do a better job than an electric drill, it's VERY rare that's the case and more often than not you should default to the automatic way, only use manual when you either have to or the automated way is more wasteful than simply doing it manually.