Find a service and update its last ping time I found those functions reviewing a C++14 codebase I am working on:

struct Service {
    Service(std::string id): id(id) {}
    virtual ~Service() {}

    std::string id;
    HeartbeatCheckpoint lastPing;
    HeartbeatCheckpoint lastPong;
};

std::map&lt;std::string, Service&gt; services;

bool Pacemaker::updatePong(const std::string&amp; id) {
    auto found = services.find(id);
    if(found != services.end()) {
        found-&gt;second.lastPong = std::chrono::high_resolution_clock::now();
        return true;
    }
    return false;
}

bool Pacemaker::updatePing(const std::string&amp; id) {
    auto found = services.find(id);
    if(found != services.end()) {
        found-&gt;second.lastPing = std::chrono::high_resolution_clock::now();
        return true;
    }
    return false;
}


I would like to refactor them creating a common function to remove code duplication, but I can't figure out how to pass the member variable (lastPing and lastPong).

Is there a better way to write this code?
The code as posted doesn't compile; I had to add some includes and definitions:

#include &lt;chrono&gt;
#include &lt;map&gt;
#include &lt;string&gt;

using HeartbeatCheckpoint = std::chrono::high_resolution_clock::time_point;


Having done that, the simplest refactoring of the functions would be to just pass a member pointer to specify the field you want to set in the found object:

bool update(const std::string&amp; id, HeartbeatCheckpoint (Service::*field))
{
    auto found = services.find(id);
    if (found == services.end())
        return false;
    found-&gt;second.*field = std::chrono::high_resolution_clock::now();
    return true;
}

bool updatePing(const std::string&amp; id) {
    return update(id, &amp;Service::lastPing);
}

bool updatePong(const std::string&amp; id) {
    return update(id, &amp;Service::lastPong);
}




Alternatively, something I've used before is a small wrapper around find to give it an interface that can be tested inline:

namespace collection
{
    template&lt;typename T&gt;
    inline bool contains(const T&amp; container, const typename T::key_type&amp; key)
    {
        return container.find(key) != container.end();
    }
    template&lt;typename T&gt;
    inline bool contains(const T&amp; container, const typename T::key_type&amp; key, typename T::const_iterator&amp; it)
    {
        return (it = container.find(key)) != container.end();
    }
    template&lt;typename T&gt;
    inline bool contains(T&amp; container, const typename T::key_type&amp; key, typename T::iterator&amp; it)
    {
        return (it = container.find(key)) != container.end();
    }
    template&lt;typename T&gt;
    inline bool contains(const T&amp; container, const typename T::key_type&amp; key, typename T::mapped_type&amp; result)
    {
        typename T::const_iterator it;
        return contains(container, key, it) &amp;&amp; (result = it-&gt;second, true);
    }
}


This is then used slightly more readably (at the expense of being unfamiliar outside your team):

bool updatePing(const std::string&amp; id) {
    std::map&lt;std::string, Service&gt;::iterator it;
    const bool found = collection::contains(services, id, it);
    if (found)
        it-&gt;second.lastPing = std::chrono::high_resolution_clock::now();
    return found;
}


It's arguably slightly neater if we assign to a pointer, rather than to an iterator:

namespace collection
{
    template&lt;typename T, typename V&gt;
    inline bool contains(const T&amp; container, const typename T::key_type&amp; key, const V *&amp; result)
    {
        auto it = container.find(key);
        if ((it = container.find(key)) == container.end())
            return false;
        result = &amp;it-&gt;second;
        return true;
    }
    template&lt;typename T, typename V&gt;
    inline bool contains(T&amp; container, const typename T::key_type&amp; key, V *&amp; result)
    {
        auto it = container.find(key);
        if ((it = container.find(key)) == container.end())
            return false;
        result = &amp;it-&gt;second;
        return true;
    }
}

bool updatePing(const std::string&amp; id) {
    Service *service;
    const bool found = collection::contains(services, id, service);
    if (found)
        service-&gt;lastPing = std::chrono::high_resolution_clock::now();
    return found;
}




As you haven't shown us where these functions are used, we can't determine whether the return value is important or not.  If it's not required, we can easily use the Null Object pattern to provide a sink for the result:

Service&amp; find_service_or_default(const std::string&amp; id)
{
    static Service dummy{""};
    auto it = services.find(id);
    return (it == services.end()) ? dummy : it-&gt;second;
}

void updatePing(const std::string&amp; id)
{
    find_service_or_default(id).lastPing = std::chrono::high_resolution_clock::now();
}

void updatePong(const std::string&amp; id)
{
    find_service_or_default(id).lastPong = std::chrono::high_resolution_clock::now();
}




This is all assuming that you have more functions than just those two - if not, then the refactoring is unlikely to be an improvement, and the original (with side-by-side duplication of a clearly recognisable structure) is likely to be more readable.