tags:

views:

223

answers:

10

Typically, the way I'd define a true global constant (lets say, pi) would be to place an extern const in a header file, and define the constant in a .cpp file:

constants.h:

extern const pi;

constants.cpp:

#include "constants.h"
#include <cmath>
const pi=std::acos(-1.0);

This works great for true constants such as pi. However, I am looking for a best practice when it comes to defining a "constant" in that it will remain constant from program run to program run, but may change, depending on an input file. An example of this would be the gravitational constant, which is dependent on the units used. g is defined in the input file, and I would like it to be a global value that any object can use. I've always heard it is bad practice to have non-constant globals, so currently I have g stored in a system object, which is then passed on to all of the objects it generates. However this seems a bit clunky and hard to maintain as the number of objects grow.

Thoughts?

+1  A: 

A legitimate use of singletons!

A singleton class constants() with a method to set the units?

Martin Beckett
legitimate? How so? Because someone wants a global?
jalf
What would be the benefit of a singleton over globals?
Mark Ransom
@Mark: Well, it's a design pattern, duh! What more do you need? ;)
jalf
@Mark Ransom - if you have 'almost' constants - the OP wanted to switch units and get back the value of g (9.8m/s^s or 32 ft/s^2). Probably cleaner to have a singleton class and a units() setter, than global_imperial_g and global_metric_g and a lot of if statements in your calculation
Martin Beckett
I don't like singletons. They make things like testing much messier, where you need to test on different configurations (like in this scenario) or need a pristine singleton for each new test. It's doable of course, but in most cases I still they're just plain bad. For example, they make it impossible to parallelize the test suite.
Staffan
+2  A: 

It all depends on your application size. If you are truly absolutely sure that a particular constant will have a single value shared by all threads and branches in your code for a single run, and that is unlikely to change in the future, then a global variable matches the intended semantics most closely, so it's best to just use that. It's also something that's trivial to refactor later on if needed, especially if you use distinctive prefixes for globals (such as g_) so that they never clash with locals - which is a good idea in general.

In general, I prefer to stick to YAGNI, and don't try to blindly placate various coding style guides. Instead, I first look if their rationale applies to a particular case (if a coding style guide doesn't have a rationale, it is a bad one), and if it clearly doesn't, then there is no reason to apply that guide to that case.

Pavel Minaev
Why not just put it in a separate namespace instead of the `g_` prefix?
jalf
@jalf, which is easier to search for "g_something" or namespacename::something and/or using namespace namespacename; \* lines of code in a different file *\ something? Namespaces are great for disambiguation and preventing collisions, but they're not very searchable.
David Gladfelter
Up until now, I was the only one writing the code for this simulator. It is being written in such a way that others can start extending it, and want to avoid some module writer changing the gravitational constant (in otherwords- I want my cake and want to eat it too)
MarkD
@David: But @Pavel used name clases as the reason for that prefix. He didn't talk about searchability. And if you want searchability, on't do `using namespace`. Search for `globals::`, and you catch everything in the `globals` namespace. Pretty easy to search for, I'd say. I've never had any problems searching for instances of `std::vector` either.
jalf
@jalf: no, the point was indeed searchability. I referred to name clashes in a sense that you have a local shadowing a global - not really a clash in normal sense, but a "clash" when you're trying to do a find-and-replace. There's no problem using a namespace like that, either, so long as you can ensure that everyone on the team will always fully qualify all names in it, without ever writing `using` or `using namespace`. With `g_`, this is effectively enforced on all clients.
Pavel Minaev
+1  A: 

You can use a variant of your latter approach, make a "GlobalState" class that holds all those variables and pass that around to all objects:

struct GlobalState {
  float get_x() const;

  float get_y() const;

  ...
};

struct MyClass {
  MyClass(GlobalState &s)
  {
    // get data from s here
    ... = s.get_x();
  }
};

It avoids globals, if you don't like them, and it grows gracefully as more variables are needed.

Staffan
A: 

Why is your current solution going to be hard to maintain? You can split the object up into multiple classes as it grows (one object for simulation parameters such as your gravitational constant, one object for general configuration, and so on)

jalf
It may not be hard to maintain in the end, but *seems* to be increasingly cumbersome as this code grows. The issue I am looking at now, is that a given object that "belongs" to the system object may have a few levels of objects under it, that also need the gravitational constant. I just ran into an issue where one of these sub-sub objects didn't have access to the gravitational constant, and had to go back up through my hierarchy and pass g down.
MarkD
But how many times has it saved you from adding needless dependencies? How many times would you have blindly just accessed these values if they'd been truly global, where your current solution forced you to stop and ask whether the current object actually *should* have a dependency on those values, resulting in a cleaner, and more loosely coupled solution?
jalf
A: 

It's bad to have globals which change value during the lifetime of the run.

A value that is set once upon startup (and remains "constant" thereafter) is a perfectly acceptable use for a global.

James Curran
A: 

My typical idiom for programs with configurable items is to create a singleton class named "configuration". Inside configuration go things that might be read from parsed configuration files, the registry, environment variables, etc.

Generally I'm against making get() methods, but this is my major exception. You can't typically make your configuration items consts if they have to be read from somewhere at startup, but you can make them private and use const get() methods to make the client view of them const.

T.E.D.
Instead of using `get` methods, why not just make the whole configuration object `const` except for the place where it needs to be initialized?
Mark Ransom
Most likely because I'm ignorant of that technique. I know you can initialize `const` members in an initializer, but that only works for fairly simple things. ...Are you perhaps suggesting making the (hidden) object itself variable, but having the `static` methods that return references to it return `const` views of it? I'll have to try that out next time it comes up.
T.E.D.
A: 

This actually brings to mind the C++ Template Metaprogramming book by Abrahams & Gurtovoy - Is there a better way to manage your data so that you don't get poor conversions from yards to meters or from volume to length, and maybe that class knows about gravity being a form acceleration.

Also you already have a nice example here, pi = the result of some function...

const pi=std::acos(-1.0);

So why not make gravity the result of some function, which just happens to read that from file?

const gravity=configGravity();

configGravity() {
 // open some file
 // read the data
 // return result
}

The problem is that because the global is managed prior to main being called you cannot provide input into the function - what config file, what if the file is missing or doesn't have g in it.

So if you want error handling you need to go for a later initialization, singletons fit that better.

Greg Domjan
A: 

Let's spell out some specs. So, you want: (1) the file holding the global info (gravity, etc.) to outlive your runs of the executable using them; (2) the global info to be visible in all your units (source files); (3) your program to not be allowed to change the global info, once read from the file;

Well,

(1) Suggests a wrapper around the global info whose constructor takes an ifstream or file name string reference (hence, the file must exist before the constructor is called and it will still be there after the destructor is invoked);

(2) Suggests a global variable of the wrapper. You may, additionally, make sure that that is the only instance of this wrapper, in which case you need to make it a singleton as was suggested. Then again, you may not need this (you may be okay with having multiple copies of the same info, as long as it is read-only info!).

(3) Suggests a const getter from the wrapper. So, a sample may look like this:

#include <iostream>
#include <string>
#include <fstream>
#include <cstdlib>//for EXIT_FAILURE

using namespace std;

class GlobalsFromFiles
{
public:
    GlobalsFromFiles(const string& file_name)
    {
        //...process file:
        std::ifstream ginfo_file(file_name.c_str());
        if( !ginfo_file )
        {
            //throw SomeException(some_message);//not recommended to throw from constructors 
            //(definitely *NOT* from destructors)
            //but you can... the problem would be: where do you place the catcher? 
            //so better just display an error message and exit
            cerr<<"Uh-oh...file "<<file_name<<" not found"<<endl;
            exit(EXIT_FAILURE);
        }

        //...read data...
        ginfo_file>>gravity_;
        //...
    }

    double g_(void) const
    {
        return gravity_;
    }
private:
    double gravity_;
};

GlobalsFromFiles Gs("globals.dat");

int main(void)
{
    cout<<Gs.g_()<<endl;
    return 0;
}
blue scorpion
A: 

I can understand the predicament you're in, but I am afraid that you are unfortunately not doing this right.

The units should not affect the program, if you try to handle multiple different units in the heart of your program, you're going to get hurt badly.

Conceptually, you should do something like this:

       Parse Input
            |
 Convert into SI metric
            |
       Run Program
            |
Convert into original metric
            |
      Produce Output

This ensure that your program is nicely isolated from the various metrics that exist. Thus if one day you somehow add support to the French metric system of the 16th century, you'll just add to configure the Convert steps (Adapters) correctly, and perhaps a bit of the input/output (to recognize them and print them correctly), but the heart of the program, ie the computation unit, would remain unaffected by the new functionality.

Now, if you are to use a constant that is not so constant (for example the acceleration of gravity on earth which depends on the latitude, longitude and altitude), then you can simply pass it as arguments, grouped with the other constants.

class Constants
{
public:
  Constants(double g, ....);

  double g() const;

  /// ...
private:
  double mG;

  /// ...
};

This could be made a Singleton, but that goes against the (controversed) Dependency Injection idiom. Personally I stray away from Singleton as much as I can, I usually use some Context class that I pass in each method, makes it much easier to test the methods independently from one another.

Matthieu M.
A: 

Globals aren't evil

Had to get that off my chest first :)

I'd stick the constants into a struct, and make a global instance of that:

struct Constants
{
   double g;
   // ...
};

extern Constants C = { ...  };

double Grav(double m1, double m2, double r) { return C.g * m1 * m2 / (r*r); }

(Short names are ok, too, all scientists and engineers do that.....)

I've used the fact that local variables (i.e. members, parameters, function-locals, ..) take precedence over the global in a few cases as "apects for the poor":

You could easily change the method to

double Grav(double m1, double m2, double r, Constants const & C = ::C) 
{ return C.g * m1 * m2 / (r*r); }  // same code! 

You could create an

struct AlternateUniverse
{
    Constants C; 

    AlternateUniverse()
    {
       PostulateWildly(C);   // initialize C to better values
       double Grav(double m1, double m2, double r) { /* same code! */  }
    }
}

The idea is to write code with least overhead in the default case, and preserving the implementation even if the universal constants should change.


Call Scope vs. Source Scope

Alternatively, if you/your devs are more into procedural rather thsn OO style, you could use call scope instead of source scope, with a global stack of values, roughly:

std::deque<Constants> g_constants;

void InAnAlternateUniverse()
{
   PostulateWildly(C);    // 
   g_constants.push_front(C);
   CalculateCoreTemp();
   g_constants.pop_front();
} 


void CalculateCoreTemp()
{
  Constants const & C= g_constants.front();
  // ...
}

Everything in the call tree gets to use the "most current" constants. OYu can call the same tree of coutines - no matter how deeply nested - with an alternate set of constants. Of course it should be encapsulated better, made exception safe, and for multithreading you need thread local storage (so each thread gets it's own "stack")


Calculation vs. User Interface

We approach your original problem differently: All internal representation, all persistent data uses SI base units. Conversion takes place at input and output (e.g. even though the typical size is millimeter, it's always stored as meter).

I can't really compare, but worksd very well for us.


Dimensional Analysis

Other replies have at least hinted at Dimensional Analysis, such as the respective Boost Library. It can enforce dimensional correctness, and can automate the input / output conversions.

peterchen