duanegoodner / xiangqigame

C++ AI engine for Chinese Chess, wrapped in Python manager and CLI.
MIT License
1 stars 0 forks source link

JsonUtilityInterface and implemention too tightly coupled to imported/exported objects #87

Open duanegoodner opened 3 hours ago

duanegoodner commented 3 hours ago

Problem Statement JsonUtilityInterface and its implementations currently require implementing methods for specific objects to be imported/exported. If/when we want to import additional objects and/or change from Nlohmann Json to some other json library, would have to write object-specific methods.

Current arrangement can also result in circular dependencies (e.g. between json_utility_nlohmann.hpp and piece_points_bpo.hpp.

Proposed Solution A better approach would be to have an interface class that requires implementation of more fundamental json operations, and then in our data transfer object classes, we could just use these fundamental interface methods. The interface could look something like this:

//! @file json_utility_interface.hpp

class JsonUtilityInterface {
public:
    virtual ~JsonUtilityInterface() = default;
    virtual void Load(const std::string& path) = 0;
    virtual void Save(const std::string& path) const = 0;
    virtual void set(const std::string& key, const std::string& value) = 0;
    virtual std::string get(const std::string& key) const = 0;
    virtual std::string ToString() const = 0;
};

And our implementation that utilizing the Nlohmann json library could look like this:

//! @file nlohmann_json_utility.hpp
#include <nlohmann/json.hpp>
#include <fstream>
#include <json_utility_interface.hpp>

class NlohmannJsonUtility : public IJson {
    nlohmann::json json;

public:
    void Load(const std::string& path) override {
        std::ifstream jsonFile(path);
        if (!jsonFile.is_open()) {
            throw std::runtime_error("Unable to open file: " + path);
        }
        jsonFile >> json;
        jsonFile.close();
    }

    void Save(const std::string& path) const override {
        std::ofstream jsonFile(path);
        if (!jsonFile.is_open()) {
            throw std::runtime_error("Unable to open file: " + path);
        }
        jsonFile << json.dump(4);
        jsonFile.close();
    }

    void set(const std::string& key, const std::string& value) override {
        json[key] = value;
    }

    std::string get(const std::string& key) const override {
        try {
            return json.at(key).get<std::string>();
        } catch (nlohmann::json::exception& e) {
            throw std::runtime_error("JSON key not found: " + key);
        }
    }

    std::string ToString() const override {
        return json.dump(4);
    }
};
duanegoodner commented 2 hours ago

Note that proposed solution also involves changing from CRTP interface to traditional abstract base class interface. This is consistent with proposed solution for Issue [#88 ].