Is this a bad programming habit?

[lafoniz]

Hello, I would like to ask whether this habit is good or bad, I have read FAQ about using global variables, but I'm not sure does this situation is exactly the same bad as this described in wiki.

Let's assume I'm trying to write a player class, so i have a cpp and hpp file. In cpp file I write something like this:

#include "Player.hpp"

constexpr sf::Vector2f playerPos{100.0,300.0};

void Player::Player()
{
        player.setPosition(playerPos);
        ...
        ...
}

Someone may ask, why I'm doing something like this. Well when I have 6-7 parameters it's very nice to have them in one place, so configuration takes 3 seconds instead of 30, because I don't need to look for them. What should I do if this is not the best solution? I was thinking of static variable, but I'm not sure about this.

[Ixrec]

The way you're doing it, this is actually a very good thing.

The main reason globals are normally "evil" is that people make globals simply to allow several parts of their code to easily access and modify them.  This quickly devolves into an incomprehensible, undebuggable mess unless you have a flawless understanding of exactly who modifies the global in what ways at what times in what order...which nobody does.

The second--arguably less serious--reason is that they can "pollute the namespace" of code that's including your code.  By that I mean, if you put playerPos in Player.hpp (and you weren't using namespaces), then anybody including your player class would be unable to define playerPos without accidentally referring to your object.  This also quickly gets out of hand when a lot of other libraries are involved.

Having a global constant at the top of a .cpp file has neither of these problems.  No code is ever exposed to the name except for Player.cpp itself, and it's constant, so no matter what crazy stuff you do in Player.cpp it's never going to change in weird, unpredictable ways.  This is a perfectly standard thing to do, and far better than repeating {100.0,300.0} all over the place.


P.S. As proof that #2 actually happens in the real world: http://en.sfml-dev.org/forums/index.php?topic=16882.msg121225#msg121225  Admittedly, macros were involved, so it was extra evil, but you get the point.

[ChronicRat]

It is not bad.  It is much better than a magic number. "static const" is even better for C++. You can use #define PLAYER_POS 100.0, 300.0 but this C-way. =)
PS But player's position, for me, must be set from config file or script.
PPS Very good book - http://www.amazon.com/Coding-Standards-Rules-Guidelines-Practices/dp/0321113586

[Laurent]

To make it even better you should put it in an anonymous namespace (the C++ equivalent of "static" in C), so that it is not visible from other translation units.

[lafoniz]

Big thanks for every reply, learned a lot of from it, but I got one question.
When I use static prefix with an example constant or I will put this constant inside anonymous namespace - does it gonna reduce visibility to the file where it was created?

So for example let's have a file named test.cpp and we have code like this:

static constexpr int im_visible_in_this_file;
namespace
{
constexpr int im_visible_only_here_too;
}
 

So this variables would only be visible in this file?

[Ixrec]

Correct.  I forgot in my reply that just being in a non-header file only limits it to the "translation unit", rather than specifically that file.  Admittedly most translation units are a single hpp and cpp file, but still.

[Laurent]

Translation unit and cpp file are the same thing. What the static keyword or anynomous namespace do is to explicitly disallow this:

// A.cpp

int global_var = 0;

// B.cpp

extern int global_var;

// use "global_var"

Whoever declares a global variable or function can use it if it is defined in some other translation unit, so "static" or anonymous namespace prevents this by hidding the variable/function.