3

I found a piece of code as the following.

#define READWRITE READWIRTE
#define READWIRTE(varType, varName, funName)                \
private: varType varName;                                    \
public: varType get##funName(void) const { return varName; }\
public: void set##funName(varType var){ varName = var; }

READWIRTE(int, mSessionId, SessionId)

I want to know why not defining a function in a normal form. I mean, like this:

private:
  int mSessionId;
public:
  public int getSessionId() const;
  public void setSessionId(int sessionId); 

then define these two functions.

When should I use the preprocessor directives?

NiklausTseng
  • 235
  • 2
  • 10

6 Answers6

2

This macro will ensure that with each variable you have properly defined its setter and getter.

This will help in scenarios where your developer thinks writing variable, getter, and setter each time is a waste of time. He can simply call this macro. This will be easier for someone doing code review because he/she will know what to expect (in this case it will be macro) if you want setter and getters.

RJFalconer
  • 10,890
  • 5
  • 51
  • 66
user258367
  • 3,247
  • 2
  • 18
  • 17
2

As said by @user258367, this macro helps you in declaring private fields with public getter and setter.

If you have a whole bunch of those and do not have an IDE that does that low level coding for you, it can be an option, because :

READWIRTE(int, mSessionId, SessionId)

is more concise than the full code.

But unless it is a common usage in a corporate environment and included in a .h shared among many project, I would not use it :

  • writing first the fields, and later the getters and setters seems a more common usage
  • it encourages to always have getter and setter even one was useles (but you might have other macros for READONLY and WRITEONLY)
  • it leaves you implicitely in public: visibility which can be confusing

    private:
        int foo; // private, no need for getter or setter
        READWIRTE(int, mSessionId, SessionId);
        double bar; // is public !
    
    public:
        void othermethod(); 
        ...
    
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
2

Please don't do it.

The reason why it is done is that it saves you a bit of typing. The HUGE disadvantage is that it interferes badly with debugging. Say you see a call to setSessionId in an area of your code that might contain a bug. You search for the definition of setSessionId. And you can't find it. There is no definition of that method anywhere in your source code. Tools might not be clever enough to point you to the READ_WIRTE macro. So at that point you are totally stuck. Instead of spending five seconds looking for the method definition and seeing that it just sets a member variable and is likely harmless, you spend ages hunting down the definition of the function.

(Some people might argue that you should expect setSessionId to just set a member variable. Not true when you have a bug in your code. You know something doesn't do what you expect it to do).

gnasher729
  • 51,477
  • 5
  • 75
  • 98
2

The code looks to be intended to provide the user with a shorthand for adding a property to a class very simply. At first glance it appears a useful shortcut to prevent needless typing. However, simply adding a getter and setter round a member function may not always be the best route forward for the following reasons.

  1. In this instance, hardly any value is being added over simply adding a public member variable since the functions simply set or return the value.
  2. It's quite possible that one of the two functions may not be needed. For example if using RAII, the value is set at object construction, adding a setter may not be required or actually be undesirable.
  3. It might not support large objects at all or in an efficient manner.
  4. It won't (in it's current form) be thread-safe.
  5. Unless the code that uses this is also either generated or uses similar macros, at some point these function calls are going to be called by client code so the getters and setters will have to be coded by hand there (even though they don't strictly appear in the headers) and ...
  6. ... debugging will be more difficult when looking through the source code for the definition of, for example, setSessionId() it won't be found anywhere.

And while it's possible to achieve this with macros, the normal warnings on using macros apply. Conclusion would be to think twice before going down this route.

Community
  • 1
  • 1
Component 10
  • 10,247
  • 7
  • 47
  • 64
1

It seems that this is a macro for generating "properties" on classes. The author most probably uses it as a shortcut instead of typing the private member/accessor/mutator by hand.

Tasos Vogiatzoglou
  • 2,393
  • 12
  • 16
1

I see two good reasons to use macros:

  • like here, as a shorthand to avoid tedious typing;

  • to ensure portability of the code between different platforms or different variants of the code, when you don't want to wrap non-portable constructs (function calls) in extra functions.