Problem in logic, not in code?

[superQ]

So I'm writing this simple project, just for my own entertainment. Its nearly done, and most of it works fine. Part of it involves four functions that move in one of four directions randomly in a two dimensional matrix. Three of these functions work perfectly. Down does not.

I figured this out by putting a cin.ignore to slow down the output and I followed it action by action until I realized that when the down switch has to wrap around to the top (When theres no lower row) the point in the matrix that called it gets duplicated instead of swapping.

So I've gone over this tiny tiny function for hours, trying to figure out how it might be doing this, and I'm stumped. Theres no errors, so I dont think my syntax is bad or anything, my logic is just messed up. Can someone take a look?

Here is the broken down function:

void symgame::swapdown(int y, int x)
{
	int temp, tempdown, a; //a is a temp y
	temp = matrix[y][x];
	if (y+1 <= 8 && matrix[y+1][x] != '~')
	{
		a = y+1;
		tempdown = matrix[a][x];
	}
	else if (y+1 > 8 || matrix[y+1][x] == '~')
	{
		a = -1;
		do
		{
			a++;
			tempdown = matrix[a][x];
		}while (matrix[a][x] == '~'); 
	}
	matrix[a][x] = temp;
	matrix[y][x] = tempdown;
	matrixmoves[matrix[y][x]]++; //Matrix moves, by the way, is the number
	matrixmoves[matrix[a][x]]++; //of swaps that number has travelled
}

And here, for comparison, is the working up function.

void symgame::swapup(int y, int x)
{
	int temp, tempup, a;
	temp = matrix[y][x];
	if (y-1 >= 0 && matrix[y-1][x] != '~')
	{
		a = y-1;
		tempup = matrix[a][x];
	}
	else if (y-1 < 0 || matrix[y-1][x] == '~')
	{
		a = 9;
		do
		{
			a--;
			tempup = matrix[a][x];
		}while (matrix[a][x] == '~');
	}
	matrix[a][x] = temp;
	matrix[y][x] = tempup;
	matrixmoves[matrix[y][x]]++;
	matrixmoves[matrix[a][x]]++;
}

If anyone feels up to looking through it that would be amazing, I'm driving myself insane over it. Also if you want to see any other code just lemmie know, but theres alot of it. It isn't the most elegant program.

[superQ]

Now I'm officially more confused. I built a little test .cpp with a main() i threw together in 10 minutes and C/P'd the broken function in to see what would happen. Heres the whole thing...

#include <iostream>
using namespace std;

void swapdown(int, int);

int matrix[9][1];
int matrixmoves[9];

int main()
{
 
 for (int i = 0; i < 9; i++)
	 matrix[i][1] = i;
 for (int i = 0; i < 9; i++)
	 matrixmoves[i] = i;

 for (int j = 0; j < 9; j++) 
	 cout<<matrix[j][1]<<endl;
 cout<<endl<<endl;
 swapdown(8,1);
 for (int j = 0; j < 9; j++) 
	 cout<<matrix[j][1]<<endl;

 return 0;
}


void swapdown(int y, int x)
{
	int temp, tempdown, a; //a is a temp y
	temp = matrix[y][x];
	if (y+1 <= 8 && matrix[y+1][x] != '~')
	{
		a = y+1;
		tempdown = matrix[a][x];
	}
	else if (y+1 > 8 || matrix[y+1][x] == '~')
	{
		a = -1;
		do
		{
			a++;
			tempdown = matrix[a][x];
		}while (matrix[a][x] == '~'); 
	}
	matrix[a][x] = temp;
	matrix[y][x] = tempdown;
	matrixmoves[matrix[y][x]]++;
	matrixmoves[matrix[a][x]]++;
}

And the output is:

0
1
2
3
4
5
6
7
0


0
1
2
3
4
5
6
7
2

Wtf happened?!?! First off I'm obviously an idiot because I'm getting 012345670 for an unaltered list of nine consequtive numbers, and then something was written over like I expected, but it was 2....I have no idea whats going on anymore...

[Disch]

re: your second post, you're corrupting the heap. Line 6 makes the matrix with dims [9][1], and later you index [x][1] (and not [x][0] like you should be)

This could explain why your numbers are not sequential, as your writes to 'matrix' might actually be writes to 'matrixmoves', and so when you actually do write to 'matrixmoves' you're corrupting your 'matrix' values.

Still looking at original post. Will edit/make another post later if I find anything.

EDIT: what significance does the '~' character have? If all you want to do is swap to adjacent elements, the logic is pretty straightforward, and you shouldn't need to do any looping like you are now. Can you explain what the '~' is all about?

[smilodon]

what are the dimensions of your matrix
based on your second post I am guessing 8x8
but your code suggests 9x9

[superQ]

@both of you

symgame::symgame()
{
	int temp0[9][9] = {
		{ '~','~','~','~',0,'~','~','~','~' },
		{ '~','~','~',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,38,39,'~','~','~' },
		{ '~','~','~','~',40,'~','~','~','~' }
		};
	for(int i = 0; i < 9; i++)
		for(int j = 0; j < 9; j++)
			matrix[i][j]=temp0[i][j];

Thats the matrix in the initializer. The swap needs to wrap around not only at the bottom of the matrix but at the ~'s, which are the boundries of the real grid.

[EDIT] And yeah, I knew I just did something stupid in that test code :-p I'm getting frustrated and angry with this. Its really a simple program, I'm just being a moron haha.

[superQ]

Interesting update: after fixing that really stupid mistake in my little crap test program, it worked. But in the real program, the exact same code is still overwriting.

[Disch]

I don't see anything wrong with your swapdown function. I'd wager your problem is elsewhere (I wouldn't be surprised if it was more heap corruption --- those are nasty buggers).

If you can supply the whole program I can take a closer look. If it's too much to paste on these boards, you can use a site like pastebin or upload the source files to geocities or something.

[superQ]

http://pastebin.com/m5a27db38

Note that there are three seperate files in there, a .h and 2 .cpps. I seperated them with lots of ///////'s. And don't bother trying to figure out what I did with matrixshow(); It wasn't very pretty to begin with but pastebin's text wrapping made it completely incomprehensible.

[EDIT] Oh FYI if you compile it yourself and want to look at the output, un-// the system("pause") so you can slow it down and look at whats happening step by step. With out the pause, you'll see that eventually everything gets overwritten with one number, but that can take awhile. The first overwrites start early though, best to slow it down.

[Disch]

I'm on linux so I had to remove the windows crap --- and after replacing system("pause") with getchar() I was ready to go.

The problem is with swapright, not swapdown

void symgame::swapright(int y, int x)
{
	int temp, tempright, a;
	temp = matrix[y][x];
	if (x+1 <= 8 && matrix[y][x+1] != '~')
	{
		a = x+1;
		tempright = matrix[y][a];
	}
	else if (x+1 > 8 || matrix[y][x+1] == '~')
	{
		a = -1;
		do
		{
			a++;
			tempright = matrix[a][x];   //  [a][x] ?  pretty sure you mean [y][a]
		}while (matrix[a][x] == '~');    // same 
	}
	matrix[y][a] = temp;
	matrix[y][x] = tempright;
	matrixmoves[matrix[y][x]]++;
	matrixmoves[matrix[y][a]]++;
}

You also did have a bit of buffer overflow, but that didn't seem to cause errors -- YET:

int matrixmoves[40];  // since the range is 0-40 inclusive, this should actually be [41] ! 

Hopefully that fixes things for you.

[superQ]

Huh. Well how about that. Thankye disch, works like a charm now :)

You're a life saver. I was going to go insane...

[Disch]

No problem. Glad I could help. ^^

The problem was you assumed too much about the problem without fully diagnosing it. Because the copy from bottom to top (or from top to bottom or whatever) you assumed it was a problem with swapdown -- and so you had it in your head that swapdown had a problem when in fact it was fine, resulting in you banging your head against the wall looking in all the wrong places.

To diagnose a problem you should isolate it, and get it to be consistently repeatable. Your original diagnosis of "swapdown breaks when it tries to wrap" only looked at the symptom, but if you would have tried swapdown(8,4) on its own (like you did you the mini test program you wrote separately), you would've found that it wasn't repeatable, and that might have hinted that it wasn't the problem.

If you can set breakpoints so they trip just before the bug presents itself, you can step through the code in a debugger to find the exact line that's causing the problem. However this hinges on you being able to know when the bug is going to trip -- and therefore means you must reproduce it (ie: get rid of rand() stuff for debugging, and substitute in constants -- like 8,4 -- which appear to be triggering the problem).

Anyone will tell you that the hardest bugs to track down are the ones you can't reliably reproduce -- the ones that appear to just happen at random.

Debugging is like a whole separate thing you have to learn on top of coding. It can be very rough! And it's nowhere near as fun -- but it's important nonetheless.

[superQ]

Yeah, I'm starting to learn that. I tend to end up going through line by line on my own, running the program step by step in my head. This works until I go and write myself a program with a randomizer in it :-p

Thanks again for the help Disch :)