Reading and storing values from JSON

I have a class ConfigFile that reads some values from a JSON using Boost and stores them in variables. Everything is done in the constructor:

ConfigFile::ConfigFile(const std::string& configFileNamePathIn)
{
    PTree tmpPT;
    std::string path;

    /// reading aws config file ///
    json::read_json(configFileNamePathIn, tmpPT);

    // read a string value
    path = "path1";
    m_param1 = tmpPT.get(path, "");
    checkEmptyness(m_param1, "param1", path);

    // read a float value
    path = "path2";
    m_param2 = tmpPT.get(path, -1);
    checkNegativity(m_param2, "param2", path);

    // ... much more lines like these two also for int and double
}

Because the constructor is long, I thought of creating 4 functions that return string, int, float or double and that receive as input parameters the string path and the PTree:

std::string readStringValue(const std::string& pathIn, const PTree& tmpPT)
{
  path = "path1";
  return tmpPT.get(path, "");
}

In the constructor, I just call the function and verify the result:

// ...
m_param1 = readStringValue("path1", tmpPT);
checkEmptyness(m_param1, "param1", path);
// ...

Do you think this is a good improvement, or shall I keep the initial constructor?

I think your proposed change is an improvement for two reasons. First, it breaks up one long function into multiple smaller ones which is better for human comprehension. Second, it also allows for reuse of the functions which may also improve efficiency, reduce code size, etc.

There are, however, a couple of things that might be considered for further improvements.
Exceptions

With a long and complex constructor like this, and especially one which involves I/O, things can go wrong. Carefully considering what might go wrong and using C++ exceptions has the possibility of making your code more durable.
Named variables vs. object collection

Although you're using a constructor, the code looks very procedural. Instead of populating a number of named variables, consider each of those variables itself to be an object. You could store the name (e.g. "path1"), type, and any required value checks encapsulated within individual objects, making the code easier to read, understand and maintain. I'd be inclined to use a static data structure to store and associate those things rather than burying all of those details in code. A std::map might be appropriate.
Example code

To flesh out that latter suggestion a bit, consider that you might have a string, and an unsigned int as configuration items. To refer to them, you could use just the variable names, or you could create a map:

std::map > config;

Here ConfigItem would be a base class for all configuration items. You would specialize that class for each different kind of configuration item, such as a plain string, a string that represents a filename, a floating point number, etc. Using the items would then use the std::string as a key to look up and retrieve the corresponding object. The std::unique_ptr is just to make sure that the contained objects are deleted when config is deleted.

Here's an example base class:

class ConfigItem
{
public:
    friend std::ostream &operator<<(std::ostream &out, const ConfigItem &ci) {
        return ci.printTo(out);
    }
    virtual operator int() const { return 0; }
    virtual operator std::string () const { return std::string(); }
    virtual ~ConfigItem() {};
protected:
    virtual std::ostream &printTo(std::ostream &out) const {
        return out;
    }
};

Note that the member functions are all virtual. Here's a specialization for a string:

class ConfigString : public ConfigItem
{
public:
    ConfigString(std::string value) : mValue(value) {}
    std::ostream &printTo(std::ostream &out) const {
        return out << mValue;
    }
    operator std::string () const { return std::string(mValue); }
private:
    std::string mValue;
};

Here's a specialization for an int:

class ConfigInt : public ConfigItem
{
public:
    ConfigInt(int value = 0) : mValue(value) {}
    std::ostream &printTo(std::ostream &out) const {
        return out << mValue;
    }
    operator int() const { return mValue; }
private:
    int mValue;
};

If you look at those two classes carefully, I'm sure it will be pretty easy to spot that they could have been written as a template instead.

Here's a test driver:

#include 
#include 
#include 
#include 
#include 
#include "ConfigItem.h"
    
int main()
{
    std::map > config;
    config.emplace("inputFilename", 
                   make_unique("/var/log/messages"));
    config.emplace("maxMessages", 
                   make_unique(200));

    for (const auto &it : config)
        std::cout << it.first << " = " << *it.second << '\n';

    int count = *config["maxMessages"];
    std::cout << "count = " << count << '\n';

    std::string filename = *config["inputFilename"];
    std::cout << "file name = \"" << filename << "\"\n";
}

Also note that if you're only using C++11 and don't have access to a C++14 compiler (one that contains a definition for make_unique() yet), you can use this:

template
std::unique_ptr make_unique( Args&& ...args )
{
    return std::unique_ptr( new T( std::forward(args)... ) );
}

Sorry that's so long, but I wanted to show a clear, complete and correct example.

StackExchange Code Review Q#63238, answer score: 3