2
\$\begingroup\$

I tried my level best to get it done. But I would like to have a better solution in all terms.

Given 2 huge numbers as seperate digits, store them in array and process them and calculate the sum of 2 numbers and store the result in an array and print the sum.

Input:

Number of digits:12

9 2 8 1 3 5 6 7 3 1 1 6
Number of digits:9
7 8 4 6 2 1 9 9 7

Output:

9 2 8 9 2 0 2 9 5 1 1 3
    #include<stdio.h>
    #include<stdlib.h>
    int main()
    {
        int num1[]={9,9,9,9,9,9,9,9,9};
        int num2[]={1,1,1};
        int num1_n=sizeof(num1)/sizeof(int);
        int num2_n=sizeof(num2)/sizeof(int);
        int z=num1_n+num2_n;

        int carry=0;
        int digi=0;
        int i=0;
        int j=0;
        int *num3;
        if(num1_n>num2_n)
        {
            num3=(int*)malloc(sizeof(int)*(num1_n+1));
            for(i=0;i<num1_n+1;i++)
            {
                num3[i]=0;
            }
            for(i=num1_n-1,j=num2_n-1;i>=0 || j>=0 ;i--)
            {
                num3[i+1]=num3[i+1]+num1[i]+num2[j];
                printf("i %d j %d num3[i] %d num1[i] %d num2[i] %d\n",i,j,num3[i],num1[i],num2[j]);
                if(num3[i+1]>9)
                {
                    carry=1;
                    num3[i+1]=num3[i+1]%10;
                }
                if(carry==1)
                {
                    num3[i]=1;
                    carry=0;
                }
                if(j>=0)
                {
                    j--;
                }
            }
        }
        for(i=0;i<num1_n+1;i++)
        {
            printf("%d ",num3[i]);
        }
        return 0;
    }
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Your description seems to suggest that your code isn't currently working as expected. Does it currently work? \$\endgroup\$
    – forsvarir
    Commented Dec 18, 2016 at 12:47
  • \$\begingroup\$ Ya it is working ! \$\endgroup\$
    – Bharad Waj
    Commented Dec 18, 2016 at 14:37
  • \$\begingroup\$ Code fails when if(num1_n>num2_n) is false, printf("%d ",num3[i]); attempts to access uninitialized num3. \$\endgroup\$
    – chux
    Commented Dec 19, 2016 at 18:27

2 Answers 2

3
\$\begingroup\$

Define an API

The first thing you should do is define a function to wrap the implementation. This gives it a name and a defined interface. You can then set expectations such as the first number is the largest, the caller needs to pre-allocate a buffer for the result, etc.

Naming

It would be easier to follow your code if i and j had more meaningful names. num3 could be target_number or sum etc.

Unused variables

You define variables z and digi, but don't use them. If you don't need them, get rid of them, they simply add unnecessary noise.

carry?

Do you really need your carry flag? You could simply combine the code that sets it with the code that checks it:

if(num3[i+1]>9)
{
    num3[i+1]=num3[i+1]%10;
    num3[i]=1;
}

Bugs

Your code has two major issues.

  • As pointed out by @chux in the comments, if num1 has the same number or more digits than num2 your code skips the addition step and then accesses num3 to print it out. This will probably crash because num3 hasn't been allocated.
  • If num1 has more digits than num2, you access a negative index of num2. Once you pass the left side of the shortest number you should stop adding from it.
\$\endgroup\$
2
\$\begingroup\$

I have reviewed your code and here is what I've found.

Break up the code into smaller functions

Rather than having everything in one long main function, it would be easier to read and maintain if each discrete step were its own function. For example there could be separate add and print functions.

Eliminate unused variables

Unused variables are a sign of poor quality code, and you don't want to write poor quality code. In this code, z and digi are unused. Your compiler is smart enough to tell you about this if you ask it nicely.

Free memory that you allocate

This program leaks memory because you have allocated space using malloc but never released with free. That's bad practice.

Add in place

One way to make this both neater and probably faster would be to duplicate the larger number and add the shorter one to it. This would allow you to easily implement the following suggestion.

Separate I/O from program logic

Right the printing is done within the logic for addition. It's often better design to separate the two so that the program logic is independent of the I/O with the user.

Take advantage of shortcuts

During addition, if you've already used each of the digits of the shorter number, the program is done adding when the carry bit is clear.

Fix the bug

The code takes care to make sure that num1_n > num2_n, but then prints the wrong result if that test fails. Better would be to swap the operands to make sure that the code always knows which is the largest input.

Use more whitespace to enhance readability of the code

Instead of crowding things together like this:

num3=(int*)malloc(sizeof(int)*(num1_n+1));

most people find it more easily readable if you use more space:

num3 = (int *)malloc(sizeof(int) * (num1_n + 1));

Think of the user

Rather than required the user to recompile the code to add two different numbers, it would be nice to read them dynamically during runtime. This would be a useful feature, but would require more code and careful "input sanitation" (that is, looking for and rejecting invalid inputs.)

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

\$\endgroup\$
4
  • \$\begingroup\$ To return 0; or not return 0;, that is the question. unfortunate;y gets too much comment time, including this comment. The best answer is for OP to code "Addition of two number using array of unequal size" as a stand-alone function, with test code driven (in)directly from main(). No need to even review main() in that case. \$\endgroup\$
    – chux
    Commented Dec 22, 2016 at 22:20
  • 1
    \$\begingroup\$ @chux Agreed on all counts. \$\endgroup\$
    – Edward
    Commented Dec 22, 2016 at 22:21
  • \$\begingroup\$ The cast of malloc() is generally considered a Bad Thing, as is the need to keep the argument to sizeof consistent - num3 = malloc((sizeof *num3) * (num1_n + 1)) is safer and clearer (the parens around the sizeof expression are just there for clarity). \$\endgroup\$ Commented Aug 9, 2017 at 16:23
  • \$\begingroup\$ @TobySpeight Yes, good point. \$\endgroup\$
    – Edward
    Commented Aug 9, 2017 at 17:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.