initializing objects in a constructor

mzimmers (578)
I seem to be having a senior moment, and can't for the life of me remember how to do this.

I have a class:

1
2
3
4
5
6
7
8
9
10
 
class NyqMultBlock

{
	FlipFlopReg32	regDelay, regProduct;
	const	bool	delay;
public:
	NyqMultBlock(int32_t rv=0, bool d=false);//  constructor w/ reset parameter
.
.
.


And another class:

1
2
3
4
5
6
 
class DemodNyqCell
{
	vector<NyqMultBlock>		blocks;
	int32_t						sum;
	int32_t						resetValue;
public:


When an object of type DemodNyqCell is created, I want to create a vector of NyqMultBlock elements, and initialize them. They won't all be initialized the same.

And, I can't remember how to code the constructor to give me access to the member objects' constructor. Can someone jog my memory?

Thanks.

hamsterman (4538)
Google "initialization list". Though that isn't too useful here. You'll be able to call vector's constructor, but not its elements. You should instead do
1
2
3
4
5
6
 
... {
   blocks.reserve(n);
   blocks.push(constructor1);
   blocks.push(constructor2);
   ...
}
Last edited on
mzimmers (578)
Thanks, hamsterman. I assume you mean like this (I didn't put in any of the changing parameters here, to keep the example simple):

1
2
3
4
5
6
7
8
 
DemodNyqCell::DemodNyqCell(int32_t rv)    // default constructor
	: sum(rv), resetValue(rv)
{
	for (int i = 0; i <= DEMOD_NYQ_NBR_BLOCKS; ++i)
	{
		blocks.push_back(NyqMultBlock(rv, false));
	}
}


That compiles, so I'm halfway home. Now, I need to do something different in my copy constructor, because of the const member of NyqMultBlock. I'm accustomed to writing copy constructors like this:

1
2
3
4
5
6
7
8
9
 
DemodNyqCell::DemodNyqCell			// copy constructor
(const DemodNyqCell &dnc)
{
	for (int i = 0; i <= DEMOD_NYQ_NBR_BLOCKS; ++i)
	{
		blocks.at(i) = dnc.blocks.at(i);
		sum = dnc.sum;
	}
}


but the compiler is telling me I can't use the default assignment operator. What am I doing wrong here?

Thanks again.
Last edited on
mzimmers (578)
OK, now that I'm looking at with rested eyes, I can see that this wouldn't work, because the vector elements haven't been created. So, now I tried this:

1
2
3
4
5
6
7
8
9
10
11
12
 
DemodNyqCell::DemodNyqCell			// copy constructor
(const DemodNyqCell &dnc)
{
	blocks.reserve(DEMOD_NYQ_NBR_BLOCKS);
	for (int i = 0; i <= DEMOD_NYQ_NBR_BLOCKS; ++i)
	{
//		blocks.at(i) = dnc.blocks.at(i);
		blocks.push_back(NyqMultBlock(dnc.blocks.at(i).getResetValue(),
					  dnc.blocks.at(i).getDelay()));
		sum = dnc.sum;
	}
}


which seems like it should work, but I'm getting a compiler error (actually two). Here's the first:

error: passing 'const NyqMultBlock' as 'this' argument of 'int32_t NyqMultBlock::getResetValue()' discards qualifiers


Anyone want to tell me what this means, and maybe what I'm doing wrong? Constructors are still a bit of a mystery to me.

Thanks.
mzimmers (578)
I've also tried bypassing the calls to the constituent object by doing this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
DemodNyqCell::DemodNyqCell			// copy constructor
(const DemodNyqCell &dnc)
{
	int32_t	l_rv;
	bool	l_d;

	blocks.reserve(DEMOD_NYQ_NBR_BLOCKS);
	for (int i = 0; i <= DEMOD_NYQ_NBR_BLOCKS; ++i)
	{
//		blocks.at(i) = dnc.blocks.at(i);

		l_rv = dnc.blocks.at(i).getResetValue();
		l_d = dnc.blocks.at(i).getDelay();

//		blocks.push_back(NyqMultBlock(dnc.blocks.at(i).getResetValue(),
//									  dnc.blocks.at(i).getDelay()));
		blocks.push_back(NyqMultBlock(l_rv, l_d));
		sum = dnc.sum;
	}
}


But I'm getting the same error. Anyone? I really have no idea how to interpret this error message.

Thanks...
ne555 (10692)
Now, I need to do something different in my copy constructor, because of the const member of NyqMultBlock
Nope. Every member will be created with its own copy constructor. The constant member will be a copy of the other constant member.
The problem arises if you have something without a copy constructor, like a stream or a reference. (or if the copy is no good, like with pointers)

getResetValue() discards qualifiers
That means that your method is not const correct. in the copy constructor you are passing a constant object, so you can only use constant methods
1
2
3
4
 
class foo{
public:
  void bar() const; // it can be used with constant objects
};


By the way, it is a good idea to reserve() in order to avoid reallocations.
Last edited on
mzimmers (578)
I commented out the "const" on my member variable, because I was getting some other errors I didn't want to deal with at the time. So, I don't think that's the problem; the only place I use "const" is in the copy constructors.

SIK (309)
When having a const member in your class you should get the error msg:

... must be initialized in constructor base/member initializer list


So the implementation for NyqMultBlock constructor should be:

1
2
3
4
5
6
7
 
#define DEF_DELAY   20

NyqMultBlock::NyqMultBlock(int32_t rv, bool d) : delay(DEF_DELAY)
{
   ...
   ...
}


This should take care of the initialization of the const value without violating const by later on trying to set value.

ne555 (10692)
1
2
3
4
5
6
7
8
9
10
 
class foo{
public:
  void a_constant_method() const;
  void destroy_the_world();
};

void bar( const foo &constant_object ){
  constant_object.a_constant_method(); //life is good
  constant_object.destroy_the_world(); //passing ‘const foo’ as ‘this’ argument of ‘void foo::destroy_the_world()’ discards qualifiers
}
mzimmers (578)
ne555: I'm sorry, but I'm not following you on this. Are you saying I need to declare my copy constructor as const? And again, I've removed (temporarily) the const modifier from my member declaration.
SIK (309)
I think mzimmers is pointing that your error:

error: passing 'const NyqMultBlock' as 'this' argument of 'int32_t NyqMultBlock::getResetValue()' discards qualifiers

relates to your getResetValue method is not a const method - ie it does not have te const keyword after its definition - which implies that the method may change the state of the object.

In your copy constructor however, you are passing this as

const DemodNyqCell &dnc

this means that the entire dnc instance shouldn't change after te copy construction has taken place - however the call to:

dnc.blocks.at(i).getResetValue()

is potentially changing the respective element stored in dnc, hence is ultimately changing dnc which can't happen since it's const.
mzimmers (578)
Thank you for the explanation, SIK. So what is the lesson here: that any function used in a copy constructor must be const? (Given that copy constructors are passed const pointers.)

Anyway, that compiled. Last thing to clean up: I returned the "const" qualifier to my member in the first post, and am back to getting this error with respect to my default constructor:

error: non-static const member 'const bool NyqMultBlock::delay', can't use default assignment operator
/usr/include/c++/4.2.1/bits/vector.tcc: In member function 'void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = NyqMultBlock, _Alloc = std::allocator<NyqMultBlock>]':
/usr/include/c++/4.2.1/bits/vector.tcc:256: note: synthesized method 'NyqMultBlock& NyqMultBlock::operator=(const NyqMultBlock&)' first required here


So, the compiler is telling me I need to make my own assignment operator, but I have a feeling that the real solution lies elsewhere. Any help would be appreciated. Thanks!
ne555 (10692)
So what is the lesson here: that any function used in a copy constructor must be const?
Yep. But remember that you can access at the private data, as you are inside the class.
Also the default copy constructor is enough in most cases.


AFAIK, yes, you need to provide an assignment operator. Kinda sucks considering that you want a member to member copy.
But as after a = b; they may have different 'delay' value, makes sense.

On second thought ¿wtf is the STL doing?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
  template<typename _Tp, typename _Alloc>
    void
    vector<_Tp, _Alloc>::
    _M_insert_aux(iterator __position, const _Tp& __x){
      if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage){ //¿what does this mean?
        this->_M_impl.construct(this->_M_impl._M_finish, (*(this->_M_impl._M_finish - 1)) );
        ++this->_M_impl._M_finish;

        _Tp __x_copy = __x; //¿what is this for?

        std::copy_backward(__position.base(), this->_M_impl._M_finish - 2, this->_M_impl._M_finish - 1);

        *__position = __x_copy; //using the assignment
      }

OK, now I'm confused. (push_back) It is not using the assignment operator (that makes sense), ¿why is it asking it for?
Last edited on
mzimmers (578)
Yeah, something's just not right here. For clarity, let me post some updated code snippets:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
class NyqMultBlock

{
	FlipFlopReg32	regDelay, regProduct;
	int32_t			resetValue;
	const	bool	delay;

public:
	NyqMultBlock(int32_t rv=0, bool d=false);	//  constructor w/ reset parameter
	NyqMultBlock (const NyqMultBlock &nmb);	// copy constructor
...
NyqMultBlock::NyqMultBlock 	// copy constructor
(const NyqMultBlock &nmb)
	: regDelay(nmb.regDelay), regProduct(nmb.regProduct), delay(nmb.delay) {}


and

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
class DemodNyqCell
{
	vector<NyqMultBlock>		blocks;
	int32_t						sum;
	int32_t						resetValue;
public:
	DemodNyqCell(int32_t rv = 0);				// default constructor
	DemodNyqCell(const DemodNyqCell &dnc);		// copy constructor
...
DemodNyqCell::DemodNyqCell(int32_t rv)    // default constructor
	: sum(rv), resetValue(rv)
{
	bool	l_d;

	blocks.reserve(DEMOD_NYQ_NBR_BLOCKS);
	for (int i = 0; i <= DEMOD_NYQ_NBR_BLOCKS; ++i)
	{
		l_d = false;
		blocks.push_back(NyqMultBlock(rv, false));
	}
}


All I'm trying to do is instantiate an object from within another object's constructor. It's hard to believe that I'd have to supply an operator for that.

Any ideas, anyone?

EDIT: just for fun, I created an assignment operator, and got the predictable error about trying to assign a read-only data-member (delay). So, that doesn't help; it only postpones the problem. Could it be that what I'm trying to do just isn't legal? Seems unlikely, but I don't know what else to try.
Last edited on
SIK (309)
Herewith my implementation of these classes:

NyqMultBlock.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 

#define DEF_DELAY    false


class NyqMultBlock
{
public:
   NyqMultBlock(int rv=0, bool d=false);
   NyqMultBlock(const NyqMultBlock & rhs);
   ~NyqMultBlock();

   void operator =(const NyqMultBlock & rhs);


private:
   double             regDelay, regProduct;
   const bool       delay;

};


NyqMultBlock.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
 
#include "NyqMultBlock.h"



NyqMultBlock::NyqMultBlock(int rv, bool d) : delay(DEF_DELAY)
{

}


NyqMultBlock::NyqMultBlock(const NyqMultBlock & rhs) : delay(rhs.delay)
{
   *this = rhs;
}


NyqMultBlock::~NyqMultBlock()
{

}


void NyqMultBlock::operator =(const NyqMultBlock & rhs)
{
   regDelay = rhs.regDelay;
   regProduct = rhs.regProduct;

   // ...
}



DemodNyqCell.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
...
class DemodNyqCell
{
public:
   DemodNyqCell(int rv);
   DemodNyqCell(const CDemodNyqCell & rhs);
   ~DemodNyqCell();

private:
	vector<NyqMultBlock>		blocks;
	int   						sum;
	int   						resetValue;
};
...



DemodNyqCell.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
#include "DemodNyqCell.h"


DemodNyqCell::DemodNyqCell(int rv) : sum(rv), resetValue(rv)
{
   for (int i=0; i<DEMOD_NYQ_NBR_BLOCKS; i++)
   {
      blocks.push_back(NyqMultBlock(rv, false));
   }
}


DemodNyqCell::DemodNyqCell(const DemodNyqCell & rhs)
{
   resetValue = rhs.resetValue;
   sum = rhs.sum;
   blocks = rhs.blocks;
}


DemodNyqCell::~DemodNyqCell()
{

}


I changed some of the types for ease of compiling, but this should effectively still show what is important - unless I'm not understandng the issue correctly.

The assignment operator is required by stl's vector class since we are not storing pointers of NyqMultBlock but actual objects of that class - hence internally when the vector wants to set the object received in push_back call to it's allocated value internally it needs to have the [ = ] resolved for this class.

Does this help?
mzimmers (578)
Hey, SIK: thanks for putting so much work into this. What you have above is essentially what I have, with the exception that you're using a #define instead of a const for the delay. (I was taught to avoid #defines, but that's another story for another time.) So...I take it, your version compiles? I guess it gets around the problem with the const assignment I'm trying to in the constructor.

Thanks for looking at this so closely.
Topic archived. No new replies allowed.