How to change unwanted behavior in a method

Question

We have several classes:

struct HttpRequest {
    std::string url;
};

struct HttpResponse {
    int          status_code;
    std::string  status_text;
    std::string  content;
};

struct HttpClient {
    void execute(HttpRequest const& req, HttpResponse& resp);
};

size_t
appender(void *contents, size_t size, size_t nmemb, void *userp)
{
    auto& ct = * reinterpret_cast<std::string*>(userp);
    size_t bytes = size * nmemb;

    ct.append(reinterpret_cast<char*>(contents), bytes);
    return bytes;
}

void HttpClient::execute(HttpRequest const& req, HttpResponse& resp)
{
    auto curl = curl_easy_init();

    curl_easy_setopt(curl, CURLOPT_URL, req.url.c_str());
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, appender);
    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &resp.content);
    // header callback etc.

    CURLcode code = curl_easy_perform(curl);

    curl_easy_cleanup(curl);
    if (code != CURLE_OK) {
        throw std::runtime_error("curl fail");
    }

    return; // enjoy your HttpResponse
}

We used them as follows:

bool fatal(std::runtime_error const& rte) {
    return rte.what()[0] == 'F'; // if it starts with F, it fatal
    // not the actual implementation :)
}

const int retries = 7;

int do_something(std::string const& url)
{
    HttpClient client;
    HttpRequest req{ url };

    for (int r = 0; r < retries; ++r) try {
        HttpResponse resp;

        client.execute(req, resp);
        //
        if (resp.status_code / 100 == 2) { // a 2xx success code
            return std::stoi(resp.content);
        }
        else { // maybe a 4xx client error or 5xx server error
            throw std::runtime_error("HTTP error");
        }
    }
    catch (std::runtime_error const& e) {
        if (fatal(e))
            throw;
        else
            continue; // retry
    }

    // ran out of retries
    throw std::runtime_error("out of retries");
}

and everything was fine. However, someone applied a different template:

int worker(HttpRequest const& req, HttpResponse& resp)
{
    HttpClient client;
    for (int r = 0; r < retries; ++r) try {
        client.execute(req, resp);
        //
        if (resp.status_code / 100 == 2) { // a 2xx success code
            return std::stoi(resp.content);
        }
        else { // maybe a 4xx client error or 5xx server error
            throw std::runtime_error("HTTP error");
        }
    }
    catch (std::runtime_error const& e) {
        if (fatal(e))
            throw;
        else
            continue; // retry
    }

    // ran out of retries
    throw std::runtime_error("out of retries");
}

int do_something_else(std::string const& url)
{
    HttpRequest req{ url };
    HttpResponse resp;
    return worker(req, resp);
}

and now we found that we have an undesirable behavior in HttpClient::execute: if the HttpResponse is not empty when writing (for example, we received 503 Service is unavailable, try again and get 200 OK), it executesimply adds to the existing content. This is not what we want: we already acted in response 503, and we do not want the contents of answer 200 to be added to any content that the answer 503 had.

We looked at our code and found several ways similar do_something, and only one do_something_else. We never needed the (random) ability to add to an existing answer.

---

, , ? :

• ; HttpResponse do_something_else; ( , :)
• HttpClient::execute, HttpResponse . ( ).
• - HttpClient, HttpResponse, HttpClient::execute. , , - , . , .
• , , , . execute_append execute ( , ).
• execute, HttpRequest HttpResponse . execute HttpResponse, . , NRVO, HTTP. , , HttpClient::execute googlemock. , , , SetArgReferee, Returns.

?

Answer 1

HttpClient::execute - HTTP .

, HttpResponse .

execute() req HttpResponse.

++ 11 , POD, , , - .