How do I improve my vector search function?

Question

So I was implementing a TCP-server in c++ and was storing all the users in a vector. Now I need a more generalized function to search for multiple different properties, How do I improve this code?

struct client {
    std::string ip_address = "";
    int socket_id = 0;
    bool blocking = false;
};

enum client_codes {
    ip_address,
    socket_id,
    blocking,
};

template<typename T>
std::vector<client>::iterator search_vector(std::vector<client> &list, int type, T query) {

    std::vector<std::function<bool(client)>> comparators;

    comparators.push_back([&](client ob) {return ob.ip_address == std::to_string(query); });
    comparators.push_back([&](client ob) {return ob.socket_id == query; });
    comparators.push_back([&](client ob) {return ob.blocking == query; });

    return std::find_if(std::begin(list), std::end(list), [&](client obj) {return comparators[type](obj); });

}

// Implementation
std::vector<client> client_list;
auto search1(search_vector(client_list, socket_id, 321));
auto search2(search_vector(client_list, blocking, true));
auto search3(search_vector(client_list, ip_address, "192.168.0.85"));

To ask about improvement of already working code you should better ask at [SE Code Review](https://codereview.stackexchange.com/). — πάντα ῥεῖ, Mar 23 '19 at 16:21
What do you mean by "search for multiple different properties" exactly? Can you give an example? — Acorn, Mar 23 '19 at 16:46
Keep it simple. There are no reason to fill a vector and then pass an index into the vector pass the appropriate predicate directly. Or have simple (possibly inline) functions like `FindBySocketId`. That way, you follow the **DRY** principle and you localize the code that know the internal details of how the data is kept which can help maintenance if at some point you need to improve the code. — Phil1970, Mar 23 '19 at 17:28

alter_igel · Accepted Answer · 2019-03-23T16:44:13.293

Since you're searching over different properties of the same types, I'd say this is a rare use case for pointers to members. The following code should help clean this up:

struct client {
    std::string ip_address = "";
    int socket_id = 0;
    bool blocking = false;
};

// search_vector accepts a pointer to member of type T, and a value of type T
template<typename T>
std::vector<client>::iterator search_vector(std::vector<client>& list, T client::*member, T value){
    return std::find_if(list.begin(), list.end(), [value, member](const client& c){ return c.*member == value; });
}

This is how you would now use it, without any extra enums or special logic.

auto it1 = search_vector(client_list, &client::socket_id, 321);
auto it2 = search_vector(client_list, &client::blocking, true);
auto it3 = search_vector(client_list, &client::ip_address, "192.168.0.85");

This exact search_vector function is of course limited to member variables, and exact equality tests. But it would be fairly straightforward to extend this with pointers to member functions for functions like get_ip_address().

For more generality, an overload with something like std::function<bool(T)> condition instead of T value could also help you do more specific searches when you don't want exact equality.

score 0 · Answer 2 · answered Mar 23 '19 at 16:29

Here are a few suggestions:

Assuming you're searching the client vector multiple times - instead of using std::find_if, sort your vector and perform a binary search (e.g. using std::equal_range). If you often add new clients, consider using an additional, small, unsorted buffer of recently-added clients, and occasionally integrating the two into a single sorted vector.
Don't place the code which checks the search type within the predicate - that way it get run again and again; and you also need the heap-based vector of comparators - ugh! ... instead template your search function on the search type, and have it use a single comparator. Use a wrapper function which selects the appropriate search function.

How do I improve my vector search function?

2 Answers2