0

I thinked at two techniques for my #define :

Technique 1

SomeClass.h:

#define SOME_BUFFER_LENGTH 256

class SomeClass {
  /* ... */
}; 

Technique 2

Config.h:

#define SOME_BUFFER_LENGTH 256
#define SOME_OTHER_THING_USED_ELSEWHERE 23

SomeClass.h:

#include "Config.h"

#ifndef SOME_BUFFER_LENGTH
  #define SOME_BUFFER_LENGTH <Default Value>
#endif

class SomeClass {
  /* ... */
};

What are pros and cons of these two methods, and objectively for a big project, which is the best ?

Edit:

Hi, I learnt two things from your comments and answers :

@gnasher729: If there might be reasons to change it to a different value, then it's bad if you have to hunt down such constants in many different source files, but better to have it in a configuration file with the express intent to configure things.

and

@Phil1970: Better to use C++ constants instead of the processor.

So I end with this behavior:

Config.h:

namespace Config {
  const type constant = value;
  const type anotherconstant = anothervalue;
}

SomeClass.h:

#include "Config.h"
/* ... */
void somefunc() {
  usefulfunc(Config::constant, /* ... */);
}

I think it's perfectly suitable for an OpenSource project, I'm awaiting your point of views about this !

Nurrl
  • 179
  • 10
  • Possible duplicate of [How would you know if you've written readable and easily maintainable code?](https://softwareengineering.stackexchange.com/questions/141005/how-would-you-know-if-youve-written-readable-and-easily-maintainable-code) – gnat Aug 26 '17 at 14:07
  • see also [What is the problem with "Pros and Cons"?](https://softwareengineering.meta.stackexchange.com/q/6758/31260) – gnat Aug 26 '17 at 14:07
  • 5
    @Nurrl you should use `static constexpr` values and not defines, so the compiler can help you if you do something wrong. (you is here not necessarily you, but anyone who'll look at your code) – NaCl Aug 26 '17 at 14:15
  • 1
    @gnat: #defines are not addressed in that question. – Robert Harvey Aug 26 '17 at 14:35
  • 1
    You use separate .h files when those #defines are shared between applications. – Robert Harvey Aug 26 '17 at 14:59
  • @RobertHarvey So for a single app, I shouldn't create an external .h to group all settings in one file ? – Nurrl Aug 26 '17 at 15:10
  • It's all about scope. – Blrfl Aug 26 '17 at 15:19
  • 2
    Neither: the only reason to use `#define` in C++ is compiler-agnostic include guards. –  Aug 27 '17 at 04:38
  • As already said in other comments, better to use C++ constants instead of the processor. I would not recommand Technique 2 as ot could lead to different definitions. Put default values at the end of Config.h. If you need to be able to configure at different level, then have distinct files for things that are shared between apps or project... – Phil1970 Aug 28 '17 at 16:37

2 Answers2

6

It depends. Do you need that #define to avoid "magic constants", or do you use the #define to make it configurable?

If SOME_BUFFER_LENGTH needs to be 256, and you don't want to write 256 everywhere in your source code, keep it in the source file. If there might be reasons to change it to a different value, then it's bad if you have to hunt down such constants in many different source files, but better to have it in a configuration file with the express intent to configure things.

BTW. The "#ifndef" method has the big disadvantage that the outcome might depend on the order you are including header files, or whether you include them at all. Let's say I have two files containing the following:

#ifndef SOME_BUFFER_LENGTH
    #define SOME_BUFFER_LENGTH 256
#endif

and

#ifndef SOME_BUFFER_LENGTH
    #define SOME_BUFFER_LENGTH 1024
#endif

Changing the order in which you include the header files changes the value. Worse, if two source files include them in different orders, they will disagree about the value. Now imagine they both have a struct with a member "char buffer [SOME_BUFFER_LENGTH];". You are in trouble.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
2

Depends whether SOME_BUFFER_LENGTH is used in only one class/file, or must be available in different places. In the latter case, adhering to the SPOT rule and moving them to a common header may save you many a headache.

BTW, in C++ there are better ways than using type-unsafe defines/macros:

//Anonymous enum constant
enum
{
    SomeBufferLength = 256
}

//Named enum constant (C++11, type-safe)
enum class BufferLength : size_t
{
    SomeBuffer = 256
}

//Anonymous namespace (replaces local static variables/constants)
namespace
{
    const size_t SomeBufferLength = 256;
}

The enums may be part of the public or private interface of the class where they are used (or where they should reside when you stick to proper OO design) provided all methods that access them can include the declaration. And the anonymous namespace is valid in the local code file only, you can apply it if it's guaranteed not to be used anywhere else.

Murphy
  • 821
  • 6
  • 15