Jump to content
  • Advertisement
Sign in to follow this  
moucard

can you confirm if the copy constructor and the = operator are ok?

This topic is 4749 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

OK, I've got this cell class which makes for a dungeon. Could you please confirm that I've implemented it correctly? (Especially the copy constructor and the assignment operator) I want to be able to make a vector of CCells.
Header
#ifndef CELL_H
#define CELL_H
#include <vector>
using namespace std;

class CCell
{
private:
	int ID;				// the cell position in the maze, counting row-wise
	int difficulty;		// the difficulty of the cell (1 for Dragon   (D)
						//							  -1 for Helper   (H)
						//							 -k1 for Fairy    (F)
						//						 k3 - k2 for Fighter  (G)
						//							   0 for Clearing (E)
	bool passed;		// set to true when the knight enters this cell
	char contents;
	vector<int> nextCells;	// the cells that the knight can go to
public:
	CCell(int ID_, char contents_, int difficulty_);
	CCell(CCell &);
	~CCell(void);

	CCell &operator=(CCell &h);
	int  getID();
	int  getDifficulty();
	char getContents();
	void visit();
	bool passedBefore();
	void addNextCell(int cellID);
};

#endif

Source
#include "Cell.h"

CCell::CCell(int ID_, char contents_, int difficulty_)
{
	ID			= ID_;
	contents	= contents_;
	difficulty	= difficulty_;
	passed		= false;
}

CCell::CCell(CCell &c)
{
	ID			= c.ID;
	contents	= c.contents; 
	difficulty	= c.difficulty;
	passed		= c.passed;
	nextCells	= c.nextCells;
}

CCell::~CCell(void)
{
	// Do nothing
}

CCell &CCell::operator=(CCell &c)
{
	ID			= c.ID;
	contents	= c.contents; 
	difficulty	= c.difficulty;
	passed		= c.passed;
	nextCells	= c.nextCells;

	return *this;
}

int  CCell::getID()
{
	return ID;
}

int  CCell::getDifficulty()
{
	return difficulty;
}

char CCell::getContents()
{
	return contents;
}

void CCell::visit()
{
	passed = true;
}

bool CCell::passedBefore()
{
	if (passed) 
		return true;
	else
		return false;
}

void CCell::addNextCell(int cellID)
{
	for (unsigned int i = 0; i < nextCells.size(); i++)
		if (cellID == nextCells)
			return;	// the cellID of an adjacent cell is already inserted. We can skip it.

	nextCells.push_back(cellID);
}

Thanks for your time.

Share this post


Link to post
Share on other sites
Advertisement
Test it yourself. It is very simple: make a console program that includes the code you posted and a variable declared as type vector<CCell>. Throw in a few lines manipulating the variable by changing the size of the vector and accessing some random elements. If it compiles successfully and doesn't crash when you run it, then it's probably fine.

Share this post


Link to post
Share on other sites
The copy constructor and assignment operator seem perfectly fine. There's one problem, but it's probably just an accident:

this:

void CCell::addNextCell(int cellID)
{
for (unsigned int i = 0; i < nextCells.size(); i++)
if (cellID == nextCells.ID)
return;

nextCells.push_back(cellID);
}

Share this post


Link to post
Share on other sites
Thanks guys. It's just my first time that I'm properly creating a class that will be used by a vector. I'm nervous and I'd hate things to go wrong down the line. That had happened a long time ago, when I had no idea about copy constructors and assignment operators and the thing blew my foot (after about 5000 lines of coding) so I'm trying to be very careful here.
IFoobar, I don't think I've got a mistake where you're pointing. I want to search the nextCells vector and if the cellID is _not_ in there, then I add it. Hope I'm alright there. I guess, just to be on the safe side, I'll do as mumpo suggested to.

Share this post


Link to post
Share on other sites
Yeeyyy. Actually testing the damn thing did pay off! I had to add the const keyword in the copy constructor like:

CCell::CCell(const CCell &c)
{
ID = c.ID;
contents = c.contents;
difficulty = c.difficulty;
passed = c.passed;
nextCells = c.nextCells;
}

I hope I'll get the hang of this. STL here I come!

Share this post


Link to post
Share on other sites
hehe congrats on it all ;]
  Although, you do know that you don't need a copy constructor or assignment operator for that class?
  I mean it's all just direct assignments of each member & that's what c++ would do by default anyway
  They are usually only needed if, for instance, your class stores pointers to allocated memory
  In that case you'd want it to stop copying the pointer address, & instead allocate seperate memory for the new instance & copy the contents across appropriately
  There other, ofcourse, other cases for special classes where you need to take control
  But in your class currently all of it's members will function perfectly fine with direct assignments
  C++ will do this on it's own anyway if you don't specify
But if you knew this & were just doing it for the sake of learning then ignore that babble & have a thumbs up :] \o/

Next step in learning would be looking up initialisation lists..

They're used in constructors (like your copy constructor) to specify values that you want to pass to the members as they are actually created & they come before the {} codeblock
@ the minute in your copy constructor they are being created first with empty (default) constructors & you are applying assignments to them after. While it's hardly a speedgain in most cases, actually specifying the values as parameters for the members constructors is slightly faster (well really it's just neater :P). But it's sometimes necassary if you find you have to pass stuff to their constructors
Sorry if I'm coming accross as pushy & if you feel I'm sweeping too fast just ignore what I've said
It doesn't look like it will be a necessity for you yet..
..But it is another usefull trick card you can add to your c++ deck

Share this post


Link to post
Share on other sites
Quote:
Original post by moucard
Thanks guys. It's just my first time that I'm properly creating a class that will be used by a vector. I'm nervous and I'd hate things to go wrong down the line. That had happened a long time ago, when I had no idea about copy constructors and assignment operators and the thing blew my foot (after about 5000 lines of coding) so I'm trying to be very careful here.
IFoobar, I don't think I've got a mistake where you're pointing. I want to search the nextCells vector and if the cellID is _not_ in there, then I add it. Hope I'm alright there. I guess, just to be on the safe side, I'll do as mumpo suggested to.


Doh!! Sorry, I thought that was a vector of CCell objects!

But anyway, as propuke said, you don't really need to define them urself since all the data the class contains, have their own assignment operator. The compiler would create an assignment and copy constructor for you by default for classes that are like that.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!