Logic Bug in a small C++ function...

DarkThinker

Platinum Member
Mar 17, 2007
2,822
0
0
Hey everyone, I have a little bug in a function and I have been examining it and trying to make it work, nothing so far.

The program is supposed to ask the user for an array's length, then ask him to populate it with values, after that a function count(number, length) is called, it's a recursive function. count()'s purpose is to look for repeated entries and output them back the user. Sounds simple but the bug is making it harder than it is.

I would appreciate any help towards fixing this

Working Code,

#include <iostream>
#include <string>

using namespace std;
class project9_3{

public:
//Methods
void count(int number, int length);
void createArray();

//Variables
int *array;
int length_max;
int arrayIndex;
int tempCountMatch;
string junkString;
};

/*
createArrray is a method that initializes the
array's contents and size according to the user's inputs
*/
void project9_3::createArray(){

cout << "Enter the length of the array > ";
cin >> length_max;
getline(cin, junkString);
array = new int[length_max];

for(int i=0;i<length_max;i++){
cout << "Enter value for position " << i << "\n";
cin >> array[ i ];
getline(cin, junkString);
// cout << "array[" << i << "]=" << array[ i ] << "\n";
}

arrayIndex=0;
count(array[0], length_max);
}


void project9_3::count(int number, int length){
// cout << "Entering count\n";
tempCountMatch=0;
for(int i=0;i<length;i++){
// cout <<"number= " << number << " for i= " << i << " for tempCountMatch= " << tempCountMatch << "\n";
if(number==array[ i ]){
tempCountMatch++;
}
if(tempCountMatch==2){
cout << number << " has repeated more than once\n";
break; //make the for loop exit
}
}
if(arrayIndex<length){
arrayIndex++;
// cout << "arrayIndex= " << arrayIndex << "\n";
number=array[arrayIndex];//increment index so we check next number
//cout << "Recursive count call\n";
count(number, length);
}else{
delete [] array;
return;//exits the program
}
}


int main()
{
project9_3 test;
test.createArray();
}

int main()
{
project9_3 test;
test.createArray();
}
 

DarkThinker

Platinum Member
Mar 17, 2007
2,822
0
0
Originally posted by: HigherGround
ever heard of the 'Enter' key?

I pasted the code form VIM and that's how it came out to be, I didn't notice. As soon as I get home I will update the code. Sorry
 

DarkThinker

Platinum Member
Mar 17, 2007
2,822
0
0
Originally posted by: Cooler
Nice to see im not the only coder out there that uses "we" in comments.

:laugh: it's a bad habit., I tend to think of myself having to explain my code to a group of people when I am coding, it's absurd I know.
 

Markbnj

Elite Member <br>Moderator Emeritus
Moderator
Sep 16, 2005
15,682
13
81
www.markbetz.net
Originally posted by: DarkThinker
OK, I have repasted the code in the OP

Ok, now tell us what it's doing that you don't expect. I read it through, and nothing glaringly obvious jumped out except for the fact that you aren't bounds checking the user input. Of course, if it were obvious you'd have found it.
 

HalflifeDivided

Junior Member
Jun 16, 2007
24
0
0
When you allocate memory for "array", you are redefining the variable. So your only really allocating space for the variable "array" inside of createArray()'s scope.

If you wish to allocate memory for the global variable "array", change the line to:

array = new int[length_max];

instead of

int *array = new int[length_max];


also you are leaking memory by not calling an explicit delete on the array after you are done with it.

Good Luck.
 

CTho9305

Elite Member
Jul 26, 2000
9,214
1
81
Your bug is a scoping issue.

int foo = 0;
void Foo() {
int foo = 5;
}

void bar() {
cout << "foo is " << foo << endl;
Foo();
cout << "foo is " << foo << endl;
}
 

Markbnj

Elite Member <br>Moderator Emeritus
Moderator
Sep 16, 2005
15,682
13
81
www.markbetz.net
Originally posted by: HalflifeDivided
When you allocate memory for "array", you are redefining the variable. So your only really allocating space for the variable "array" inside of createArray()'s scope.

If you wish to allocate memory for the global variable "array", change the line to:

array = new int[length_max];

instead of

int *array = new int[length_max];


also you are leaking memory by not calling an explicit delete on the array after you are done with it.

Good Luck.

His array pointer is declared as a member of the class, so I don't think it's a scoping issue.
 

HalflifeDivided

Junior Member
Jun 16, 2007
24
0
0
Originally posted by: Markbnj
Originally posted by: HalflifeDivided
When you allocate memory for "array", you are redefining the variable. So your only really allocating space for the variable "array" inside of createArray()'s scope.

If you wish to allocate memory for the global variable "array", change the line to:

array = new int[length_max];

instead of

int *array = new int[length_max];


also you are leaking memory by not calling an explicit delete on the array after you are done with it.

Good Luck.

His array pointer is declared as a member of the class, so I don't think it's a scoping issue.

His code has been changed since we first discussed it. Are you still running into issues OP?

 

dighn

Lifer
Aug 12, 2001
22,820
4
81
Originally posted by: Markbnj
Originally posted by: DarkThinker
OK, I have repasted the code in the OP

Ok, now tell us what it's doing that you don't expect. I read it through, and nothing glaringly obvious jumped out except for the fact that you aren't bounds checking the user input. Of course, if it were obvious you'd have found it.

Yeah really. When asking for help it would be to your advantage to give as much info as possible. If it's a compilation error, give the line that has the problem, and the associated error. If it's a run time error, describe the symptom.

Not trying to give you a hard time but I see this a lot when people ask for programming help. Not giving enough info only reduces the likelihood of you getting proper help.
 

DarkThinker

Platinum Member
Mar 17, 2007
2,822
0
0
Originally posted by: HalflifeDivided
Originally posted by: Markbnj
Originally posted by: HalflifeDivided
When you allocate memory for "array", you are redefining the variable. So your only really allocating space for the variable "array" inside of createArray()'s scope.

If you wish to allocate memory for the global variable "array", change the line to:

array = new int[length_max];

instead of

int *array = new int[length_max];


also you are leaking memory by not calling an explicit delete on the array after you are done with it.

Good Luck.

His array pointer is declared as a member of the class, so I don't think it's a scoping issue.

His code has been changed since we first discussed it. Are you still running into issues OP?

Thanks to you guys the issues have been resolved I have updated the code in the OP and it's all good after that.(eventhough the repeated number ends up getting printed as many times as it was repeated, that's fine for now, it's not an issue ATM).


Originally posted by: dighn
Originally posted by: Markbnj
Originally posted by: DarkThinker
OK, I have repasted the code in the OP

Ok, now tell us what it's doing that you don't expect. I read it through, and nothing glaringly obvious jumped out except for the fact that you aren't bounds checking the user input. Of course, if it were obvious you'd have found it.

Yeah really. When asking for help it would be to your advantage to give as much info as possible. If it's a compilation error, give the line that has the problem, and the associated error. If it's a run time error, describe the symptom.

Not trying to give you a hard time but I see this a lot when people ask for programming help. Not giving enough info only reduces the likelihood of you getting proper help.

Well I am sorry, I just thought it was pretty obvious that I was talking about the recursive function Count throughout my OP .
 

engineereeyore

Platinum Member
Jul 23, 2005
2,070
0
0
Just a comment. In the count() function, are you sure you want this?

if(tempCountMatch==2)

Wouldn't you want:

if(tempCountMatch > 1)

Otherwise, if the user puts in a value 3 times, it doesn't look like your code will pick it up. I could be missing something though.
 

dighn

Lifer
Aug 12, 2001
22,820
4
81
Originally posted by: DarkThinker
Well I am sorry, I just thought it was pretty obvious that I was talking about the recursive function Count throughout my OP .

But you didn't say how it was not working. e.g. did it crash? gave you the wrong count? Information like that can make it a lot quicker for us to diagnose the problem.
 

dighn

Lifer
Aug 12, 2001
22,820
4
81
Originally posted by: engineereeyore
Just a comment. In the count() function, are you sure you want this?

if(tempCountMatch==2)

Wouldn't you want:

if(tempCountMatch > 1)

Otherwise, if the user puts in a value 3 times, it doesn't look like your code will pick it up. I could be missing something though.

the lack of formatting makes it harder to read, but that line is inside the loop so it goes to 2 first.
 

LeetestUnleet

Senior member
Aug 16, 2002
680
0
0
Originally posted by: DarkThinker
Originally posted by: Cooler
Nice to see im not the only coder out there that uses "we" in comments.

:laugh: it's a bad habit., I tend to think of myself having to explain my code to a group of people when I am coding, it's absurd I know.

// We do it too
 

sao123

Lifer
May 27, 2002
12,648
201
106
Originally posted by: DarkThinker
Originally posted by: HalflifeDivided
Originally posted by: Markbnj
Originally posted by: HalflifeDivided
When you allocate memory for "array", you are redefining the variable. So your only really allocating space for the variable "array" inside of createArray()'s scope.

If you wish to allocate memory for the global variable "array", change the line to:

array = new int[length_max];

instead of

int *array = new int[length_max];


also you are leaking memory by not calling an explicit delete on the array after you are done with it.

Good Luck.

His array pointer is declared as a member of the class, so I don't think it's a scoping issue.

His code has been changed since we first discussed it. Are you still running into issues OP?

Thanks to you guys the issues have been resolved I have updated the code in the OP and it's all good after that.(eventhough the repeated number ends up getting printed as many times as it was repeated, that's fine for now, it's not an issue ATM).


The reason why the program is repeating the output is because you are checking the entire array again... (you are doing too much work.)

Watch... Sample array = 1 2 3 2

(1) 2 3 2
1 doesnt have a match, so move on.

1 (2) 3 2
2 has a match, output & move on.

1 2 (3) 2
3 doesnt have a match, so move on.

1 2 3 (2)
2 has a match, output & move on.

reached the max length = end

Generally when processing an array recursively, you only pass on a sub portion of the array. The correct way to perform this is...

(1) 2 3 2
1 doesnt have a match, so move on.

(2) 3 2
2 has a match, output & move on.

(3) 2
3 doesnt have a match, so move on.

(2)
2 doesnt have a match, output & move on.

Array empty = stop



Rather than passing the entire array, perhaps you want to pass on a counter which could indicate which position in the array would be the one to process first on this iteration, and ignore everything before it.
 

Aikouka

Lifer
Nov 27, 2001
30,383
912
126
Out of curiosity, did it have to be done recursively (i.e. for a project focusing on learning recursion)? One of my favorite recursion problems is translating math functions (i.e. basic ones) into recursive functions. Those seem to stump people pretty well, because they think of the basic math functions as... well basic. But doing them recursively makes you simplify them even more.
 
sale-70-410-exam    | Exam-200-125-pdf    | we-sale-70-410-exam    | hot-sale-70-410-exam    | Latest-exam-700-603-Dumps    | Dumps-98-363-exams-date    | Certs-200-125-date    | Dumps-300-075-exams-date    | hot-sale-book-C8010-726-book    | Hot-Sale-200-310-Exam    | Exam-Description-200-310-dumps?    | hot-sale-book-200-125-book    | Latest-Updated-300-209-Exam    | Dumps-210-260-exams-date    | Download-200-125-Exam-PDF    | Exam-Description-300-101-dumps    | Certs-300-101-date    | Hot-Sale-300-075-Exam    | Latest-exam-200-125-Dumps    | Exam-Description-200-125-dumps    | Latest-Updated-300-075-Exam    | hot-sale-book-210-260-book    | Dumps-200-901-exams-date    | Certs-200-901-date    | Latest-exam-1Z0-062-Dumps    | Hot-Sale-1Z0-062-Exam    | Certs-CSSLP-date    | 100%-Pass-70-383-Exams    | Latest-JN0-360-real-exam-questions    | 100%-Pass-4A0-100-Real-Exam-Questions    | Dumps-300-135-exams-date    | Passed-200-105-Tech-Exams    | Latest-Updated-200-310-Exam    | Download-300-070-Exam-PDF    | Hot-Sale-JN0-360-Exam    | 100%-Pass-JN0-360-Exams    | 100%-Pass-JN0-360-Real-Exam-Questions    | Dumps-JN0-360-exams-date    | Exam-Description-1Z0-876-dumps    | Latest-exam-1Z0-876-Dumps    | Dumps-HPE0-Y53-exams-date    | 2017-Latest-HPE0-Y53-Exam    | 100%-Pass-HPE0-Y53-Real-Exam-Questions    | Pass-4A0-100-Exam    | Latest-4A0-100-Questions    | Dumps-98-365-exams-date    | 2017-Latest-98-365-Exam    | 100%-Pass-VCS-254-Exams    | 2017-Latest-VCS-273-Exam    | Dumps-200-355-exams-date    | 2017-Latest-300-320-Exam    | Pass-300-101-Exam    | 100%-Pass-300-115-Exams    |
http://www.portvapes.co.uk/    | http://www.portvapes.co.uk/    |