program crashes when I try to access a vector, please help

[Synthesizer]

Hi, I'm new here, I hope I posted this in the correct forum.

I want to describe a network of nodes. Each node needs to connect to all its neighbours. Therefore, in order to be able to do some calculations, I need to access the properties of each of the neighbours of the node that I am currently working on. I decided to set this up with a vector, which is new to me. My code compiles just fine, but when I run it, the program just crashes. I THINK that I am doing something wrong in the iterator arithmetic, I THINK that I am messing about with incompatible types; but I can't seem to figure out how to fix it; the fixes I tried didn't compile and gave me error messages that didn't help me. I'll provide snippets from my code, and I would be thankful if anybody would point me in the right direction. (NOTE: there are more functions and there is more code in the program, but I think I have the relevant part in this post. If you suspect differently, please tell me so!)

In node.h:

#ifndef NODE_H
#define NODE_H

#include <cstdio>
#include <iostream>
#include <cmath>
#include <vector>

using namespace std;

class Node
{
    public:
        Node(); //default

        Node(double set_x, double set_y, int set_RowNumber); //argument ctor

        double Get_x() { return x; }
        void Set_x(double val) { x = val; }

        int Get_RowNumber() { return RowNumber; }
        void Set_RowNumber(int val) { RowNumber = val; }

        double Get_y() { return y; }
        void Set_y(double val) { y = val; }

        double Get_D(int i) {return D[i]; }
        void Set_D(int i, double val) { D[i] = val; }

        double Get_H(int i) { return H[i]; }
        void Set_H(int i, double val) { H[i] = val; }

        double Get_alpha(int i) { return alpha[i]; }
        void Set_alpha(int i, double val) { alpha[i] = val; }

    protected:
    private:
        double x;
        double y;
        int RowNumber;
        double D[6];
        double H[6];
        double alpha[12];
};

class Network
{
    public:
        Network(int set_NumRows, int set_NumNodesInRow, double *xval, double *yval, int *RowNumberval); //ctor
        Node NodeIndex(int index) { return nodelist.at(index); }
        void CalculateFacets() ;
        int GetNumNodes() { return NumNodes; }
        int GetNumNodesInRow() { return NumNodesInRow; }
        int GetNumRows() { return NumRows; }

    private:
        std::vector<Node> nodelist;
        int NumRows;
        int NumNodesInRow;
        int NumNodes;
};
#endif // NODE_H 

and from node.cpp:

#include ".\\include\\Node.h"

Node::Node(double set_x, double set_y, int set_RowNumber)
{
    x = set_x;
    y = set_y;
    RowNumber = set_RowNumber;
    for (int i = 0 ; i <12; i++)
    {
        if (i < 6)
        {
            D[i] = 0;
            H[i] = 0;
        }
        alpha[i] = 0;
    }
}


Network::Network(int set_NumRows, int set_NumNodesInRow, double *xval, double *yval, int *RowNumberval)
{
    int NumRows = set_NumRows;
    int NumNodesInRow = set_NumNodesInRow;
    int NumNodes = NumRows*NumNodesInRow;
    nodelist.resize(NumNodes);

    for (int i = 0; i<NumNodes; i++)
    {
        this->nodelist.at(i).Set_x(*(xval+i));
        this->nodelist.at(i).Set_y(*(yval+i));
        this->nodelist.at(i).Set_RowNumber(*(RowNumberval+i));
    }
}


void Network::CalculateFacets()
{
    int Ntot, Nperrow;
    Ntot = this->GetNumNodes();
    Nperrow = this->GetNumNodesInRow();

    int RowCounter = 0;
    int NodeCounter = 0;

    for (vector<Node>::iterator i = this->nodelist.begin();
                            i != (this->nodelist.end()-Nperrow);
                            i++) //ignore last row 

    {
        RowCounter = (*i).Get_RowNumber();
        cout << "RowCounter = "<<RowCounter<<endl;
        cout << "y i = "<< ( (*i).Get_y() )<<endl;
        cout << "x i = "<< ( (*i).Get_x() )<<endl; 
//program outputs correct values for RowCounter, x and y for the first node, then crashes, so the bug manifests itself in the next line:
        cout << "x i+Nperrow = " << ((*(i + Nperrow)).Get_x())<<endl;
        (*i).Set_D(1,( (*(i+Nperrow)).Get_x() - (*i).Get_x() ));
        cout << (*i).Get_D(1);
    }

PS: I didn't post main.cpp as I think it isn't relevant, I think the mistake is in how I reference node # (i+Nperrow); it would take quite a bit of "snipping" to present the relevant bit of the main code; I could do it if it is required for the solution.

PS2: I tried making Nperrow of type iterator, but clearly I am using an incorrect syntax, since all my attempts at that have resulted in compiler errors. Is that even what is wrong with the code?

PS3: thanks in advance!

[Catfish2]

Crashes with what, "segmentation fault"? That usually means you're trying to access memory you shouldn't.

And by the way:

Elegance is not a dispensable luxury but a factor that decides between success and failure.

The competent programmer is fully aware of the strictly limited size of his own skull; therefore he approaches the programming task in full humility, and among other things he avoids clever tricks like the plague.

The computing scientist's main challenge is not to get confused by the complexities of his own making.

- Edsger Dijkstra

[cire]

You're using needlessly complicated syntax which makes your code hard to read.

void Network::CalculateFacets()
{
    typedef std::vector<Node> vn_t ;

    int Nperrow = GetNumNodesInRow();

    for (vn_t::iterator i = nodelist.begin(); i != nodelist.end()-Nperrow; i++) //ignore last row
    {
        int RowCounter = i->Get_RowNumber();

        cout << "RowCounter = " << RowCounter << endl ;

        cout << "y i = " << i->Get_y() << endl;
        cout << "x i = " << i->Get_x() << endl; 
        cout << "x i+Nperrow = " << (i+Nperrow)->Get_x() <<endl;

        i->Set_D(1, (i+Nperrow)->Get_x() - i->Get_x()) ;

        cout << i->Get_D(1) ;
    }
}

Declare your variables as close as possible to first use. The fact that the error occurs on line 17 of the snippet above, would seem to suggest that i+NPerrow is out of range for the vector it refers to, however you don't provide a minimal, compilable code example that exhibits the problem so it's hard to say what exactly may be wrong.

[ Edit: Well, actually I think I see the problem. The class variables NumRows, NumNodesInRow, and NumNodes are never assigned values in the Network constructor. Instead you create local variables that hide the class variables and assign values to those. ]

[Synthesizer]

Thanks for your answers. Yes, it crashes on a segmentation fault, sorry for not mentioning that. I don't have formal programming training and am kind of teaching it to myself, using books and online information. I have written a smaller version of this program which lacked some functionality which I am trying to add now. I introduced the vector description because I wanted to be able to resize it at runtime (I will need that for my added functionality), and I read that the vector class helps you take care of memory. I figured now's as good a time as any to learn how to use it.

I don't fully comprehend your answers yet but I'll study it more and try some simple stuff, figure it out and actually learn how to avoid the same mistake in the future (rather than pasting the code and move along).

Could I bother you for a follow up question before studying this further?
What does this do:
typedef std::vector<Node> vn_t ;
In my understanding, this only makes it such that you don't have to write "std::vector<Node>" in the for loop and any future call (i.e. just write vn_t for brevity and readability). What I am wondering: am I wrong and does it do something more than that?

[cire]

The typedef is purely to make the for loop more readable.

for ( auto i=nodelist.begin(); ... ) should be preferred if your compiler supports the auto keyword.

[Catfish2]

if your compiler supports the auto keyword.

That is to say, the latest C++ standard, from 2011, named C++11.

Anyway, as cire posted, you might be inputting a value for NPerrow which throws i+NPerrow out of range.
(Also, NPerrow is of type (signed) int. What if, it happens to store a negative value?)

Another thing: you are overusing getters and setters.
If you don't add data sanity checks to your setters, and simply trust the data you're being fed, it's a sign you may be better off putting your data members in the public section instead.