copy constructor causing confusing error message. (gcc c++)

qwijibow · 09-21-2005, 07:19 AM

Hey guys, im having quite alot of trouble with this copy constructor, and i just dont understand the error message.

Compile Command

Code:

g++ -c ./geometry.cpp

geometry.hpp

Code:

#ifndef GEO_HPP
#define GEO_HPP

namespace nn {

	class geometry {
		private:
			int MAX;
			int *neuronLayer;

		public:
			int getMax();
			int *getMem();

			geometry(const geometry&);
			geometry(const int*,int);
			~geometry();

			int getLayerNum(int);
			int getInputNum();
			int getOutputNum();
	};
};

#endif

geometry.cpp

Code:

#include"geometry.hpp"
#include<cstring>
using std::memcpy;

int nn::geometry::getMax() { return MAX; }
int* nn::geometry::getMem() { return neuronLayer; }

nn::geometry::geometry(const nn::geometry& g) {

	MAX = g.getMax();
	neuronLayer = new int[MAX];
	memcpy(neuronLayer, g.getMem(), sizeof(int) * MAX);
}

nn::geometry::gometry(const int *layer, int layers) {

	MAX=layers;
	neuronLayer = new int[MAX];
	memcpy(neuronLayer, layer, sizeof(int) * MAX);
}

nn::geometry::~geometry() { delete[] neuronLayer; }

int nn::geometry::getLayerNum(int n) {

	return neuronLayer(n);
}

int nn::geometry::getInputNum(int n) {

	return getLayerNum(0);
}

int nn::geometry::getOutputNum(int n) {

	return getLayerNum(MAX-1);
}

ERROR MESSAGE !!!

Code:

g++ -c src/geometry.cpp -o build/geometry.o
src/geometry.cpp: In copy constructor `nn::geometry::geometry(const nn::geometry&)':
src/geometry.cpp:11: error: passing `const nn::geometry' as `this' argument of `int nn::geometry::getMax()' discards qualifiers
src/geometry.cpp:13: error: passing `const nn::geometry' as `this' argument of `int* nn::geometry::getMem()' discards qualifiers
src/geometry.cpp: At global scope:

any idea's ???

df_0903 · 09-21-2005, 08:59 AM

code:

#ifndef GEO_HPP
#define GEO_HPP

namespace nn {

class geometry {
private:
int MAX;
int *neuronLayer;

public:
int getMax()const;
int *getMem()const;

geometry(const geometry&;
geometry(const int*,int);
~geometry();

int getLayerNum(int)const;
int getInputNum()const;
int getOutputNum()const;
};
};

#endif

orgcandman · 09-21-2005, 09:03 AM

Your first problem is that the declaration of the copy constructor should be:

Code:

  geometry(const geometry & ); //notice, no ; before the )

Your next problem is that in the CPP file, you don't need to explicity state the namespace (why even use a namespace).
Just say "using namespace nn;" and don't worry about using it in the rest of the file. Or drop the namespace entirely.

The member variable neuronLayer doesn't get called as though it were a function. If you're looking for an element in the array,
you use the array operators. (search online for using arrays).

getInputNum() and getOutputNum() take arguments in the CPP file, but aren't declared to take arguments in the header.

Now for the copy constructor issue. In copy constructors, you should NEVER EVER call member functions of the reference class. It's a
no-no. Instead, you should just directly assign from the member variables.

IE:

Code:

   MAX = g.MAX;
   ...
   memcpy(neuronLayer, g.neuronLayer, sizeof(int) * MAX);

Also, there are better ways of instantiating certain member variables (like ints, etc) without having to specify foo == reference.foo; or
other silly things. Look up member list initialization.

qwijibow · 09-21-2005, 01:37 PM

Thanks...
but from what i remember, its considered bad programming practice to declaire member variables public, when they are only manipulated internally within the class.

and that they should be decalired private, and use an accessor function if somthing needs to read it from outside the class.

by adding the accessor, and calling it dureing the copy, i was following general rule of thumb.

Why is it a no-no to call the referance on a copy constructor ?
And how can i copy neuronLayer without making it pubic, or using an accessor ???

thanks.

sirclif · 09-21-2005, 02:29 PM

yes, dont make them public. that is considered bad by most people. im not sure what the proper technique for copy constructors is...

anyhow, for present and future reference, your specific error is occuring because you are calling member functions of an object that is declared constant, and the functions are not declared constant. specifically

Code:

nn::geometry::geometry(const nn::geometry& g) {

	MAX = g.getMax();
	neuronLayer = new int[MAX];
	memcpy(neuronLayer, g.getMem(), sizeof(int) * MAX);
}

in this case, g is declared to be a constant object of type nn::geometery. then you call g.getMax(), you get an error because you have not specified that getMax() is a function that can be called by a constant object. you would need to declare the function as:

int getMax() const;

and defined as:

int getMax() const {return MAX;}

as df_0903 replied but didnt explain.

this tells the compiler that getMax() can be called by a constant object. this allows you to limit the access of functions by constant object, so that you can keep them from accessing any functions that would change thier member variables.

qwijibow · 09-21-2005, 02:35 PM

Excellent, Thanks for that explanation, just what i needed

orgcandman · 09-21-2005, 08:34 PM

with copy constructors, since the argument is a reference to the same type, it doesn't matter if the variable is public or private...you have access to it.

Of course, for external interfaces you allow access only through appropriate get/set functions.

However, for internal data structures, you don't want to make function calls to get access to the member variables. The reason is that you make extra overhead on the stack, and increase CPU processing time. This is especially true for virtual functions since the linker has to make additional lookups.