Finding occurrence of vector entries in another vector without nested for loops

Question

I have a piece of code that I'm migrating from Fortran to C++, and I'd like to avoid some of the nested for loop structures I had to create in the original F77 code.

The problem is this: I have a vector of objects called nodes that each include a vector holding (among other important info) the indices of other node objects to which each is connected (a connection graph). Like this

struct Node {
    vector<int> conNode;
};
vector<Node> listOfNodes;
vector<int> nodeListA;    // a subset of nodes of interest stored as their vector indices

I need to look for nodes that nodes in nodeListA are connected to, but only if those nodes are also in nodeListA. Right now, my code looks something like this:

// Loop over the subset of node indices
for (int i=0; i<nodeListA.size(); i++) {
    // Loop over the nodes connected to the node i
    for (int j=0; j<listOfNodes[nodeListA[i]].conNode.size(); j++) {
        // Loop over the subset of node indices again
        for (int k=0; k<nodeListA.size(); k++) {
            // and determine if any of node i's connections are in the subset list
            if (nodeListA[k] == listOfNodes[nodeListA[i]].conNode[j]) {
               // do stuff here
            }
        }
    }
}

There HAS to be a much simpler way to do this. It seems like I'm making this way too complicated. How can I simplify this code, possibly using the standard algorithm library?

I don't know if you use `nodeListA` for purposes other than the one shown here. But if this is the main or only purpose, it may be a good idea to use a `std::set` or (C++11 or Boost) a `std::unordered_set`, rather than a `std::vector` for it. Sets are much more suitable for searching. — jogojapan, Oct 19 '12 at 15:49

Tadeusz Kopec for Ukraine · Accepted Answer · 2012-10-20T08:43:28.950

If your variable should express a set of values, use std::set instead of std::vector. Then you'll have

typedef std::set<int> SetOfIndices;
SetOfIndices setOfIndices; // instead of nodeListA
for(SetOfIndices::const_iterator iter = setOfIndices.begin(); iter != setOfIndices.end(); ++iter)
{
    Node const & node = listOfNodes[*iter];
    for (int j = 0; j < node.conNode.size(); ++j)
    {
        if (setOfIndices.find(node.conNode[j]) != setOfIndices.end())
        {
            // do stuff here
        }
    }
}

EDIT As Jerry Coffin suggests, std::set_intersection can be used in outer loop:

struct Node {
    SetOfIndices conNode;
}
typedef std::set<int> SetOfIndices;
SetOfIndices setOfIndices; // instead of nodeListA
for(SetOfIndices::const_iterator iter = setOfIndices.begin(); iter != setOfIndices.end(); ++iter)
{
    Node const & node = listOfNodes[*iter];
    std::vector<int> interestingNodes;

    std::set_intersection(setOfIndices.begin(), setOfIndices.end(),
                      node.conNode.begin(), node.conNode.end(),
                      std::back_inserter(interestingNodes));

    for (int j = 0; j < interestingNodes.size(); ++j)
    {
        // do stuff here
    }
}

ANOTHER EDIT
About efficiency - it depends what is the dominant operation. The number of executions of part described as "do stuff here" will not vary. The difference is in time of traversing your collections:

Your original code - nodeListA.size()^2 * [average conNode size]
My first solution - nodeListA.size() * log(nodeListA.size()) * [average conNode size]
After Jerry Coffin suggestion - nodeListA.size()^2 * [average number of interesting conNode elements]

So it seems that set_intersection use doesn't help in this case.

If you're using `std::set` (or just keep the items in the vectors sorted) look up `std::set_intersection` and see if it won't work. — Jerry Coffin, Oct 19 '12 at 15:58
Ah, that's what I was looking for on the one hand. On the other, it arguably looks more complex than the original. Is there an efficiency improvement when doing it this way? — Fadecomic, Oct 19 '12 at 16:19

jogojapan · Answer 2 · 2012-10-20T02:42:15.467

I'd suggest using a dictionary (an O(log n) one like std::set, or better a hash-based one like std::unordered_set from C++11) for nodeListA. The following is a C++11 code example.

#include <unordered_set>
#include <vector>

struct Node {
  std::vector<int> conNode;
};

int main()
{
  std::vector<Node>       listOfNodes;
  std::unordered_set<int> nodeListA;

  for (int node_id : nodeListA)
    for (int connected_id : listOfNodes[node_id].conNode)
      if (nodeListA.find(connected_id) != end(nodeListA))
        /* Do stuff here.. */
          ;

  return 0;
}

The advantage of using a std::unordered_set is that look-ups (i.e. searching for a given node-id) are extremely fast. The implementation included in your standard library, however, may not be particularly fast. Google's sparse hash and dense hash implementation is an alternative that provides the same interface and is known to be very good for most purposes: http://code.google.com/p/sparsehash/

Depending on what you want to do with the resulting nodes, it may be possible to replace the inner loop of the above code with an STL algorithm. For example, if you want to put all the nodes identified by the algorithm in a vector, you could code it as follows (use this as a replacement for both loops together):

std::vector<int> results;
for (int node_id : nodeListA)
  std::copy_if(begin(listOfNodes[node_id].conNode),
               end(listOfNodes[node_id].conNode),
               back_inserter(results),
               [&nodeListA](int id){return nodeListA.find(id) != end(nodeListA);});

Again, this is C++11 syntax; it uses a lambda as function argument.

Finding occurrence of vector entries in another vector without nested for loops

2 Answers2