https://www.cplusplus.com/forum/general/279238/

[Cplusc]

Let say I have a mesh of two elements and four nodes.
Coordinates

    1              -8         3.12281               0
    2              -8         1.12281               0
    3              -6         3.12281               0
    4              -6         1.12281               0

Elements

1 2 4 3
2 3 1 2

I have two classes for node and Elements,

#ifndef _NODE_H_
#define _NODE_H_

#include "includes.h"
#include "typedefs.h"

class Node {
public:

	VecDbl_t* Coordinates_;
	Node() = default;
	Node(VecDbl_t* coord);

	double get_x();
	double get_y();
};

#endif

#include "node.h"


Node::Node(VecDbl_t* coord) {
	Coordinates_ = coord;
}

double Node::get_x() {
	return (*Coordinates_)[0];
}
double Node::get_y() {
	return (*Coordinates_)[1];
}
[code]
[code]#ifndef _ELEMENT_H_
#define _ELEMENT_H_

#include "includes.h"
#include "typedefs.h"
#include "node.h"

typedef std::vector<Node>		VecNode_t;

class Element{
public:

	VecIdx_t* Connectivities_;
	VecVecDbl_t DN_;
	double ElmSize_;
	VecDbl_t baricenter_;
	size_t Node0;
	size_t Node1;
	size_t Node2;

	Element() = default;
	Element(VecIdx_t* connec, VecNode_t& Nodes);
	double Calculate_ElmSize_(VecVecDbl_t& tmp_coordinates);
	void Calculate_DN_(VecVecDbl_t& tmp_coordinates);

};

#include "element.h"
#include "node.h"
#include <algorithm>

Element::Element(VecIdx_t* connec, VecNode_t& Nodes) {

	VecVecDbl_t tmp_coordinates;
	Connectivities_ = connec;
	size_t n_nodes = Connectivities_->size();
	tmp_coordinates.resize(n_nodes);

	for (size_t i = 0; i < n_nodes; i++) {
		size_t i_node = (*Connectivities_)[i];
		//tmp_coordinate.push_back(Coordinate(Nodes[i_node].get_x(), Nodes[i_node].get_y()));
		tmp_coordinates[i].push_back(Nodes[i_node].get_x());
		tmp_coordinates[i].push_back(Nodes[i_node].get_y());
	}

	Calculate_ElmSize_(tmp_coordinates);
	Calculate_DN_(tmp_coordinates);
}

now in main I have vector of nodes and elements,

		VecNode_t Nodes;
		for (size_t i_node = 0; i_node < Global_Mesh.Coordinates_.size(); i_node++) {
			VecDbl_t coord = Global_Mesh.Coordinates_[i_node];
			Node node(&coord);
			Nodes.push_back(node);

			std::cout << Nodes[i_node].get_x() << " " << Nodes[i_node].get_y() << std::endl;
		}

		VecElm_t Elements;

		for (size_t i_elm = 0; i_elm < Global_Mesh.Connectivities_.size(); i_elm++) {
			VecIdx_t connect = (Global_Mesh.Connectivities_[i_elm]);
			Element elm(&connect, Nodes);
            Elements.push_back(elm);
		}

when I run this program it gives me assertation failed, subscript out of range. I tried to use debugging method and I found that in element constructor when it wants to calculate the

tmp_coordinates[i].push_back(Nodes[i_node].get_x());
it will crash. when I continue debugging it goes to the get_x() function of Node class says vector out of range, why the same method is working for Nodes vector but for elements vector no?

[coder777]

Most likely i_node is invalid. You may place a debug cout there. It is possible Connectivities_is already corrupt.

[Cplusc]

I_node is giving me the correct value and Connectivities_ as well

[seeplus]

VecDbl_t coord = Global_Mesh.Coordinates_[i_node];
Node node(&coord);
Nodes.push_back(node);

coord is a local variable which looses scope at the end of the for body. Hence it is a new variable every time around the loop - which may or may not have a different address.

node is taking the address of coord and setting coordinates equal to this passed pointer using a shallow copy. But this pointer is only valid within the scope of the for loop body. Once the body ends and the next for iteration takes place, &coord is invalid. Depending upon the phase of the moon and star alignment, this might or might not 'appear' to work. But it isn't really.

Within the for loop L72 - 77, these saved addresses are de-referenced to get x and y - but these saved addressed are no longer valid. Hence the crash.

[Cplusc]

I understand what you said, when I try to print the results outside of for loop it crushes.what can be the possible solution for solving this problem?

[MikeyBoy]

Have the Node class store its own copy of the data it needs from coord, rather than a pointer to coord.

EDIT: If you absolutely must have it be a pointer to something that it doesn't own, then make sure it's pointing to something that you can be absolutely sure will remain valid for the lifetime of the Node object.

[Cplusc]

You mean passing the 2d vector of coord passing through Node class?

[MikeyBoy]

You mean passing the 2d vector of coord passing through Node class?

To clarify, is that question a reponse to the first of my suggestions, or to the second, edited-in suggestion?

[Cplusc]

First one

[MikeyBoy]

How would passing in the 2d vector cause Node to store its own copy of the data?

I'm saying you should change the way Node stores the coordinate data. Not changing what you pass into the constructor.

[Cplusc]

you mean something like this?

Node::Node(VecDbl_t coord) {
	Coordinates_ = coord;
}

double Node::get_x() {
	return Coordinates_[0];
}
double Node::get_y() {
	return Coordinates_[1];
}

or

		VecVecDbl_t coord;
		for (size_t i_node = 0; i_node < Global_Mesh.Coordinates_.size(); i_node++) {
			coord[i_node] = Global_Mesh.Coordinates_[i_node];
			Node node(&coord[i_node]);
			Nodes.push_back(node);

			std::cout << Nodes[i_node].get_x() << " " << Nodes[i_node].get_y() << std::endl;
			coord.push_back(coord[i_node]);
		}

Also I tried this, but no success

std::vector<double> Node::get_x() {
	VecDbl_t x_coord;
	x_coord.push_back((*Coordinates_)[0]);
	return x_coord;
}
std::vector<double> Node::get_y() {
	VecDbl_t y_coord;
	y_coord.push_back((*Coordinates_)[1]);
	return y_coord;
}

[seeplus]

Why have Node at all? What not just use std::pair<double, double> ?

Then:

typedef std::vector<std::Pair<double, double>>	VecNode_t;

Or even:

using Coord = std::pair<double, double>;
using VecNode_t = std::vector<Coord>;

Then something like (not tried);

VecNode_t Nodes;

for (size_t i_node = 0; i_node < Global_Mesh.Coordinates_.size(); i_node++) {
    const auto coord = Global_Mesh.Coordinates_[i_node];
    Nodes.emplace_back(coord[0], coord[1]);

    std::cout << Nodes[i_node].first << " " << Nodes[i_node].second << std::endl;
}

for (size_t i = 0; i < n_nodes; i++) {
    size_t i_node = (*Connectivities_)[i];
    //tmp_coordinate.push_back(Coordinate(Nodes[i_node].get_x(), Nodes[i_node].get_y()));
    tmp_coordinates[i].push_back(Nodes[i_node].first);
    tmp_coordinates[i].push_back(Nodes[i_node].second);
}

[Cplusc]

How pair vectors gonna work?

[MikeyBoy]

you mean something like this?

That follows my second suggestion, not my first. You still have Node storing nothing more than a pointer to the vector of co-ordinates, but at least you're making sure that the thing it's pointing to (an element of coord) persists for longer than the lifetime of the pointer itself.

Or are you? What's the lifespan of Nodes? If it outlives coord, then you'll still have pointers to memory that's no longer valid.

Just make Node own its own copy of the coordinates vector, rather than pointing to something it doesn't own.

[seeplus]

@resabzr - see my revised post above.

What is the type of Global_Mesh.Coordinates_ ?

[Cplusc]

There is text file containing all the mesh properties(connectivity and coordinate).Also, There is class Named Mesh responsible for reading the text file. The object of this class will be Global_Mesh. Then I send chunk of elements to each processor and that would be Local_mesh.
Both global and Local are of type Mesh.