Thread: Adding a Large number digit by digit

Code:

int main(int argc, char* argv[])
{
unsigned char number1 [256] = {0};
unsigned char number2 [256] = {0};
unsigned char result [256] = {0};
int i, y=0, x=0, sum=0, carry=0;

...

memset (result, 0, sizeof(result));

for(i=255; i>=0; i--) {
	sum += carry;
	for(y=255; y>=0; y--) {
		sum += number1[i]+number2[y]-48;
	}
...
}

You don't need to use memset to initialise an array, use curly brackets (braces) to it when you declare the variable.
Also, you're not converting the digits to numbers before you add them and more importantly, why are you subtracting 48??

Code:

sum += atoi(number1[i]) + atoi(number2[y]);

Also, why do you have two loops? Sure you only need one. If I'm not mistaken, your code will add all the digits in number2 to *every* digit in number1

QuantumPete

Code:

	for(i=0; i<=256; i++) {
			number1 [i];
	}
	for(y=0; y<=256; y++) {
			number2 [y];
	}

What's that supposed to do? I'd guess that the compiler would optimise that code out of existence, which is fortunate since it might otherwise crash due to buffer overrun.

the remainder is getting stored as a weird symbol, not even a number

result is stored as number 0,1,2 ...
and you are trying to print the array of numbers as string...
to do it you should store result as characters '0', '1', ... and null-terminate it

to convert digit sum%10 into char you should add the '0'

You don't need the single quotes in the initialiser, just zero will suffice. Similarly you don't need memset, the result variable is already initialised to zero.

And, BY GOD, take out those goto statements! You want the 'continue' keyword to terminate the current loop iteration and start the loop at the next one. In which case you can also take out the y-- int the two if statements.

QuantumPete

> And, BY GOD, take out those goto statements!
Quite so.
Even the continue is unnecessary since only one branch of the if/else if chain is ever executed, you may as well just let the program flow the normal way.

In addition, the goto introduces a BUG in that it avoids the loop comparison step to terminate the loop normally. Each y-- should be followed by a y>=0 test, and the goto avoids this.

> memset (result, 0, sizeof(result));
So why didn't you use 0 rather than '0' in the initialisation of result to begin with?

> CompE271HW2.cpp
You're compiling C code with a C++ compiler. This will lead you astray at some point.
You can easily do without that stdafx header file as well. Just go to project settings and turn OFF the precompiled headers, you don't need them.

[QUOTE=Salem;671051Even the continue is unnecessary since only one branch of the if/else if chain is ever executed, you may as well just let the program flow the normal way.
[/QUOTE]

In my blind panic at seeing a goto statement in C code, I completely overlooked that
This is probably what you want:

Code:

for (y = 255; y >= 0; --y) {
    if ((number1[y] == 0) || (number2[y] == 0)) {
        continue;
    }
    sum += carry;
    sum += number1[y] - '0' + number2[y] - '0';
    carry = sum / 10;
    result[y] = sum % 10 + '0';
    sum = 0;
}

Since you want to terminate the loop iteration if either digit is 0, you may as well use an OR statement. I've also introduced some whitespacing, which makes it nicer to look at. Also when incrementing/decrementing variables with ++/-- and not assigning them to anything, use the prefix operators, as it's cheaper and has the same effect.

QuantumPete

Or just invert the test for even more simplicity, and perhaps a bit more obvious as to what it's about

Code:

if ( (number1[y] != 0) && (number2[y] != 0) ) {
    sum += carry;
    sum += number1[y] - '0' + number2[y] - '0';
    carry = sum / 10;
    result[y] = sum % 10 + '0';
    sum = 0;
}

Originally Posted by QuantumPete

Also when incrementing/decrementing variables with ++/-- and not assigning them to anything, use the prefix operators, as it's cheaper and has the same effect.

Sorry for side-tracking and being pedantic, but: is there any evidence of that?

If there's no assignment or other use of the variable in the same statement, I can't see how it would make any difference. In x86 the code should still be "inc <something>" or "dec <something>" [unless it's a pointer, in which case the inc/dec is replaced by add/sub with a constant of sizeof(*pointer)].

In some cases, using the right way around of *p++ or *--p would perhaps cause better code on for example PDP-11, VAX or 68K, as those processors have "mov rA,(rB)+" and "mov rA,-(rB)", where the register rB is incremented/decremened automatically "at the same time as the access to memory" - but that would be specific to certain architectures, and definitely doesn't apply to the above case you described.

--
Mats

As to the "unequal length", just pad the number with zeros (at the beginning) until the shorter one is equal in length with the longer one. This process is called "normalization".

I'd also set sum = 0 at the beginning of the for-loop, rather than at the end [inside the if-statement as per Salem's suggestion] - it just makes it clearer. You can then (if you want) remove the sum = 0 in the beginning of the code - or even move int sum = 0 inside the braces that enclose the code to add things.

If you know the length of the string [which you will do after you've normalize the numbers] you can use the length as a start position rather than 255.

If you add a further zero at the beginning of each of the strings, you don't have to worry about the carry on the last add. Just remove the zero (if you want to) when you're finished, assuming it's still zero (no carry over). Alternatively, check carry after the calculation loop and insert it ahead of the rest of the digits.

--
Mats

If there's no assignment or other use of the variable in the same statement, I can't see how it would make any difference.

I agree. The difference would come in C++, where an overloaded postfix operator++ implementation would typically involve a temporary but an overloaded prefix operator++ implementation would operate on *this directly. For primitive types, and thus for C in general, there should be no difference.