Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should I use global variables?

I have been reading about global variables and how bad they are but I am stuck in one place due to that. I am going to be very specific about if I should use global variables in this scenario.

I am working on a game engine. And my engine consists of lots of managers. Managers do certain tasks - they store resources, load them, update them etc.

I have made all my managers a singleton because so many classes and functions needs access to them. I was thinking of removing the singleton but I don't know how i can not have it and get access to these managers.

Here is an example of what I am trying to tell (im bad at english, sorry):

Singleton.h

template<class T> class Singleton {
private:
    Singleton( const Singleton& );
    const Singleton& operator=( const Singleton& );

protected:
    Singleton() { instance = static_cast<T*>(this); }
    virtual ~Singleton() {}

protected:
    static T * instance;

public:
    static T &Instance() {
        return *instance;
    }

};

ScriptManager.h

class ScriptManager : public Singleton<ScriptManager> {
public:
    virtual void runLine(const String &line)=0;
    virtual void runFile(const String &file)=0;
};

PythonScriptManager.cpp

class PythonScriptManager : public ScriptManager {
public:
    PythonScriptManager() { Py_Initialize(); }
    ~PythonScriptManager() { Py_Finalize(); }

    void runFile(const String &file) {
        FILE * fp = fopen(file.c_str(), "r");
        PyRun_SimpleFile(fp, file.c_str());
        fclose(fp);
        fp=0;
    }

    void runLine(const String &line) {
        PyRun_SimpleString(line.c_str());   
    }

};

Entity ScriptComponent

#include <CoreIncludes.h>
#include <ScriptManager.h>
#include <ScriptComponent.h>

void update() {

    ScriptManager::Instance().runFile("test_script.script");
    //i know its not a good idea to open the stream on every frame but thats not the main concern right now.
}

Application

int main(int argc, const char * argv) {
    Application * app = new Application(argc, argv);
    ScriptManager * script_manager = new PythonScriptManager;
    //all other managers

    return app->run();
}

As you see I am not even including the files above in my ScriptComponent.cpp file which wins me some compilation time. How can I get that kind of a result without globals which will make it easy to integrate as this one. The singleton is not thread safe but adding threads won't take a long time.

I hope I could explain the problem.

Thanks in advance,
Gasim Gasimzada

like image 835
Gasim Avatar asked Jul 16 '11 11:07

Gasim


2 Answers

I won't say you should never use globals, but:

  • Never use singletons. Here is why. They're horrible, and they're much worse than plain old globals.
  • "Manager" classes are bad. What do they "manage"? How do they "manage" it? "Manager" classes need to be broken up into something that you can describe. Once you've figured out what it means to "manage" an object, you can define one or more object with better-defined responsibilities.
  • When you use globals, don't make them mutable. A write-only global can be acceptable (consider a logger. You write to it, but its state never ever affects the application), and read-only globals can be ok too (consider various constants that are never changed, but which you frequently need to read from). Where globals become harmful is when they have mutable state: when you both read to, and write from, them.

And finally, the very very simple alternative: Just pass dependencies as arguments. If an object needs something in order to function, pass it that "something" in its constructor. If a function needs something in order to operate, pass it that "something" as an argument.

This might sound like a lot of work, but it isn't. When your design is cluttered with globals and singletons, you get a big huge spaghetti architecture where everything depends on everything else. Because the dependencies are not explicitly visible, you get sloppy, and rather than thinking about what the best way to connect two components is, you just make them communicate through one or more globals. Once you have to think about which dependencies to explicitly pass around, most of them turn out to be unnecessary, and your design becomes much cleaner, more readable and maintainable, and much much easier to reason about. And your number of dependencies will drop dramatically, so that you only actually need to pass an extra argument or two to a small number of objects or functions.

like image 104
jalf Avatar answered Oct 22 '22 15:10

jalf


How about removing the ScriptManager base class and use static methods in the specialization classes? It looks like there is no state involved with any ScriptManagers, and no real heritage other than the purely virtual functions.

I could not figure out from your code samples if you actually use polymorphism here. If not, static member functions look OK to me.

like image 26
Gabriel Avatar answered Oct 22 '22 17:10

Gabriel