What is the issue with multiplication in my C++ program?

[peterjaybee]

Hi,

I am having an issue with multiplication in C++. I am writing a program to check the validity of an NHS number. To do this I must;
multiply digit 1 of the NHS number by 10,
multiply digit 2 of the NHS number by 9,
multiply digit 3 of the NHS number by 8 ...etc

The answer from each multiplication is added together to form a total I have called sum.

When I wrote the code, the sum at the end of the process was far too big, so I have written some checks into the program to see at which step the program is going wrong and the only step that seems incorrect is the multiplication itself. This is a bit of a mystery to me.

#include <iostream>
#include <math.h>
#include <string>

using namespace std;

int main() 
{
	string nhs_number;
	
	cout << "Please enter your 10 digit NHS number and press ENTER: " << endl;
	getline(cin, nhs_number);
	
	while (nhs_number.size() != 10 && nhs_number.size() != 0)
	{
		cout << "The format you have entered is incorrect" <<endl;
		cout << "Please re-enter the number" << endl;
		getline(cin, nhs_number);
	}
	
	if (nhs_number.size() == 0) //breaks the program
	return 0;
	
	
	else {
		cout << "The NHS number you have entered is: " << nhs_number << endl;
		int i, n, sum;
		
		i = 0;
		n = 10;
		sum = 0;
		
		// need to set up a for loop to take the ith digit of the NHS number and
		// multiply it by a weighing factor.
		// The weighing factor (n) is different for each digit
		// for digit 0, n=10
		// for digit 1, n=9
		// for digit 2, n=8 etc
		// The final digit (digit 9) is a check digit and is not needed in this 
		// part of the program.
		
		for (i=0; i<9; i++)
		{	
		
		// print to screen values as a check of where the program is going wrong
		
			cout << "Current running total is " << sum << endl;
			cout <<"NHS number element " << i << " is " << nhs_number[i] << endl;
			cout <<"Weighing factor is " << n << endl;
			
			
		// the next line is where the program goes wrong	
		
			cout << "Nhs number digit " << i << " x the weighing factor is "<< (nhs_number[i] * n) << endl;
			
			sum += (nhs_number[i] * n);
			n--;
		}
		
		cout << sum << endl;
		cout << 11 - (sum % 11);
	}
	
	return 0;
}

I have been using different 10 digit numbers as tests. so for example imputing the nhs_number = 1234567890.

 

[peterjaybee]

For example once the program is run for the nhs number 1234567890, I get the following initial output.

Current running total is 0
NHS number element 0 is 1
weighing factor is 10
NHS number digit 0 x the weighing factor is 490!

this should be 1 x 10 = 10

 

[Borek]

ASCII code vs character meaning...

 

[hamster143]

That's because nhs_number is a character, not a number. The character that represents the number 1 is not equal to 1, it's equal to '1' (or, as your code demonstrates, to 0x31).

The easiest way to go is to write sum += (nhs_number-'0') * n;

 

[peterjaybee]

The easiest way to go is to write sum += (nhs_number-'0') * n;

I have never seen this type of code before, but it works! What does the -'0' do?

Many Thanks,

Peter

 

[Borek]

Subtracts the value of '0' - that is, ASCII code for the character.

 

[peterjaybee]

Oh ok. Pretty sneaky way around the problem. I like it!

 

[RedX]

What does the line:

using namespace std;

do? Is std the name of a variable, namespace the type, and using a descriptor for the type?

As in

long int number;

?

 

[Borek]

http://en.wikipedia.org/wiki/Namespace_(computer_science )

using namespace std; is more or less a way of telling compiler "add std:: to all entities within the block".

 

[RedX]

http://en.wikipedia.org/wiki/Namespace_(computer_science )

using namespace std; is more or less a way of telling compiler "add std:: to all entities within the block".

Which entities would you have to add by hand if you didn't tell the compiler that? Also which block? main()?

 

[Hurkyl]

Everything in the C++ standard libraries are in the namespace std, and so requires the scope operator to use, e.g. std::cout. I think technically anything from the C standard library that isn't a macro is also supposed to be there in C++.

Old-fashioned headers like <iostream.h> (the correct header is <iostream>) pull all of the corresponding symbols into the global scope for backwards compatibility. e.g. if you go look at the header file, you'll probably see something like

#include <iostream>
using std::cout;
using std::endl;
// ...

These headers are either deprecated or eliminated from the C++ standard, so their existence is purely an extra feature provided by those who provided you with the C and C++ libraries. I think most systems do the same with the C libraries too, even if you use, say <cstddef> instead of <stddef.h> like you're "supposed" to in C++; e.g. I very rarely find a system where I am required access size_t as std::size_t).

(<stddef.h> is where things like size_t come from. Some other C headers include this, so you don't often need to do it yourself)

 

[RedX]

I searched all over for the header files on my linux system, and all I could find are C header files in /usr/include/, and they all have .h extensions. Actually, I don't believe my distro came with g++ installed, so maybe that's why I can't find iostream or iostream.h

Maybe I'm not understanding it correctly, but don't you always want to have "using namespace std;"? If you include iostream, that means you want to use the functions in iostream. When you type in "using namespace std;", that means you can't name your own function cout, because then the preprocessor will append std:: to it, and you have the wrong function. If you don't include iostream, then you are free to name your function cout.

 

[Hurkyl]

Maybe I'm not understanding it correctly, but don't you always want to have "using namespace std;"? If you include iostream, that means you want to use the functions in iostream. When you type in "using namespace std;", that means you can't name your own function cout

If you wanted to name a function cout, and still be able to use the features in <iostream>, you can! That's why the standard library is put in the std namespace -- so that its symbols will not conflict with anything^* the user wants to use. But the using directive exists so that the user can pull symbols into local scope whenever he wants, for convenience. (or for other purposes)

Incidentally, using is not a preprocessor thing -- it's part of the language, and it's a directive that affects name lookup. e.g. when the compiler searches for cout, it only checks places that are visible from the local scope, such as the global scope. using std::cout; places cout in the current scope. It's almost like declaring ostream &cout = std::cout; except it's better because there is only one cout instead of two.

*: There are certain exceptions. e.g. I believe the user is not guaranteed by the C++ standard to ever be allowed to use a symbol that starts with two underscores.

 

[airborne18]

The problem with this code is that the input type is not validated. A user can enter anything, which is why the issue with the ascii vs numeric.

I think if you focus on the validation of the input, you will solve the input type conversion. Your input range is limited to 10 numbers, so you have very specific user input rules. Why not get each char, validate it as a 0-9 number, and then stop at 10. If the user violates the input rules, prompt again. This way you can fill an array of 10 integers.

It will simplify the code, and make the calculation easier.

You can add a break character, like Q, if the user wants to stop trying to input and exit the program.