Is it fine to have sf::Font and sf::Text as global

[paupav]

So, i have

namespace menu
{
    void loadFiles();
    void menu(sf::RenderWindow &window);
    void main(sf::RenderWindow &window);
    void options();

}

and I have sat up Text and Font inside the menu::loadFiles, and I'm drawing it inside the menu::main(). I've made them global so that I can use it inside both functions and I wanna know is it fine to have them global or should I find some other way?

[eXpl0it3r]

It's bad design and can lead to crashes. In most cases using globals to solve a problem is a bad decision.

Manage the font with for example a ResourceHolder and create the text object where you need it.

[Mörkö]

It's bad design and can lead to crashes. In most cases using globals to solve a problem is a bad decision.

Can you show some examples of where people experienced crashes explicitly due to using globals (threads on the forum etc)? Alternatively a small code example reproducing a bug caused by a global variable.

[Jesper Juhl]

Just search the forum. There are plenty of threads.
Some of the SFML classes have interdependencies that go wrong when the classes get destructed in the wrong order (which you lose control over with globals in different translation units).
And even if you don't get crashes, designs involving globals (including singletons, which are just globals by a different name IMHO) are usually bad anyway for loads of other reasons (tight coupling, hard to refactor, multiple globals depending on individual construction/destruction order, globals being constructed before main() and destroyed after it ends and not being able to deal with that, functions having hard to predict side effects due to modifying globals, objects living longer than they need to (since they are global) and needlessly consuming resources. Etc etc etc).
Just try to avoid globals - please ;-)

[StormWingDelta]

From what my teacher said the only globals that are safe are primitives like, int, double, bool, string, etc.  Like others have said here when it comes to objects the order they are destructed in as globals is undefined and can cause crashes because of it.

So:

namespace menu
{
    void loadFiles();
    void menu(sf::RenderWindow &window);
    void main(sf::RenderWindow &window);
    void options();

}
 

Should/Could be:

class menu
{
public:
    void loadFiles();
    void menu(sf::RenderWindow &window);
    void main(sf::RenderWindow &window);
    void options();

}
 

[Ixrec]

From what my teacher said the only globals that are safe are primitives like, int, double, bool, string, etc.

Gonna have to disagree there.  First, string is not a primitive.  Second, a global int has all the problems one would normally associate with a global.  In particular, everything can change it, so you can quickly lose control of how it's being changed and end up with brittle code in a sufficiently large program.  Third, if you had a global struct, splitting that into its constituent global primitives wouldn't make the globalness any less problematic (if anything that would be slightly worse).

The only globals I would call "safe" are global constants.

[StormWingDelta]

The only globals I'd ever use anyways are ReadOnly/constants, Kinda forgot to finish that line.   Basically the only good globals are either none or readonly ones that aren't objects or messing with objects.

[Neil]

I'm not sure what you gain by just switching the namespace to a class.

[Ixrec]

I think his post was missing a lot of things he intended to put in there.  I assume those code samples were meant to look more like this:

namespace menu
{
    sf::Font evilGlobalFont;
    sf::Text evilGlobalText;

    void loadFiles();
    void menu(sf::RenderWindow &window);
    void main(sf::RenderWindow &window);
    void options();

}

class menu
{
public:
    void loadFiles();
    void menu(sf::RenderWindow &window);
    void main(sf::RenderWindow &window);
    void options();
private:
    sf::Font goodEncapsulatedFont;
    sf::Text goodEncapsulatedText;
}

[Jesper Juhl]

I fail to see how that makes things radically better...

[Gambit]

Maybe make "menu" a full public singleton?

[Neil]

What if more than one thing in your program uses the font, won't that way load it into memory multiple times?

This is why I suggest a global repository of non-modifiable resources sourced from disk. Properly managed in such a way that nothing is loaded before main function is called, and everything gets unloaded before the exit of main.. or pass it as a pointer to all your stuff.

I wouldn't make the menu object itself a global variable, which a singleton essentially is. From what I've seen, it doesn't need to store any state at all.

[Gambit]

Global objects suck. That is a really well known fact about C++ due to them having weird destructions and so on. Singletons arent global variables. If you use it write, you should have any problems. That being said, its a cheap workaround for the real issue here. paupav should implement a proper state system into the application and use a context-type system for handling data (Like resources) between states.

[Neil]

A singleton is a nicely packaged global variable, which can solve some problems such as creation order. However as far as other design aspects are concerned, they suffer many of the same problems as global variables. So I consider them practically the same. After all, the singleton has a "static" in there somewhere, right?

[Gambit]

Like I said though, singletons arent variables. You cant get the address of a singleton, you can only call the static members of it. Singletons can fix issues properly (That is, not using them as an ugly hack) but thats usually because they dont store states for anything, the state is created in the function calls and generally returned.

In the case of a resource manager, a singleton is a pretty bad idea, especially if you want multiple caches. That being said, it all comes down to personal preference and if you want to use a singleton or global variables for states or whatever thats all on the programmer.