Compilation error redefining object

[SirLukeJonPerrier]

main:

#include <cstdlib>
#include <iostream>
#include <fstream>
#include "Board.cpp"

int main(int argc, char* argv[]){
	argc=5;
	argv[argc];

	int rowNum=atoi(argv[1]);
	int colNum=atoi(argv[2]);
	
	int startRow=atoi(argv[3]);
	int startCol=atoi(argv[4]);

	if(startRow<=0 || startRow>rowNum || startCol<=0 || startCol>colNum){
		std::cout <<"Invalid starting point, cannot continue.";
		exit(-1);
	}

	Board b(rowNum, colNum);
	Knight knight;
	knight.setPosition(startRow, startCol);
	bool success=b.solve(startRow, startCol, 0);
	b.print(success);
}

Board.cpp

#include "Board.h"
#include <iostream>
#include <cstdlib>

Board::Board(int nRows, int nCols){
	numRows=nRows;
	numCols=nCols;

	//creates array of pointers(rows) each pointing to array of items(columns)
	board=new int*[numRows];
	for(int i=0; i<numRows; i++){
		board[i]=new int[numCols];
		
		for(int j=0; j<numCols; j++){
			board[i] [j]=-1;
		}
	}
}

Board::Board(const Board& brd){
	this->numRows=brd.numRows;
	this->numCols=brd.numCols;

	for(int i=0; i<brd.numRows; i++){
		for(int j=0; j<brd.numCols; j++){
			this->board [i] [j]=brd.board[i] [j];
		}
	}	
}

Board::~Board(){
	for(int j=0; j<numRows; j++){
		delete [] board[j];
	}
	delete [] board;
}

bool Board::solve(int row, int col, int stepCount){

	bool moveWorked=false;

	//checks that move is in range of board
	if(row>=0 && row<numRows && col>=0 && col<numCols){
		//checks that space hasn't been moved to yet
		if(board [row] [col]==-1){
			board [row] [col]=stepCount;
			stepCount++;
			
			//if stepCount made it past number of spaces on board, done	
			if(stepCount>(numRows*numCols)){
				return true;
			}
			//check all possible moves
			if(!moveWorked)
				moveWorked=solve(row+2, col+1, stepCount);
			if(!moveWorked)
				moveWorked=solve(row+2, col-1, stepCount);
			if(!moveWorked)
				moveWorked=solve(row+1, col+2, stepCount);
			if(!moveWorked)
				moveWorked=solve(row+1, col-2, stepCount);
			if(!moveWorked)
				moveWorked=solve(row-2, col+1, stepCount);
			if(!moveWorked)
				moveWorked=solve(row-2, col-1, stepCount);
			if(!moveWorked)
				moveWorked=solve(row-1, col+2, stepCount);
			if(!moveWorked)
				moveWorked=solve(row-1, col-2, stepCount);
			if(!moveWorked){
				//if every move option fails, backtrack to previous move
				//and reset space on board
				board [row] [col] = -1;
				stepCount--;
			}
			return moveWorked;
		}
	}
	else{
		return false;
	}
}

void Board::print(bool success){
	if(success){
		std::cout <<"Success!\n";
	}
	else{
		std::cout <<"Failed!\n";
	}	

	for(int i=0; i<numRows; i++){
		for(int j=0; j<numCols; j++){
			if(board [i] [j]<10){
				std::cout <<"[ " <<board [i] [j] <<"] ";
			}
			else{
				if(board [i] [j]<0){
					std::cout <<"[  ] ";
				}
				else{				
					std::cout <<'[' <<board [i] [j] <<"] ";
				}
			}
		}
	std::cout <<'\n';
	}
}

Board.h:

#include "Knight.h"

class Board{
	private:
		int** board;
		int numRows;
		int numCols;
		int move;

	public:
		Board(int nRows, int nCols);
		Board(const Board& brd);
		~Board();
		
		bool solve(int row, int col, int stepCount);
		void print(bool success);
};

and my barely used Knight class, which will probably be consolidated into a single method within Board:

#include "Knight.h"

Knight::Knight(){
	
}

Knight::~Knight(){

}

void Knight::setPosition(int x, int y){
	knightRow=x;
	knightCol=y;
}

int Knight::getKnightRow(){
	return knightRow;
}

int Knight::getKnightCol(){
	return knightCol;
}

and Knight.h:

class Knight{
	private:
		int knightRow;
		int knightCol;

	public:
		Knight();
		~Knight();
		
		void setPosition(int x, int y);
		int getKnightRow();
		int getKnightCol();
};

If you still want to take a look, I can't find a place where there are two definitions of any of the Board methods

[Disch]

Do you give bodies for these in your class header? That'd be the most likely thing I can think of.

On a side note... your copy ctor is wrong. You never allocate or initialize this->board so you're writing to a bad pointer there.

[SirLukeJonPerrier]

@Disch, I added the relevant portion of my header, I did allocate this->board within the header.

[Disch]

Hm your class body looks fine. Are you doing something weird like #including a cpp file or something? And you're sure you don't have a second definition of these functions somewhere?




Side point / Lesson in OOP:
Your copy ctor is still wrong. The ctor is run first so calling initializeBoard to create the array will not work in this case because the copy ctor is trying to write the the array:

Board::Board(const Board& brd){  // <- copy ctor
	this->numRows=brd.numRows;
	this->numCols=brd.numCols;
	// this->board is uninitialized.  initializeBoard has not been called yet

	for(int i=0; i<brd.numRows; i++){
		for(int j=0; j<brd.numCols; j++){
			this->board [i] [j]=brd.board[i] [j]; //<- EXPLODE!!!!
			//  trying to write to this->board but it hasn't
			//  been initialized yet
		}
	}	
}

Further, initializeBoard kind of defeats the point of having a constructor. The constructor is supposed to do the initializing.

At the very least your ctor should be null-ing the board pointer so you don't try to delete a bad pointer in your dtor. Example:

int main()
{
    {
        Board a;  // <- ctor called... 'board' left uninitialized
        // <- do not call initializeBoard
    } // <- dtor called here... you try to delete a.board... but a.board is unitialized
       // so deleting it will probably make your program explode 

In order for your class to work as expected... you are requiring the user of the class have to call functions in a very specific order.

First: ctor
second: setBoard
third: initializeBoard

If they call those things in the wrong order, or forget to call one of them... not only will the class not work as expected... but it will likely EXPLODE and cause the program to crash.

Can you imagine if classes like std::string, std::vector, etc worked like that? They'd be much, much harder to use.

Also... if they call initializeBoard multiple times... then your class will leak memory. And if you call setBoard after initializeBoard, then your board will have incorrect values for the number of rows/columns.

The whole point of OOP is to make objects "self contained" so the user has to actually try very hard to misuse them. You are treating functions like they are simple getters/setters (setBoard sets your width/height.... initializeBoard sets your board). Getters/setters are bad OOP.

But anyway I'll stop talking your ear off because this is not related to your actual problem.

[SirLukeJonPerrier]

I understand that, but I changed it to the above because I thought it was part of the problem. I originally had a single call in main, "Board b(rowNum, colNum);" and then this as my constructor instead:

Board::Board(int nRows, int nCols){
	numRows=nRows;
	numCols=nCols;

	//creates array of pointers(rows) each pointing to array of items(columns)
	board=new int*[numRows];
	for(int i=0; i<numRows; i++){
		board[i]=new int[numCols];
		
		for(int j=0; j<numCols; j++){
			board[i] [j]=-1;
		}
	}
}

Board::Board(const Board& brd){
	this->numRows=brd.numRows;
	this->numCols=brd.numCols;

	for(int i=0; i<brd.numRows; i++){
		for(int j=0; j<brd.numCols; j++){
			this->board [i] [j]=brd.board[i] [j];
		}
	}	
}

Board::~Board(){
	for(int j=0; j<numRows; j++){
		delete [] board[j];
	}
	delete [] board;
}

so that makes more sense in your context, but I'm getting a similar problem

[Disch]

Even in that code... you still are not allocating the board in your copy ctor. You're only doing it in the default ctor. Remember only one ctor is called so if the copy ctor is running that means the default ctor did not run.

But yes, that is better than what you had in your first post.

That said... I can't help further with your main problem without seeing more (read: probably all) of the code. You must have either a 2nd definition somewhere or you must be #including something you shouldn't be. You're not pasting the part of the code that has the problem.

EDIT: and actually... a reasonable IDE would probably let you just double-click on the error message and it'll jump you to the 2nd definition.

[SirLukeJonPerrier]

I don't have a reasonable IDE, but it's a work in progress. But feel free to look into the rest of my code above.

[xismn]

int main(int argc, char* argv[]){
	argc=5;
	argv[argc];

	int rowNum=atoi(argv[1]);
	int colNum=atoi(argv[2]);
	
	int startRow=atoi(argv[3]);
	int startCol=atoi(argv[4]);
//... 

Bro those lines scare me.

[norm b]

Bro those lines scare me.

I'm with xismn.

Compiled here. Only made the following changes:

int main(int argc, char* argv[]){
//	argc=5;
//	argv[argc];

	int rowNum=atoi(argv[1]);

Ran it with a command line of:

./app 8 8 1 2

and got the following output

Success!
[ -1] [ -1] [ -1] [ -1] [ -1] [ -1] [ -1] [ -1]
[ -1] [ -1] [ 0] [ -1] [ -1] [ -1] [ -1] [ -1]
[ -1] [22] [ -1] [ -1] [ -1] [ -1] [ -1] [ -1]
[ -1] [ -1] [ -1] [ 1] [ -1] [ -1] [16] [ -1]
[21] [ -1] [19] [ -1] [17] [ -1] [ -1] [ -1]
[10] [ -1] [ 8] [ -1] [ 2] [13] [ 4] [15]
[ -1] [20] [11] [18] [ -1] [ 6] [ -1] [ -1]
[ -1] [ 9] [ -1] [ 7] [12] [ 3] [14] [ 5]

[SirLukeJonPerrier]

I know, but I'm really limited to specific formatting for my class, by the professor. Fixed my problem, accidentally #included Board.cpp instead of Board.h. Stupid mistake I overlooked too many times. Thanks y'all