Very newbie question

Started by
6 comments, last by Zahlman 18 years, 8 months ago
Hi all! I'm just starting out developing in C++, so please forgive the extreme stupidity of this question. I have the following function in a simple tic-tac-toe game, which pops up with the error. main.cc:94: error: incompatible types in assignment of `int' to `int[4]'


bool entry_process(char receive[2])
{
	char column, number;
	bool entry_valid;
	int x,y;
	
	column=receive[1];
	number=receive[2];
	entry_valid=TRUE;
	
	if (!(column=("a"||"b"||"c"||"A"||"B"||"C")))
		{entry_valid=FALSE;}
		
	if (!(number=("1"||"2"||"3")))
		{entry_valid=FALSE;}
		
	if (entry_valid=FALSE)
		{return FALSE;}
		
	if (column=('a'||'A'))
		{x=1;}
		
	if (column=('b'||'B'))
		{x=2;}
		
	if (column=('c'||'C'))
		{x=3;}
		
	if (number='1')
		{y=1;}
		
	if (number='2')
		{y=2;}
		
	if (number='3')
		{y=3;}
		
	grid[x,y]=player;	
	
	return TRUE;
		
}




The line in question is at the bottom (grid[x,y]=player). Does anyone know why the types apparently don't match? Thanks in advance for any help. J
Advertisement
First of all, you should use comparison operator (==) in your if statements instead of assignment operator (=). It just messes your code up and it's incorrect.

As for the error, I think it's self-explanatory. You're trying to assign an integer to an array of integers.
Assuming that grid is an two-dimensionall array the proper syntax for arrays is
grid[x][y] = player


Also, those if(column=('a'||'A')) won't work. Both a and A is larger than 0, thus true. a || A will because of that always be true. So what you're doing is checking if column is true, ie. larger that 0. The correct way would be:
if(column == 'a' || column == 'A')

And as j0seph said, it's very important to use == instead of =. Using = will result in setting column to true, or 1 since it's an int. And that would perhaps not be what you were trying to do, I suppose. Perhaps you should take at a look at the switch statement too, that would make your code a bit nicer.
Hope this helps [smile]
There is also the line:

column=receive[1];
number=receive[2];

you are reading outside the array, it should be

column=receive[0];
number=receive[1];

This is because in an array of n elements the first is 0 and the last is n-1, so in an array of 2 elements the first is 0 and the last is 1
Quote:Original post by jamyskis
The line in question is at the bottom (grid[x,y]=player). Does anyone know why the types apparently don't match? Thanks in advance for any help.


It is because you have only subscripted the (presumably 2d) array once, leaving a 1d array reference. The [x,y] is not appropriate syntax here; what you have instead invoked is the "comma operator" within a single subscript. The behaviour is "evaluate x (possibly causing side effects, although it won't here), throw the result away, evaluate y, and then use the result of evaluating y as a subscript".

Anyway, you have many, many errors here - legal constructs, but things that won't do anything remotely like what you want.

	column=receive[1];	number=receive[2];


This will fail at runtime; for an array of dimension [2], the valid subscripts are 0 and 1, not 1 and 2. Array indexing starts at 0; learn to work with it rather than against it. In the long run you will find it MUCH more convenient, trust me.

	entry_valid=TRUE;


In C++, bool is a real type (not like BOOL which is really a typedef for int), and its two possible values are properly spelled true and false (i.e. in lowercase). This will compile if you actually have the appropriate defines provided somewhere (either by yourself or from something you included), but it's just plain a bad idea. Make use of the boolean type; the C++ type system is much more advanced than what C offers, and is there to help you.

	if (!(column=("a"||"b"||"c"||"A"||"B"||"C")))


First, as noted by Perost, the logic is wrong here. Yes, || is read as "or", but it is not a magic "or" that can mean anything that English "or" can mean - the whole expression is still evaluated as an expression, with proper precedence. And remember that constants are expressions too, and anything non-zero is "true".

Second, you definitely want == there rather than =; a single equals sign is doing an assignment, not a comparison.

Third, single character constants (as opposed to strings) are properly encased in single quotes (not double). A double-quoted value is a "string literal", of type char* - that's something entirely different from the 'char' type of column.

The next several lines of course suffer from the same problems.

Oh, the whole entry_valid thing is needlessly complicating your logic. Guard clauses are a good idea here, but you can just return directly.

Fixed (in the sense of at least doing what it's intended to, with the same "general contract"; there are still lots of "style" issues):

bool entry_process(char receive[2]) {	// Where possible, you should initialize variables at declaration.	char column = receive[0];	char number = receive[1];	int x,y;	if (!(column == 'a' || column == 'b' || column == 'c' ||              column == 'A' || column == 'B' || column == 'C')) {		return false;	}	if (!(number == '1' || number == '2' || number == '3')) {		return false;	}	if (column == 'a' || column == 'A') {x=1;}	if (column == 'b' || column == 'B') {x=2;}	if (column == 'c' || column == 'C') {x=3;}	if (number=='1') {y=1;}	if (number=='2') {y=2;}	if (number=='3') {y=3;}	grid[x][y] = player; // <-- where does this 'player' come from?	return true;	}


Repetitive, isn't it? Fortunately, there are ways of dealing with that - but I'll not overload you with too much at once, yes? :)
Thanks everybody. I should have spotted the error with the array - comes from a history of using BASIC I guess, getting used to using square brackets instead of round ones takes some getting used to :-p

I have found out a number of times the hard way that the last element of an array is always one less than what it says in the line because it begins at 0.

I didn't know that "true" and "false" (lower case) were already defined, so I'd created constants for them. No matter, I can change that easily enough ;)

Anyway, again people, thanks for the help. :)

J
Hi all,

Here's the new code:

bool entry_process(char receive[2]){	int x,y;	char column=receive[0];	char number=receive[1];		if (!(receive[0]=='A'||receive[0]=='B'||receive[0]=='C'||receive[0]=='a'||receive[0]=='b'||receive[0]=='c')) {		return FALSE;}		if (!(receive[1]=='1'||receive[1]=='2'||receive[1]=='3')) {		return FALSE;}			if (!(receive[0]=='A'||receive[0]=='a')) {		x=1;}		if (!(receive[0]=='B'||receive[0]=='b')) {		x=2;}		if (!(receive[0]=='C'||receive[0]=='c')) {		x=3;}	if (receive[1]=='1') {		y=1;}	if (receive[1]=='2') {		y=2;}	if (receive[1]=='3') {		y=3;}				}


I think it works - all except for the fact that the parameter receive[2] is not being passed correctly. It wasn't checking them properly (always FALSE) so I put a quick std::cout in to display the values of the two variables in the array, which were turning out as some weird hexadecimal code. I can theoretically check against that, but that would probably break being able to compile on other OSs? Does anyone have any suggestions?

Thanks again,

J
Show the calling code.

Also, why not just pass two separate char variables? They don't have any relationship to each other, so the array "binding" isn't gaining you anything - you don't iterate over the array or anything.

This topic is closed to new replies.

Advertisement