Learn to Code
John F. Dumas
contact me | resume | how it works | example programs | testimonials | main page
Help Example: Four
one | two | three | four | five
► QUESTION: The program I'm working on is designed to find out which number 'N' (less than 10,000) has the largest number of unique divisors (where the divisors are greater than 1 and less than N). So (for example) 6 would have two unique divisors (2 and 3) but 4 would only have one (2).
Here's my code (which looks logically to be correct) but it does not compile. When I try to build it, the compiler complains that 'count' is an ambiguous symbol, can you help me figure out what is going wrong?
#include <iostream>
using namespace std;
int count = 0;
void getDivisorCount(int value)
{
count = 0;
for(int i = 2; i < value; i ++)
if(value % i == 0)
count++;
}
int main()
{
int maxDivisors = 0;
int theNumber = 0;
for(int i = 0; i < 10000; i ++)
{
getDivisorCount(i);
if(count > maxDivisors)
{
maxDivisors = count;
theNumber = i;
}
}
cout <<
"The number: " << theNumber << " has the maximum number " <<
"of divisors: " << maxDivisors << "\n";
return(0);
}
► ANSWER: Sure, there are a number of things about this program that need to be addressed but let's start with the biggest one:
using namespace std;
In C++, if a function or variable is in a different namespace, you typically have to prefix it with the name of that namespace to access it. So, if the function 'getValues' were part of the 'util' namespace, you'd invoke it this way:
util::getValues(42);
The 'std::' prefix says "This function is not in the global namespace but I'm telling you where to find it" (the 'std' namespace). When you do:
using namespace std;
You are telling the compiler "I can now use anything in the 'std' namespace without having to prefix 'std::' before it". In a small program or teaching environment, this might be fine but it has serious consequences in other situations.
Imagine you're addressing a group of 100 people and several of the people are named "Dave". Normally, you'd just say: "Dave Johnson, please come here" or "Dave Smith, I need some feedback" but what if you said "Let's all be on a first name basis". Because there are 100 people, there are likely to be many repeated first names (like Dave) so when you say: "Dave, are you here?" any number of people might think that question applies to them.
Ok, inside namespace std, it turns out there is already a symbol named 'count'. It is used to determine how many matching elements are found a sequence of elements. So, when you do:
using namespace std;
And then do:
int count = 0;
You are causing the compiler to not know which 'count' symbol you're actually referring to. In short, the group of people has more than one "Dave" in it.
Let's first address this in a quick-and-dirty way by changing the symbol 'count' in your program to 'kount'. Here's that code:
#include <iostream>
using namespace std;
int kount = 0;
void getDivisorCount(int value)
{
kount = 0;
for(int i = 2; i < value; i ++)
if(value % i == 0)
kount++;
}
int main()
{
int maxDivisors = 0;
int theNumber = 0;
for(int i = 0; i < 10000; i ++)
{
getDivisorCount(i);
if(kount > maxDivisors)
{
maxDivisors = kount;
theNumber = i;
}
}
cout <<
"The number: " << theNumber << " has the maximum number " <<
"of divisors: " << maxDivisors << "\n";
return(0);
}
This change allows the code to compile and run and we see:
The number: 7560 has the maximum number of divisors: 62
The problem with this approach is that it's hard to choose a symbol name that 100% will not clash with any existing names. Instead of bringing in the entire 'std' namespace, what if we just used 'std::' for the few items that needed it? If we did that, we can change 'kount' back to 'count' and the resulting code looks like:
#include <iostream>
int count = 0;
void getDivisorCount(int value)
{
count = 0;
for(int i = 2; i < value; i ++)
if(value % i == 0)
count++;
}
int main()
{
int maxDivisors = 0;
int theNumber = 0;
for(int i = 0; i < 10000; i ++)
{
getDivisorCount(i);
if(count > maxDivisors)
{
maxDivisors = count;
theNumber = i;
}
}
std::cout <<
"The number: " << theNumber << " has the maximum number " <<
"of divisors: " << maxDivisors << "\n";
return(0);
}
Note that the only place we needed to add 'std::' was in 'std::cout'. Now, our 'count' variable no longer clashes because the 'count' function (in the std namespace) has not been added to the global namespace.
Certainly, the above is an improvement (and it gets the right answer) but there's one more change we should make. It starts with asking the question: "Why does count need to be a global variable?". The answer is: "It doesn't". Here's the code without the global variable:
#include <iostream>
static int getDivisorCount(int value)
{
int count = 0;
for(int i = 2; i < value; i ++)
if(value % i == 0)
count++;
return(count);
}
int main()
{
int maxDivisors = 0;
int theNumber = 0;
for(int i = 0; i < 10000; i ++)
{
int count = getDivisorCount(i);
if(count > maxDivisors)
{
maxDivisors = count;
theNumber = i;
}
}
std::cout <<
"The number: " << theNumber << " has the maximum number " <<
"of divisors: " << maxDivisors << "\n";
return(0);
}
Note that before, the 'count' variable needed to be used immediately after calling 'getDivisorCount' since otherwise, some subsequent call might overwrite the original 'count' value. By removing the global variable we remove this restriction and also allow our code to be thread safe.
As a general rule, your variables should always be as local as they possibly can be and in this case, the global variable was entirely unnecessary and could be removed.