C++: Help me finding a bug - 2D array search - - Computer Technical Help

Two Plus Two Forums Other Topics Computer and Technical Help

C++: Help me finding a bug - 2D array search -

Post Reply Subscribe

...

04-17-2014 , 11:13 AM

RiverPlay

adept

Join Date: Mar 2007 Posts: 1,191

My program asks for numbers and places them in a 2D array.
Then it asks for a number and performs a search, after which it prints position for said number within the 2D array.

Search function has a bug that adds 1 to expected row position. "Fixed" it by substracting 1 to "row" variable, but it is ovb not acceptable.

Currently feeling kind of dumb about this, help will be appreciated

Code:

#include <iostream>
using namespace std;

#define R 3
#define C 2

typedef int matrix[R][C];

void input(matrix m){
int i, j;
i = j = 0;
	
	for(i = 0; i< R; i++){
		for(j = 0; j < C; j++){ 
			cout << "Row " << i << " Column " << j << ": ";
			cin >> m[i][j];
		}
	}
}

void search(int x, matrix m, int& row, int& column){
int r, c;
bool OK;

OK = false;
r = 0;

	while((!OK) && (r < R))
	{
		c = 0;
		while((!OK) && (c < C))
		{
			if(m[r][c] == x)
				OK = true;
			else
				c++;
		}
	r++;	
	}

	column = c;
	row = r-1;  //LOL
}


int main(){
	matrix mat;
	int number;
	int row;
	int column;
		
	input(mat);
				
	cout << "Which number are you looking for?: ";
	cin >> number;
	search(number, mat, row, column);
	cout << endl << "Row " << row << " Column: " << column;

	fflush stdin;
	getchar();
	return 0;
}

Quote

04-17-2014 , 11:31 AM

Chips Ahoy

help me help you

Join Date: Apr 2007 Posts: 28,869

1. You don't say what isn't working. As in "when I input {} and search for x I expect y but get z"
2. Style: don't declare more than 1 variable per line. Do initialize when you declare. Do declare where you use [for(int i = 0...].
3. Why not have search return success or failure? Then you can get rid of OK and just return when you have a match. Also you can print something useful if no match.
4. Why not use for loops in search? You have initialization, test, increment. The while buys you nothing.

Quote

04-17-2014 , 12:03 PM

RiverPlay

adept

Join Date: Mar 2007 Posts: 1,191

Quote:

Originally Posted by Chips Ahoy

1.Search function has a bug that adds 1 to expected row position, ie: it searches for a value at row 2 column 1, and it prints row 3 column 1 (for row 0 it works fine).
3.Good point.
4.I think while loop is better because it stops searching as soon as it finds desired value. For an array this size is not relevant but array could get much bigger and it would be bad to keep searching after value is found, imo.

Quote

04-17-2014 , 12:22 PM

Chips Ahoy

help me help you

Join Date: Apr 2007 Posts: 28,869

Quote:

Originally Posted by RiverPlay

1.Search function has a bug that adds 1 to expected row position, ie: it searches for a value at row 2 column 1, and it prints row 3 column 1 (for row 0 it works fine).

That's because you have "r++" at the bottom of your outer loop. That happens every time, match or not.

Quote:

4.I think while loop is better because it stops searching as soon as it finds desired value. For an array this size is not relevant but array could get much bigger and it would be bad to keep searching after value is found, imo.

Except it doesn't stop right away. If you had only one loop this would be a spot for "break". Exiting multiple loops is a spot for "goto" but you need self-confidence to use that. I'd just do what I want once I have a match:

Code:

            for (int i = 0; i < R; i++)
            {
                for (int j = 0; j < C; j++)
                {
                    if (m[i][j] == x)
                    {
                        row = i;
                        column = j;
                        return true;
                    }
                }
            }

Also it's possible to have multiple conditions in a for loop:

Code:

            for (int i = 0; (i < R) && !MatchFound; i++)

Quote

04-17-2014 , 12:38 PM

RiverPlay

adept

Join Date: Mar 2007 Posts: 1,191

Ty, really good answer.

What about this as a way to end while loop?

Code:

void search(int x, matrix m, int& row, int& column)
{
int r, c;
bool OK;
OK = false;
r = 0;

	while((!OK) && (r < R))	
	{
		c = 0;
		while((!OK) && (c < C))
		{
			if(m[r][c] == x)
				OK = true;
			else
				c++;
		}
		if(!OK)
			r++;	
	}

	column = c;
	row = r; 
}

Quote

04-18-2014 , 11:46 PM

Bantam222

veteran

Join Date: Aug 2006 Posts: 2,388

That fix is functionality correct but pretty bad from a style/maintainability standpoint. You really should be pulling that code into it's own function and then you can just return true to break out of the loop when you find a match. If no match is found and the loop terminates, you know you can return false. This gets rid of your ugly "OK" variable. Then you can replace the while loops with simple for loops and iterate through the array. Makes the code a lot more readable.

Quote

04-19-2014 , 10:45 AM

ganstaman

Carpal \'Tunnel

Join Date: Sep 2008 Posts: 10,852

Also, something that took me a while to get was to name variables properly. OK is meaningless in that code. A better choice would be something like "found." You do your search while not found, and when the match is found, then found is true.

Quote

Post Reply Subscribe

...