Parsing JSON using jsoncpp
// Should header files always be included in root.h as opposed to 


You should include all header files that are required (no more than are required).

In your case you use the following types in the header file.

std::ifstream
std::string
Json::Value


So you should include the appropriate header file for these types.

#include &lt;fstream&gt;
#include &lt;string&gt;
#include "json/json.h"


The only curve ball is that if you only use a type reference then you can technically get away with a forward declaration (rather than including the header file). So you could use a forward declaration of std::string, unfortunately its not your class and you don't actually know how to forward declare it because that is not defined in the standard, so you have to include the header file.


  // Seems like I do not need to include this string container, why?


It may be included indirectly via &lt;fstream&gt; or "json/json.h". BUT you should still include the &lt;string&gt; header file because these dependencies may not always hold (on different platforms or different versions of the compiler it may including things differently). So do not assume because it worked on this platform it will always work. Think worst case and explicitly include &lt;string&gt;

The include guard seems a bit too generic:

ROOT_H


Seems like somebody else may use this. You need to make sure your guards are unique. I always use guards that include the namespace

// I own the domain name thorsanvil.com
// and I put all my classes in the namespace ThorsAnvil
// So my include guards go like this.
#define THORSANVIL_&lt;Optional Nested Namespace&gt;_ROOT_H



  // Is std::ifstream::binary ok to put it in here like this ^?
  // It's working, but would that be good practice?


Sure. That's totally fine.

You just have to understand what it mean and the difference between binary and text mode (the default). In Text mode: => "Platform Specific" "End of Line Sequence" is converted into '\n' when reading from a file. In Binary mode: no conversion is performed and you get the raw bytes.

So in the constructor:

Root::Root(const std::string &amp; filename) : m_json(filename, std::ifstream::binary)

{
    m_json &gt;&gt; m_root;
    m_json.close();  // Do I need .close() here?

                     // Yes probably.
                     // Closing it releases the related OS resources.
                     // If you don't the resource will be held until
                     // the whole object is destroyed.

}


I don't see the need for the m_json object to be part of the object. It is only every going to be used inthe constructor. Once the data is loaded it will never be used again. So just declare it as an automatic variable that is local to the constructor.

Root::Root(const std::string &amp; filename)
{
    std::ifstream  m_json(filename, std::ifstream::binary)

    m_json &gt;&gt; m_root;    
}
// Now we don't need `close()` because the object goes out of scope
// and the `std::ifstream` destructor calls close for us.



  // Is it ok to assign member to a member like so,
  // as opposed to just a variable?


It is. But why. It seems much more logical to use a local variable.

Json::Value query = rate.m_root["query"]["items"];


Prefer '\n' over std::endl

The difference is that std::endl forces the buffer to flush. The buffer will flush automatically when it needs to. Forcing it to flush is only going to make your code inefficient.


  // How can I instantiate my object like the line below?
  // Root rate("test.json", ["query"]["results"]["rate"]);


You can define the operator[]. You can make it take a string as an argument.

class Root
{
    public:
        JsonValue operator[](std::string const&amp; index)
        {
            return m_root[index];
        }
        // Other stuff in your class.
};


Now it can be used:

Root    json("file.name");
Json::Value = json["query"]["results"]["rate"];