Is this code with global variables good practice? [closed]

Question

I am currently writing a game in C++ and have some variables that I want to be accessible from everywhere. For the examples, I will have the shared variable as an object of the class Input.

At the moment I am implementing it like this:

input.h:

#ifndef INPUT_H
#define INPUT_H
extern Input input;
#endif

init.cpp:

Input input;
int main(//args){
    input.doStuff();
}

Other files that use the object.cpp:

#include "input.h"

void method(){
    input.doStuff();
}

I've got this nagging doubt that I shouldn't be doing things in this way. Is this doubt correct? What should I be doing and why?

Answer 1

Your doubt is correct, don't use global variables.

(Note: For this post, I mean variable in its true meaning, i.e. objects that may change.)

I can't say for sure which technique is the correct one without seeing your code, but normally you want to pass the variable as a parameter to whichever entity needs it:

void foo(Input input)
{
    input.doSomething();
}

int main()
{
    Input input;
    input.doSomething();

    foo(input);
}

Or, restrict it to a class as a member variable.

---

Update: Class examples

You can do it one or both of these ways:

class Person {
    std::string name_;
public:
    explicit Person(std::string const& name) : name_(name) {}
    void set_name(std::string const& name) { name_ = name; }
}

void foo()
{
    Person p("Bob Loblaw");

    p.set_name("Slartibartfast");
}

---

There are two key downsides to using global variables:

1. They can be changed everywhere.

You generally want to enlist the compiler's aid in finding bugs and errors for you. If you allow yourself to modify a variable from all over your code, it is very hard for your compiler and yourself to track down an error. Consider the following scenario:

Input input; // global

void foo()
{
    Input inputt;
    input = someThingWeird;
}

This code will compile. If you're really unlucky, it will even work for a while. If you don't have a global, the code won't compile and the bug will be easy to find.

2. They increase coupling (and reduce testability).

By reading from a global variable, your entity (i.e. function, class, whatever) depends on that global variable. If some day you want to use something else, you'll have to do a lot of refactoring. What's more important, if you change an aspect of the global variable, you will need to update all parts of the code that uses it. This is a great opportunity for suble bugs to infiltrate your code and make your life miserable. A typical example is that you modify one part of your code, and a completely different part will break.

Using globals also makes your code hard to unit test, because it depends on the global variable.

Answer 2

@Lstor is absolutely correct. However, I'll like to add some things:

• If you're using constants (indicated by the const keyword), they're okay as globals. This is because they cannot be changed elsewhere in the code, unlike global variables).

• I'd recommend classes, especially when writing a game. When used properly, they will allow you to hide your data from entities that should not have access to them. However, it's best not to use them just to avoid globals, especially when they can still be passed around. They should serve some kind of role, even if your program doesn't revolve around classes. In your case, you'll most likely have a Game class and possibly additional classes. More general info about this here.

Answer 3

Using global accessible variables is like handing your wallet to other and hoping they will only take the amount of money you owe them. You don't do this either.

I was confronted with a similar problem while developing a simulation in XNA. There are some properties that have to be public known, but I want to control access to them. So I implemented a generic ItemChangedService. Here a simplified version of it.

public abstract class ItemChangedService<T>{
    //private backing field that holds my value
    private readonly T _current; 

    //Property Current -> simple a getter and setter
    public virtual T Current {
       get {return _current;}
       set{ 
           if(value == current) return; //no change, so we can return

           _current = value;
           //Invoke Changed event.
           if(Changed == null) return;

           Changed(this, EventArgs.Empty);
       }
    }
    public event EventHandler Changed;
}

This solution has two clear advantages over yours:

First of all your variable is only changeable by getter setter you can redefine in subclasses. The second is that components can register for changes. Further more the behavior while getting/settings is extendable. So I created more *ChangedServices that are able to:

• Define what happen it the variable is set to the value it was before. (Define a RefreshBehaviour)
• Validate the input "value". (And what should be done? Exeption, LeaveExitingValue,... )

Never the less you can make typedefs in order to give this ItemChangedService meaningful names like CurrentFPSService or CamereaService and so on.

Reaction on comments: Sorry for the C# Code. Unfortunately I can only read C++, but write is a little bit difficult for me. So I try to make an example in pseucode without generics, events and properties:

template <typename T>
class ItemChangedService {
    T myVariable;

public:
    virtual T getValue() const {
        return myVariable;
    }

    virtual void setValue(T const& value) {
        if (value == myValue) return; // No change, so we can skip

        myValue = value;
    }
}

Because of the fact that the getter and setter are virtual, you can adjust these behavior in subclasses.

Answer 4

I'm going to go against the current here and suggest that "no global variables ever" is unnecessarily dogmatic, and maintaining a small handful of global singleton objects can vastly simplify your code.

For a game in particular, something like an input manager or a graphics system makes sense as a global singleton.