C++ / Qt Coding style - where should a #define go

Question

I am trying to build a break-out game clone in Qt. I need to figure out what type of a QGraphicsItem my ball collided with. For example if I the ball collides with the wall, the ball just bounces off, if it collides with a brick it has to bounce off and destroys the brick. To find out what kind of QGraphicsItem it is, I figured the best way to do that is probably to override QGraphicsItem::type() (Please let me know if this is not the right way!).

In the following code for brick.h I set my 'Brick' to have a Type of 3. Now, the value 3 looks really cumbersome to track. I would instead prefer to declare something with a '#define'

#include <QGraphicsItem>

//should this #define be here?
//#define BRICK_SPRITE 3

class Brick : public QGraphicsItem
{
public:
    Brick(const QPixmap& p, QGraphicsScene *scene = 0);
    virtual QRectF boundingRect() const;
    virtual void paint( QPainter *painter,
                        const QStyleOptionGraphicsItem *option,
                        QWidget *widget );

    QPainterPath shape() const;

    enum { Type = 3 }; //enum {Type = BRICK_SPRITE}

    int type() const { return Type; }

private:
    QPixmap pixmap;
};

Where is a good location to place the statement '#define BRICK_SPRITE 3' ? I have several other files in the project. Should I place all the definitions in a separate header file?

Attila · Accepted Answer · 2012-05-15T16:42:59.947

5

Why not just use Type instead of 3? enums in c++ are implicitly convertible to int

If you really wanted a new name instead, I suggest you use a const int variable instead of a #define -- it is type and namespace safe, while preprocessor macros aren't.

Something like:

class Brick : public QGraphicsItem
{
  static const int BRICK_SPRITE = 3;
  // rest of the class definition
};

According to the documentations I could find, the approach you are taking with the enum and overriding type() is indeed the preferred way

edited May 15 '12 at 16:42

answered May 15 '12 at 16:36

Attila

28,265
3
46
55

The enum hack should no longer be needed for decent compilers, the inline static const int is the best way. – DanDan May 15 '12 at 16:51
@Atilla - I am not sure if I understood your statement 'Why not just use Type instead of 3' . If I use enum { Type}; , Type would automatically be 0. Is this what you meant? I want the value of Type to be 3. There is no particular reason for that. I guess my reason is I want it to be unique. – Mathai May 15 '12 at 17:36
No, I meant to keep the enum defintion as you have now and use `Type` when you would want to use `BRICK_SPRITE` -- if the name `Type` is not expressive enough for you, you can change it to `BRICK_TYPE` in the enum, or define a named constant as suggested in my answer – Attila May 15 '12 at 17:37

score 5 · Answer 2 · edited May 23 '17 at 10:33

If you want to follow Qt's conventions, you can start by not naming your constants in all caps...they don't do that. Also, using preprocessor macros for constants is not idiomatic C++, as @Atilla points out. There are lots of good reasons, like being able to see the value in the debugger:

C++ - enum vs. const vs. #define

It's usually the case that you can find a class which it makes sense to "own" constants. When Qt doesn't really have a place to put it, it just puts things in the namespace Qt::

http://doc.qt.io/archives/qt-4.7/qt.html

So if you find it doesn't make sense for Brick to own this value, you can have a namespace in your program where you put such things.

I actually didn't know QGraphicsItem::type() was a standard thing...hadn't come across it. You've noticed in the documentation that it says custom user types should start at UserType + 1...so I'd definitely follow that rule. It says you only need to do this if you plan on using qgraphicsitem_cast(), so maybe the examples I've looked at just didn't need it.

The only unusual thing Qt brings to the table (that I can think of) in terms of conventions for declaring constants is something called Q_DECLARE_FLAGS, which brings type-safety in cases where you OR together flags:

http://doc.qt.io/archives/qt-4.7/qflags.html

So if you were doing something like:

class Brick : public QGraphicsItem {
public:
    static const int Type = UserType + 1;
    enum BrickFlag {
        DamagesBall = 0x1,
        ContainsCoins = 0x2,
        MultipleHits = 0x4
    };
    Q_DECLARE_FLAGS(BrickFlags, BrickFlag)
}

Q_DECLARE_OPERATORS_FOR_FLAGS(Brick::BrickFlags)

Now you have a special safe type called Brick::BrickFlags, that cannot (for instance) be OR'd together with Brick::Type.

Following your advise and Attila's, I am going to change the declaration to static const int. By the way I saw a line in Qt documententation enum { Type = UserType + 1 }; UserType in QGraphicsItem is 65536. I wonder if I should follow this instead of assigning a value 3 (which I believe will be unique in my application). If I use UserType+1 for brick, I guess I will have to have a line static const int BrickSprite = UserType + 1; — Mathai, May 15 '12 at 17:42
Ah. I didn't know type() was a standard thing. Updated my answer to reflect this. — HostileFork says dont trust SE, May 15 '12 at 17:52

C++ / Qt Coding style - where should a #define go

2 Answers2