2
\$\begingroup\$

I have recently made significant progress in overcoming beginner barriers related to using Git, CMake, libraries, and successfully implementing an API in C++. Despite my achievements, I acknowledge that there is room for improvement and would greatly appreciate someone's opinion on my code. As a beginner, I am open to constructive criticism and value your expertise in identifying areas where I can enhance my implementation. Your feedback and guidance are invaluable to my growth as a programmer. Thank you in advance for your insights and recommendations!

main.cpp

#include <iostream>
#include <string>
#include <memory>
#include <boost/beast/core.hpp>
#include <boost/beast/http.hpp>
#include <boost/beast/version.hpp>
#include <boost/asio/connect.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl.hpp>
#include "json.hpp"

namespace beast = boost::beast;
namespace http = beast::http;
namespace net = boost::asio;
using tcp = net::ip::tcp;
namespace ssl = net::ssl;
using json = nlohmann::json;

// Abstract HTTP Client
class HttpClient {
public:
    virtual ~HttpClient() = default;
    virtual void sendGetRequest(const std::string& apiEndpoint) = 0;
};

// Concrete HTTP Client implementation
class RiotHttpClient : public HttpClient {
public:
    RiotHttpClient(const std::string& apiKey) : apiKey_(apiKey), ssl_context_(ssl::context::tlsv12_client) {}

    void sendGetRequest(const std::string& apiEndpoint) override {
        try {
            net::io_context io_context;
            tcp::resolver resolver(io_context);
            auto endpoints = resolver.resolve("na1.api.riotgames.com", "https");
            ssl::stream<tcp::socket> stream(io_context, ssl_context_);
            net::connect(stream.next_layer(), endpoints);
            stream.handshake(ssl::stream_base::client);

            std::string requestUrl = apiEndpoint + "?api_key=" + apiKey_;
            http::request<http::string_body> request{http::verb::get, requestUrl, 11};
            request.set(http::field::host, "na1.api.riotgames.com");
            request.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING);
            http::write(stream, request);

            beast::flat_buffer buffer;
            http::response<http::dynamic_body> response;
            http::read(stream, buffer, response);

            json responseData = json::parse(beast::buffers_to_string(response.body().data()));
            processResponse(responseData);
        } catch (const std::exception& ex) {
            std::cerr << "Error: " << ex.what() << std::endl;
        }
    }

    void processResponse(const json& response) {
        // Process the JSON response according to the specific API endpoint
        std::cout << "Processing JSON response: " << response.dump() << std::endl;
    }

private:
    std::string apiKey_;
    ssl::context ssl_context_;
};

// Abstract API
class RiotApi {
protected:
    std::shared_ptr<HttpClient> httpClient_;

public:
    virtual ~RiotApi() = default;
    virtual void makeRequest() = 0;
    void setHttpClient(const std::shared_ptr<HttpClient>& httpClient) {
        httpClient_ = httpClient;
    }
};

// Concrete API implementation
class SummonerApi : public RiotApi {
public:
    void makeRequest() override {
        if (httpClient_) {
            httpClient_->sendGetRequest("/lol/summoner/v4/summoners/by-name/");
        }
    }
};

class ChampionApi : public RiotApi {
public:
    void makeRequest() override {
        if (httpClient_) {
            httpClient_->sendGetRequest("/lol/platform/v3/champion-rotations");
        }
    }
};

int main() {
    std::string apiKey = "";
    
    // Create the API and HTTP client objects
    //std::shared_ptr<RiotApi> summonerApi = std::make_shared<SummonerApi>();
    //std::shared_ptr<HttpClient> httpClient = std::make_shared<RiotHttpClient>(apiKey);

    // Associate the HTTP client with the API
    //summonerApi->setHttpClient(httpClient);

    // Make the API request
    //summonerApi->makeRequest();

     // Create the API and HTTP client objects
    std::shared_ptr<RiotApi> championApi = std::make_shared<ChampionApi>();
    std::shared_ptr<HttpClient> httpClient = std::make_shared<RiotHttpClient>(apiKey);

    // Associate the HTTP client with the API
    championApi->setHttpClient(httpClient);

    // Make the API request
    championApi->makeRequest();

    return 0;
}

CMakeLists.txt

cmake_minimum_required(VERSION 3.26.4)
project (myproject)

find_package(Boost COMPONENTS system REQUIRED)

include_directories(${Boost_INCLUDE_DIRS})

add_executable(main main.cpp)

target_link_libraries(main LINK_PUBLIC ${Boost_LIBRARIES} ws2_32 ssl crypto)

target_sources(main PRIVATE json.hpp)

I have been actively exploring the website "Refactoring Guru - Design Patterns in C++" to gain insights and ideas on how I can effectively utilize these design patterns in my specific use case. I have a set of features that I want to develop, but they rely on external libraries such as OpenCV. This aspect makes me feel like it might be overwhelming to tackle all of these components at once. Additionally, I'm interested in learning about best practices for CMakeLists.txt files and various techniques for linking libraries. With so much to explore and absorb, I sometimes feel a bit lost even after overcoming initial challenges.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please edit your title to describe what your program does, as described in the help center. \$\endgroup\$ Commented Jul 14, 2023 at 13:14

1 Answer 1

1
\$\begingroup\$

cmake

  1. Your cmake version is too high. If you are not using all the features of the newer version, consider downgrading the required version. I think 3.12 should work here. But, I would go with 3.4 and use some of the goodies to improve my CMakeLists.txt
  2. Use modern cmake pattern, mainly use import targets that are namespaced. Check this example.

Architecture

You are over using dynamic polymorphism where a simple class or compile time polymorphism is needed.

  1. HttpClient: Do you really need a class for RiotHttpClient that inherets from HttpClient? I would say no. The only difference between a Riot games http client and an Activision http clien (for example) is which server it connects to.
  2. Similarly, you dont need runtime polymorphism in your RiotApi.In C++, you have different options for polymorphism. Dynamic polymorphism is a good idea when you dont know at compile time which object you are going to get. One example would be when a user can select a bunch of algorithms for a drop downlist. In case of a http requests, I guess the programmer knows what he has to call. For example, when writing the leaderboard code the programmer uses the champions api. In such cases, use compile time polymorphism.
  3. Where is the processResponse function. Is it same for all the requests? If not is it same for a request regardless of where it was called from? I would suggest seperate the act of sending a get request and processing it. Why not have the sendGetRequest return the response and let the caller decide what to do with the request. In fact, I would just go ahead and rename the method to get.

Style

  1. Dont have the HttpClient in main.cpp. Create a seperate h/cpp file pair for that.
  2. In constructors, take strings by value and move them into place, instead of taking them by const references.
  3. Please dont have commented code blocks in your commits.
  4. Comments should say why you are doing something not what you are doing. People who read your code know how to read code so its redudant.
  5. Prefer auto variableName = Type{arguments} syntax for creating variables. This way it is hard to have uninitialized variables.

Updated Code

#include <iostream>
#include <string>
#include <memory>
#include <boost/beast/core.hpp>
#include <boost/beast/http.hpp>
#include <boost/beast/version.hpp>
#include <boost/asio/connect.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl.hpp>
#include <nlohmann/json.hpp>

namespace beast = boost::beast;
namespace http = beast::http;
namespace net = boost::asio;
using tcp = net::ip::tcp;
namespace ssl = net::ssl;
using json = nlohmann::json;

class HttpClient {
public:

    HttpClient(std::string const & serverName, std::string apiKey)
    : apiKey_(apiKey)
    , sslContext_(ssl::context::tlsv12_client)
    , stream_(io_context_, sslContext_){

        net::io_context io_context;
        tcp::resolver resolver(io_context);
        auto const endpoints = resolver.resolve(serverName, "https");
        ssl::stream<tcp::socket> stream(io_context, sslContext_);
        net::connect(stream.next_layer(), endpoints);
        stream.handshake(ssl::stream_base::client);
    }

    template<typename GetRequest>
    typename GetRequest::Response get() {

        std::string requestUrl = std::string{GetRequest::endPoint} + "?api_key=" + apiKey_;
        http::request<http::string_body> request{http::verb::get, requestUrl, 11};
        request.set(http::field::host, "na1.api.riotgames.com");
        request.set(http::field::user_agent, BOOST_BEAST_VERSION_STRING);
        http::write(stream_, request);

        beast::flat_buffer buffer;
        http::response<http::dynamic_body> response;
        http::read(stream_, buffer, response);

        auto json = json::parse(beast::buffers_to_string(response.body().data()));

        return json.template get<typename GetRequest::Response>();
    }

private:

    net::io_context io_context_ = {};
    std::string apiKey_;
    ssl::context sslContext_;
    ssl::stream<tcp::socket> stream_;
};

namespace RiotApi {

namespace Get {
struct Summoner {
    static constexpr char endPoint[] = "/lol/summoner/v4/summoners/by-name/";
    struct Response {
        std::string name;
    };
};

void from_json(json const & j, Summoner::Response& r){
    r.name = j.at("name");
}

struct Champion {
    static constexpr char endPoint[] = "/lol/platform/v3/champion-rotations";
    struct Response {
        std::vector<int> freeChampionIds;
        std::vector<int> freeChampionIdsForNewPlayers;
        int maxNewPlayerLevel;
    };
};

void from_json(json const & j, Champion::Response& r){
    j.at("freeChampionIds").get_to(r.freeChampionIds);
    j.at("freeChampionIdsForNewPlayers").get_to(r.freeChampionIdsForNewPlayers);  
    j.at("maxNewPlayerLevel").get_to(r.maxNewPlayerLevel);
}

} // namespace Get
} // namespace RiotApi

int main() {
    auto apiKey = std::string{""};

    auto httpClient = HttpClient{"na1.api.riotgames.com", apiKey};

    auto summonerResponse = httpClient.get<RiotApi::Get::Summoner>();
    std::cout << "Summoned by" << summonerResponse.name << std::endl;

    auto championResponse = httpClient.get<RiotApi::Get::Champion>();

    return 0;
}
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.