Can i use nested loops with vectors in cpp?

Question

i have a cpp problem and i don't know whats wrong.. maybe you can help me :). I'm trying to implement a data structure for a graph. In this graph i will connect some nodes, which have a small euclidean distance, but at the second iteration, my iterator will point to 0x0. This case appears only, if i give the distance of those two nodes to std::cout. Here is my code:

for(vector<Node*>::iterator n1 = g->getNodes().begin(); n1 != g->getNodes().end(); ++n1)
{
    for(vector<Node*>::iterator n2 = g->getNodes().begin(); n2 != g->getNodes().end(); ++n2)
    {
        if(*n2 == 0)
        {
            // This will be entered after the first iteration of n2.
            cout << "n2 null" << endl;
            continue;
        }

        double distance = (*n1)->getDistance(*n2); // just euclidean distance
        if(distance <= minDistance)
        {
            // This works fine:
            cout << "(" << *n1 << "," << *n2 << ") << endl;

            // This brings me a "Segmentation fault"
            cout << "(" << *n1 << " , " << *n2 << ") -> " << distance << endl;
        }
    }
}

Is this owed by the nested loops? Can any body tell me my fault? Thanks a lot!

EDIT: Here is some more code: node.h

#ifndef NODE_H_
#define NODE_H_
#include <vector>
#include <iostream>
#include <limits>
#include <math.h>

using namespace std;

class Node
{
private:
    int x, y, z;
public:
    Node(int x, int y, int z) : x(x), y(y), z(z)
    {
    }

    inline int getX()           { return x; }
    inline int getY()           { return y; }
    inline int getZ()           { return z; }

    inline double getDistance(Node* other)
    {
        return sqrt(pow(x-other->getX(), 2) + pow(y-other->getY(), 2) + pow(z-other->getZ(), 2));
    }
};

#endif

graph.h

#ifndef GRAPH_H_
#define GRAPH_H_

#include <vector>
#include "node.h"
using namespace std;

class Graph
{
private:
    vector<Node*> nodes;
public:
    ~Graph()
    {
        while(!nodes.empty())
        {
            delete nodes.back(), nodes.pop_back();
        }
    }
    inline vector<Node*> getNodes()             { return nodes; }
    inline int getCountNodes()                  { return nodes.size(); }
    bool createNode(int x, int y, int z)
    {
        nodes.push_back(new Node(x, y, z));
        return true;
    };
#endif

main.cc

#include <iostream>
#include <vector>
#include <algorithm>
#include <math.h>
#include "model/graph.h"
using namespace std;

int main()
{
    Graph *g = new Graph();
    int nodeDistance = 100;
    for(int z = 0; z <= 300; z += nodeDistance)
    {
        for(int x = 0; x <= 500; x += nodeDistance)
        {
            for(int y = 0; y <= 300; y += nodeDistance)
            {
                g->createNode(x, y, z);
            }
        }
    }

    for(vector<Node*>::iterator n1 = g->getNodes().begin(); n1 != g->getNodes().end(); ++n1)
    {
        for(vector<Node*>::iterator n2 = g->getNodes().begin(); n2 != g->getNodes().end(); ++n2)
        {
            if(*n2 == 0)
            {
                // This will be entered after the first iteration of n2.
                cout << "n2 null" << endl;
                continue;
            }

            double distance = (*n1)->getDistance(*n2); // just euclidean distance
            if(distance <= nodeDistance)
            {
                // This works fine:
                cout << "(" << *n1 << "," << *n2 << ") << endl;

                // This brings me a "Segmentation fault"
                cout << "(" << *n1 << " , " << *n2 << ") -> " << distance << endl;
            }
        }
    }

    delete g;
    return 0;
}

Answer 1

One major issue is that your getNodes function returns a copy of a vector, not the original vector. Therefore your iterators you use in the loops are not iterating over the same vector.

Instead, the iterators you're using in the nested loops are iterating over 4 different (but equivalent) vectors instead of the actual vector from the object in question.

There is nothing wrong in returning a copy of a vector in general. However when you do this, you have to make sure you call such a function if you really want a copy, and not the same vector. Using the getNodes function as you used it is not a valid usage in terms of what you are trying to accomplish.

The error is here:

inline vector<Node*> getNodes()  { return nodes; }

The fix:

inline vector<Node*>& getNodes() { return nodes; }

The latter ensures that a reference to the actual vector in question is returned, not a copy of the actual vector. You can add an additional function that returns the vector as a copy if you want to still have the functionality available.