A way to maintain a global set in many files by using preprocessing directives . Could we find a bet

[Xiangliang Meng]

Hi, all.

Recently, I find there is a way in our project to maintain a global set in
many files by using preprocessing directives. I'm wondering if we could find
a better method for this.

Many colors are referred in different subsystems in our projects. They are
defined as enumeration constants and a single color must be the same value
all across our projects.

-------------------color.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum colorSet
{
yellow = 0;
red = 1;
green = 2;
blue = 3;
pink = 4;
white = 5;
black = 6;
brown = 7;
cyan = 8;
purple = 9;
gray = 10;
};
#endif // _COLOR_
-------------------deskcolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum deskcolor
{
yellow = 0;
gray = 10;
};
#endif // _COLOR_
-------------------chaircolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum chaircolor
{
red = 1;
green = 2;
blue = 3;
black = 6;
};
#endif // _COLOR_
-------------------computercolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum computercolor
{
white = 5;
black = 6;
gray = 10;
};
#endif // _COLOR_
--------------------------------------

Every .c file in the different subsystems, include only a proper 'color' .h
file, but color.h will not be included by any .c file. For emaple, chair.c
includes chaircolor.h, and desk.c includes deskcolor.h. In order to prevent
including more than a 'color' .h file in a .c file by a mistake, all those
'color'. h files are using the same preprocessing directives.

When adding a new color into the color set, it's first added into color.h
and then only into the proper 'color' .h files if the subsystem really need
this kind of color.

What are the merits and defects of this method? As for me, it properly meets
our goal, but it's confusing for a newcomer and a little diffcult to
maintain. Who could point out more and provide a better method?

Best Regards,

Xiangliang Meng

 

[Michael Mair]

Hello,

Recently, I find there is a way in our project to maintain a global set in
many files by using preprocessing directives. I'm wondering if we could find
a better method for this.

If you do it in the right way: No.
However, your method is rather error prone.

Many colors are referred in different subsystems in our projects. They are
defined as enumeration constants and a single color must be the same value
all across our projects.

This is clear. Are there any restrictions that certain modules must
not know about certain colors? Or can everyone know about every color
enumeration constant?

-------------------color.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum colorSet
{
yellow = 0;
red = 1;
green = 2;
blue = 3;
pink = 4;
white = 5;
black = 6;
brown = 7;
cyan = 8;
purple = 9;
gray = 10;
};
#endif // _COLOR_

It is good to use the safe include mechanism but you should
not start your marker with an underscore, as this can lead
to problems with symbolic constants of C, your compiler or
your system. I would rather use something like COLOR_H
or PROJECTNAME_COLOR. If you are using C99, the // comment is
fine, otherwise change to /* .. */.

If you want to specify file dependent "valid" subsets of
enumeration constants, it is a good idea to add an INVALID
value to colorSet; if you want to run through a consecutive
range of enumeration constants, it is good to have constants
BEFORE and END:

enum colorSet {INVALID=-1,yellow,red, ...., gray};
or
enum colorSet {BEFORE=0,yellow,red, ...., gray, END};
(NOTE: yellow starts at 1 now; see below)

-------------------deskcolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum deskcolor
{
yellow = 0;
gray = 10;
};
#endif // _COLOR_
-------------------chaircolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum chaircolor
{
red = 1;
green = 2;
blue = 3;
black = 6;
};
#endif // _COLOR_
-------------------computercolor.h-------------------
#ifndef _COLOR_
#define _COLOR_
enum computercolor
{
white = 5;
black = 6;
gray = 10;
};
#endif // _COLOR_
--------------------------------------

This does not make sense to me, because you effectively preclude
yourself from using the same values throughout the whole project
without additional maintainance cost.

If it does not hurt that everyone knows all color constants, just
#include "color.h"
from all C files which need colors.

If you want to have only certain colors, you can do something
along the line of

-------------------deskcolor.h---------------------
#ifndef PROJECTNAME_DESKCOLOR
#define PROJECTNAME_DESKCOLOR

#include "color.h"

enum colorSet deskcolor[] = {yellow, gray, INVALID};

#endif /* PROJECTNAME_DESKCOLOR */
----------------------------------------------------

or, with C99 designated initializers,

-------------------deskcolor.h---------------------
#ifndef PROJECTNAME_DESKCOLOR
#define PROJECTNAME_DESKCOLOR

#include "color.h"

enum colorSet deskcolor[] = {
[yellow]=yellow, [gray]=gray, [END]=END;
};

#endif /* PROJECTNAME_DESKCOLOR */
----------------------------------------------------

In the first case, you can loop over all valid colors
stored in the array; in the second case, you can check
whether a color is valid by checking whether
deskcolor[colorname] is non-zero (that is the reason
for setting BEFORE to zero and having only non-zero
color values).

The second solution could also be achieved differently
but not with only these few lines.

Every .c file in the different subsystems, include only a proper 'color' .h
file, but color.h will not be included by any .c file. For emaple, chair.c
includes chaircolor.h, and desk.c includes deskcolor.h. In order to prevent
including more than a 'color' .h file in a .c file by a mistake, all those
'color'. h files are using the same preprocessing directives.

When adding a new color into the color set, it's first added into color.h
and then only into the proper 'color' .h files if the subsystem really need
this kind of color.

What are the merits and defects of this method? As for me, it properly meets
our goal, but it's confusing for a newcomer and a little diffcult to
maintain. Who could point out more and provide a better method?

I think I did point you towards one. To sum up my suggestion:

1) use your central definition of the color enumeration constants
_everywhere_. Only one enum definition.
2) If there are allowed and forbidden colors for certain purposes,
you can use "local" header files to enter them into arrays.
3) If there is no restriction whatsoever, do away with the
unnecessary "local" header files. One central color definition
is enough.

One small thing: I did not test the code above; I gave it just
to demonstrate the principle of the whole thing.

Why is this better than your approach?
You have to adjust several header files whenever something changes
with the central color definitions, whereas I have only to make
central changes in color.h.
Your approach is opaque as you need to maintain a *color.h list
of files to change when color.h changes.

Something else: It is generally a bad idea to use the same
safe include markers for different files -- users could still
#include "color.h" before "deskcolor.h", say, and you would
not realize it! If you want to make it impossible to use
more then one such header file, ask after the safe include
for the header for the marker of color.h and throw out
an error message if it is defined. After doing your
stuff in the header, #define the marker of color.h...


HTH
Michael