CODEWITHSUNDEEP
c
user379888
user379888

Reputation:

string to integer

I have made a program which converts numbers entered into a string into an integer like atoi does, but its giving wrong output.

#include<stdio.h>
#include<conio.h>
#include<math.h>
#include<string.h>
void main(void)
{
 static int sum;
 int i,x,y,z;
 char string[10];
 printf("Enter a string:\n");
 gets(string);
 x=strlen(string);
 for(i=0; ;i++)
 {
  if(string[i]=='\0')
  {
   break;
  }
  y=pow(10,i);
  z=string[x-i+1]*y;
  sum+=z;
 }
 printf("%d",sum);
 getch();
}

Upvotes: 3

Views: 707

Answers (3)

Michael Aaron Safyan
Michael Aaron Safyan

Reputation: 95639

Ok. Here is a quick review of your code. Comments embedded.

#include<stdio.h>

Leave a space between #include and <stdio.h>.

#include<conio.h>

This is a non-standard Windows-only header that you don't need. Don't include this.

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

Again use a space, when including your headers.

void main(void)

While this is legal, it is more common to find the signature int main(int argc, char* argv[]) as the signature for the main function. I would suggest that you use that signature.

 {
     static int sum;

Why are you making this static? Are you planning to invoke main repeatedly and have the previous result for sum persist from one invocation of main to another? If not, then don't make it static.

 int i,x,y,z;
 char string[10];

Consider allocating more space for your string. Ten characters is quite small. Also consider creating a variable to represent the size of your string, rather than using a magic number, since you will likely have to reference the buffer size in multiple places.

printf("Enter a string:\n");
gets(string);

No. Don't do that!!! The function gets is a major security vulnerability!. It makes your program susceptible to buffer overflow attacks. Instead, use fgets, and specify the size of the buffer that you want to fill, so that it doesn't overrun your buffer. You should never, ever use plain gets.

x=strlen(string);

Consider choosing a more descriptive name for x. Perhaps len. It is perfectly ok (and good) to create variables that have identifiers longer than a single letter.

for(i=0; ;i++)
{
  if(string[i]=='\0')
  {
     break;
  }

Consider putting the termination condition in the for-loop; for(i = 0; string[i]!='\0'; i++).

  y=pow(10,i);
  z=string[x-i+1]*y;

Hint: there is a smarter way to do this than using pow.

  sum+=z;
 }
 printf("%d",sum);

Ok. The above is fine, although you might want to use "%d\n".

 getch();

You really shouldn't be doing this on all systems. Instead, do:

#ifdef _WIN32
    system("pause");
#endif

If possible, though, I would suggest you avoid that weird pausing behavior. Suppose your professor uses an automated script to validate the output of your program. Putting any sort of pause in the program (even on Windows), will break such a script. If you don't want the terminal window to disappear while on Windows, you should invoke your program from the command prompt.

}

If you were to change the signature to something returning int as I suggested, then you would want to add the statement return 0; before the end of the function.

Upvotes: 5

polygenelubricants
polygenelubricants

Reputation: 384016

Your string do not contain the int values 0, 1, 2, ... 9.

They contain the char values '0', '1', '2', ... '9'. Encoded in e.g. ASCII, '0' == 48.

You need to convert the char to int; one way to do this is by subtracting '0', e.g.:

z = (string[x-i+1] - '0') * y;

Related questions

On Horner's Scheme

You can also do better by not using the pow, by using Horner scheme.

Here's an example (here ^ denotes exponentiation instead of bitwise-xor):

8675309 = 8*10^6 + 6*10^5 + 7*10^4 + 5*10^3 + 3*10^2 + 0*10^1 + 9*10^0
        = (((((8*10 + 6)*10 + 7)*10 + 5)*10 + 3)*10 + 0)*10 + 9

It may look complicated at first, but it really isn't. You basically read the digits left to right, and you multiply your result so far by 10 before adding the next digit.

In table form:

step   result  digit  result*10+digit
   1   init=0      8                8
   2        8      6               86
   3       86      7              867
   4      867      5             8675
   5     8675      3            86753
   6    86753      0           867530
   7   867530      9          8675309=final

I'll leave you to implement this simple algorithm on your own, since this is homework.

See also

Related questions

Upvotes: 5

adamk
adamk

Reputation: 46864

it should be: 

z=(string[x-(i+1)]-'0')*y;

Upvotes: 2

Related Questions