Skip to content

Commit

Permalink
Remove static data from Configurator. (#696)
Browse files Browse the repository at this point in the history
# Remove static data from `Configurator`.

Fixes #614 (partially)

---
# Change Description

Made the static data into singletonoid functions. The data in question
are:

- The list of configuration sources
- The true command line
- The pointer to the `AdditionalConfiguration` object

The command line values were combined into a struct to avoid passing
more than one return value.
The pointer to the additional configuration was set to a `nullptr`
initially, with null checks used when the pointer value is used.

---
# Test Description

The `Configurator_test` still passes.
  • Loading branch information
timspainNERSC authored Sep 5, 2024
2 parents 6618588 + 73a8359 commit 8a22ec7
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 26 deletions.
20 changes: 7 additions & 13 deletions core/src/Configurator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@

namespace Nextsim {

std::vector<std::unique_ptr<std::istream>> Configurator::sources;
int Configurator::m_argc;
char** Configurator::m_argv;
static NoAdditionalConfiguration noAddConf;
Configurator::AdditionalConfiguration* Configurator::p_addConf = &noAddConf;

boost::program_options::variables_map Configurator::parse(
const boost::program_options::options_description& opt)
{
Expand All @@ -25,9 +19,9 @@ boost::program_options::variables_map Configurator::parse(
// Parse the command file for any overrides
int use_argc;
char** use_argv;
if (m_argc && m_argv) {
use_argc = m_argc;
use_argv = m_argv;
if (commandLine().m_argc && commandLine().m_argv) {
use_argc = commandLine().m_argc;
use_argv = commandLine().m_argv;
} else {
// Use a fake command line to ensure at least one parse happens in all cases
use_argc = 1;
Expand All @@ -44,7 +38,7 @@ boost::program_options::variables_map Configurator::parse(
boost::program_options::store(parsed, vm);

// Parse the named streams for configuration
for (auto iter = sources.begin(); iter != sources.end(); ++iter) {
for (auto iter = sources().begin(); iter != sources().end(); ++iter) {
try {
boost::program_options::store(
boost::program_options::parse_config_file(**iter, opt, true), vm);
Expand All @@ -66,12 +60,12 @@ void Configurator::addSStream(const std::stringstream& sstream)
addStream(std::move(std::unique_ptr<std::istream>(new std::stringstream(sstream.str()))));
}

void Configurator::setAdditionalConfiguration(AdditionalConfiguration* pAC) { p_addConf = pAC; }
void Configurator::setAdditionalConfiguration(AdditionalConfiguration* pAC) { addConf() = pAC; }

void Configurator::getAdditionalConfiguration(const std::string& source)
{
if (p_addConf) {
addSStream(p_addConf->read(source));
if (addConf()) {
addSStream(addConf()->read(source));
}
}

Expand Down
36 changes: 23 additions & 13 deletions core/src/include/Configurator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Configurator {
*/
inline static void addStream(std::unique_ptr<std::istream> pis)
{
sources.push_back(std::move(pis));
sources().push_back(std::move(pis));
}
/*!
* @brief Adds several istream sources of configuration data.
Expand All @@ -94,7 +94,7 @@ class Configurator {
/*!
* Removes previously assigned stream data sources, both files and istreams.
*/
inline static void clearStreams() { sources.clear(); }
inline static void clearStreams() { sources().clear(); }

/*!
* Removes all data sources, both streams and command line.
Expand All @@ -117,8 +117,8 @@ class Configurator {
*/
inline static void setCommandLine(int argc, char* argv[])
{
m_argc = argc;
m_argv = argv;
commandLine().m_argc = argc;
commandLine().m_argv = argv;
}

/*!
Expand All @@ -145,10 +145,21 @@ class Configurator {
const boost::program_options::options_description& opt);

private:
static std::vector<std::unique_ptr<std::istream>> sources;
static std::vector<std::unique_ptr<std::istream>>& sources()
{
static std::vector<std::unique_ptr<std::istream>> sources;
return sources;
}

static int m_argc;
static char** m_argv;
struct CommandLine {
int m_argc;
char** m_argv;
};
static CommandLine& commandLine()
{
static CommandLine cmdln;
return cmdln;
}

public:
class AdditionalConfiguration {
Expand All @@ -171,12 +182,11 @@ class Configurator {
static void setAdditionalConfiguration(AdditionalConfiguration* pAC);

private:
static AdditionalConfiguration* p_addConf;
};

//! A default implementation of Configurator::AdditionalConfiguration
class NoAdditionalConfiguration : public Configurator::AdditionalConfiguration {
std::stringstream read(const std::string& source) override { return std::stringstream(); }
static AdditionalConfiguration*& addConf()
{
static AdditionalConfiguration* p_addConf = nullptr;
return p_addConf;
}
};

} /* namespace Nextsim */
Expand Down

0 comments on commit 8a22ec7

Please sign in to comment.