3

I've inherited some legacy code with lots of global variables as well as lots of global functions using them.

I intend to refactor the code into something cleaner, but I see 4 ways of doing it (which I've put below in a simplified form) and I have no idea about the pros and cons of each one.

Can anyone help me get a clearer picture?

Possibility A (free functions and variables):

// Basically it's still global functions and variables
//  but now the global namespace isn't polluted
//  and the variables are only accessible from onefile
namespace functionality
{
    namespace
    {
        int x = 0;
        double y = 0.;
    }

    int f()
    {
        return doStuff(x,y);
    }

    double g()
    {
        return doOtherStuff(x,y);
    }
}

// Called like this
auto val = functionality::f();
auto otherVal = functionality::g();

Possibility B (class with static members):

class Functionality
{
private:
    static int x = 0;
    static double y = 0.;

public:
    static int f()
    {
        return doStuff(x,y);
    }

    static double g()
    {
        doOtherStuff(x,y);
    }
};

// Called like this
auto val = Functionality::f();
auto otherVal = Functionality::g();

Possibility C (singleton):

class Functionality
{
private:
    int x = 0;
    double y = 0.;
    Functionality()
    {}

public:
    static Functionality& getSingleton()
    {
        static Functionality singleton;
        return singleton;
    }

    int f()
    {
        return doStuff(x,y);
    }

    double g()
    {
        doOtherStuff(x,y);
    }
};

// Called like this
auto val = Functionality::getSingleton.f();
auto otherVal = Functionality::getSingleton.g();

Possibility D (mix of B and C):

class Functionality
{
private:
    int x = 0;
    double y = 0.;
    Functionality()
    {}

    static Functionality& getSingleton()
    {
        static Functionality singleton;
        return singleton;
    }

    int f_impl()
    {
        return doStuff(x,y);
    }

    double g_impl()
    {
        doOtherStuff(x,y);
    }


public:
    static int f()
    {
        return getSingleton.f();
    }

    static double g()
    {
        return getSingleton.g();
    }
};

// Called like this
auto val = Functionality::f();
auto otherVal = Functionality::g();

So, what are the pros and cons of each one? In particular, is there an actual difference between A, B and D?

Eternal
  • 2,648
  • 2
  • 15
  • 21
  • If you choose the singleton option, look up on how to implement it correctly: https://stackoverflow.com/questions/1008019/c-singleton-design-pattern – nm_tp Jan 30 '19 at 10:59
  • 1
    It is question asking for opinions about code we do not see. By default (and with functionality posted) I would go with A but would move the x and y and code that uses these from header to cpp file. – Öö Tiib Jan 30 '19 at 11:06
  • tbh instead of considering different variations of globals in disguise I would rather fix the cause and ban globals completely – 463035818_is_not_an_ai Jan 30 '19 at 11:08
  • Statics in cpp file are not global. – Öö Tiib Jan 30 '19 at 11:11
  • @ÖöTiib I was not asking for opinion, or even which one to choose. I was asking about the pros and cons of each one so that I can make an informed decision – Eternal Jan 30 '19 at 11:17
  • Then the question is too broad since default design is to use ordinary classes. If you happen to need only one instance in program then so be it, but that may change. And that choice is missing, despite it wins in most comparisons. – Öö Tiib Jan 30 '19 at 11:19

0 Answers0