What is wrong with this copy constructor?

Neb1000 (48)
Ok so everything in this program works except for the copy constructor when I test it. When I run the program it segmentation faults.

Copy constructor:
1
2
3
4
5
6
7
8
9
10
 
template<typename generic>
Deque<generic>::Deque(Deque<generic>& d)
{
	m_size = d.m_size;
	m_blocks = d.m_blocks;
	m_block_size = d.m_block_size;
	m_data = new Block<generic>*[m_blocks];
	m_data[0] = new Block<generic>(m_block_size);
	*this = d;
}


It could have something to do with the assignment operator but
I don't know so here it is:
1
2
3
4
5
6
7
8
9
10
11
12
13
 
template<typename generic>
Deque<generic>& Deque<generic>::operator=(const Deque<generic>& d)
{
	clear();
	if(d.m_size > 0)
	{
		m_block_size = d.m_block_size;
		for(int i = 0; i < d.m_size; i++)
		{
			push_back(d[i]);
		}
	}
}


And here is the .h file if you need to see it:
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
30
31
32
33
34
35
36
37
 
#ifndef DEQUE_H
#define DEQUE_H

#include<stdexcept>
using std::out_of_range;
#include "block.h"

template<typename generic>
class Deque
{
 public:
  Deque();
  Deque(unsigned int n);
  Deque(Deque& d);
  ~Deque();
  void push_front(generic x);
  void pop_front();
  void push_back(generic x);
  void pop_back();
  void clear();
  Deque& operator=(const Deque& d);
  generic& operator[](unsigned int p);
  const generic& operator[](unsigned int p) const;
  unsigned int size() const;
  unsigned int block_size() const;
  unsigned int blocks() const;
  bool empty() const;

 private:
  Block<generic>** m_data;
  unsigned int m_size;
  unsigned int m_blocks;
  unsigned int m_block_size;
};

#include"deque.hpp"
#endif


If you need to see any other code or and explination of what a function does please tell me I need to get assignment done. Thanks for the help :)
firedraco (6247)
First, the copy constructor needs to take a const Deque&, just like the = operator.

I think the problem is that on line 8 in the copy ctor, you are only initializing the first "row" of Blocks. What if you have 0 rows, or 5?
Neb1000 (48)
Do you mean like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
 
template<typename generic>
Deque<generic>::Deque(Deque<generic>& d)
{
	m_size = d.m_size;
	m_blocks = d.m_blocks;
	m_block_size = d.m_block_size;
	m_data = new Block<generic>*[m_blocks];
	for(int i = 0; i < m_blocks; i++)
	{
		m_data[0] = new Block<generic>(m_block_size);
	}
	*this = d;
}

This also doesn't work and I don't think that the const will make a difference.
firedraco (6247)
You need to change the 0 to an i on line 10.

And the const will make a difference eventually, probably not in this case, but you should have it there anyway.
Neb1000 (48)
Ok well that was a dumb mistake but I agree with you about the const but my teacher gave me the .h file and when he test it with his test code it could mess up and I would lose points.
Now it runs but doesn't pass the test. What is wrong with this test?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
void Test_deque::test_copy_constructor()
{
	Deque<int> d;
	cout << endl << "TEST COPY CONSTRUCTOR" << endl;
	cout << "checking while empty: ";
	Deque<int> dd(d);
	CPPUNIT_ASSERT(d.empty() == dd.empty());
	CPPUNIT_ASSERT(d.size() == dd.size());
	cout << "PASSED" << endl;

	cout << "checking while full: ";
	for(int i = 0; i < TEST_MAX; i++) // TEST_MAX = 5000
	{
		d.push_back(i);
	}
	Deque<int> ddd(d);
	CPPUNIT_ASSERT(d.size() == ddd.size());
	for(int i = 0; i < TEST_MAX; i++)
	{
		CPPUNIT_ASSERT(d[i] == ddd[i]); // fails here
	}
	cout << "PASSED" << endl;
}


The same test works with the assignment operator which is basicly the same thing.
Last edited on
firedraco (6247)
What do "clear()" and "push_back()" do? I think the problem might be with how your copy ctor allocates memory, then just immediately clears() it by calling operator =. It might be easier just to have your copy ctor do this:

1
2
3
4
 
Deque<generic>::Deque(Deque<generic>& d) {
    //set up any state variables that need to be set before you call operator =
    *this = d;
}
Neb1000 (48)
Here is the code for push_back()
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
template<typename generic>
void Deque<generic>::push_back(generic x)
{
	if(m_data[m_blocks-1] -> bfull()) // bfull() checks if the back of the block is full
	{
		Block<generic>** temp = m_data;
		m_data = new Block<generic>*[++m_blocks];
		m_data[m_blocks-1] = new Block<generic>(m_block_size);
		for(int i = 0; i < m_blocks-1; i++)
		{
			m_data[i] = temp[i];
		}
		delete [] temp;
	}
	m_data[m_blocks-1] -> push_back(x);
	m_size++;
}


and clear()
1
2
3
4
5
6
7
8
 
template<typename generic>
void Deque<generic>::clear()
{
	while(m_size != 0)
	{
		pop_front();
	}
}


and I guess you will need pop_front() too
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
 
template<typename generic>
void Deque<generic>::pop_front()
{
	if(m_size != 0)
	{
		if(m_data[0] -> empty())
		{
			Block<generic>** temp = m_data;
			m_data = new Block<generic>*[--m_blocks];
			for(int i = 0; i < m_blocks; i++)
			{
				m_data[i] = temp[i+1];
			}
			delete temp[0];
			delete [] temp;
			m_data[0] -> pop_front();
			m_size--;
		}
		else
		{
			m_data[0] -> pop_front();
			m_size--;
		}
	}
}


The = operator has to clear it because that is not the only place it is used. It can be used other places where it might already have stuff stored in it.
Neb1000 (48)
Ok I figured it out, I was making it more complecated than it really was. I just needed to initalize it as an empty Deque with size 0 and go from there. Thanks for the help anyway. :)
firedraco (6247)
Ah, good. Glad you fixed it.
Topic archived. No new replies allowed.