A program with a function that converts a string of digits into an integer value

Question

Here is a programming task:

Write a program with a function that converts a string of digits into an integer value. Do not use the strtol() function or any other standard C library function. Write your own!

I wanted this program to work perfectly with no errors. First I did the power function (power) and then next is function that return the length of a string (lent) and lastly is function that gives the value of asciicode (ascton).

In the main function there is a loop for the string where it multiple the value of how much zeros in has after it. For example, the string is "1100" so the first 1 is 1000 then this value is stored in variable z, and then this works for the other character and add it to variable z. At the end it return z as an integer of the string.

Is this code efficient, or did I use the "long way"? Please give me some hints how I can write better, cleaner and more readable code. Any feedback is appreciated.

#include <stdio.h>
#include <math.h>
int powe(int ex , int po)
{
 int z = 1;
 for (int i = 0; i < po; i++)
 {
 z = z * (10 * 1);
 }
 return z;
}
int lent (char a [20])
{
 int i=0;
 int l = 0;
 for(i=0 ; a[i] != '0円';i++)
 {
 l++;
 }
 return l;
}
int ascton (int a)
{
 int asciicode[10] = {48,49,50,51,52,53,54,55,56,57};
 for(int i = 0; i < 10; i++)
 {
 if (a == asciicode[i]) 
 {a = i; 
 break;}
 else continue;
 }
 return a;
}
int main(void) {
 
 int z = 0;
 char sn[20];
 printf("enter your string number!:");
 scanf("%s",sn);
 
 
 int s = lent(sn);
 
 
 for(int i = 0; sn[i] != '0円';i++)
 {
 int k = 0;
 k = ascton(sn[i]);
 z = z + (k * powe(10, s-1));
 s = s-1;
 
 }
 printf("%d",z);
}
```

• aki: "is this code efficient ?" I suggest using a profiler like Instruments (mac), WPA (win) or Hotspot (linux) and check the bottlenecks once you feel that algorithm is fine.
• user7761803: HINT: You don't need a power function at all
• sig_seg_v: Your use of fixed-width arrays instead of pointers is very problematic. You don't want your function prototype to look like int f(char x[20]); because that function can't accept an arbitrary c-str; it will only accept a fixed-width array. You could prototype a variable-width array, int f(char x[]);, or simply use int f(char *x);, which I feel is preferable because it's standard. There's another way to define your asciicode[] array too: const char *asciicode = "0123456789";. However, my favorite way to go from a single ascii digit to an integer 0-9 is simply int i = a - '0';
• jamesqf: You might also explain why you're doing "z = z * (10 * 1);" Of course any level of code optimization should remove the "10 * 1", but still... Also the "else continue" in your ascton is unnecessary, though as others have pointed out, there are better ways to do the conversion.
• Peter Cordes: @sig_seg_v: it would only be a real correctness problem if the prototype was int f(char x[static 20]) (which would promise that all 20 array elements were definitely readable, even if the C string actually ended after 2 bytes, right before an unmapped page). (reference) As written, the 20 is ignored by C, and is confusing / misleading only for humans. But yes, a sane prototype would be unsigned f(const char *s).

Answer 1

regarding:

scanf("%s",sn);

To avoid any possible buffer overflow, should include a MAX characters modifier that is 1 less than the length of the input buffer ( 1 less because the %s input format conversion specifier always appends a NUL byte to the input.

Should check the returned value to assure the operation was successful. Note: the scanf() family of functions returns the number of successful 'input format conversions'

Suggest:

if ( scanf("%19s",sn) != 1 )
{
 fprintf( stderr, "scanf for the input string failed\n" );
 exit( EXIT_FAILURE );
}

For ease of readability and understanding:

Please consistently indent the code. Indent after each opening brace '{'. Unindent before each closing brace ']'. Suggest each indent level be 4 spaces. 1 or 2 space indents will 'disappear' when using variable width fonts.

Please follow the axiom:

*only one statement per line and (at most) one variable declaration per statement.*

Therefore,

 {a = i; 
 break;}

would be much better written as:

 {
 a = i; 
 break;
 }

regarding:

#include <math.h>

Nothing in that header file is being used by the OPs program. Therefore, that statement should be removed.

if the user enters a value larger than 2147483647 (aka: 2^31 -1) then the int result overflows, resulting in a negative value being printed I.E. results in undefined behavior Suggest 1) limit the user input to 10 characters + the NUL terminator, 2) check for the result becoming negative

regarding the function:

int ascton (int a)

This whole function can be reduced to:

a -= '0';

suggest, before operating on each character the user input, to first check that char is a digit. Suggest:

#include <ctype.h>
....
int main( void )
{
 ...
 if( ! isdigit( sn[i] )
 {
 fprintf( stderr, "input contains characters other than digits\n" );
 return -1;
 }
 ....

the function: lent() can be reduced to:

int lent (char a [20] )
{
 return (int)strlen( a );
}

• pacmaninbw: The spec says that can't use C library functions which leaves isdigit() out.
• user3629249: I must have misunderstood. I thought the question stated that could not use any of the string to integer functions like: strtol() strtoll() atoi(), etc
• chux: Regarding: #include <math.h>., there is an advantage to using it given "Do not use the strtol() function or any other standard C library function.". It helps detect if this code collides with the standard C library. Consider if code here was int pow(int ex , int po). By having #include <math.h>, a quick compile time error would generate. Of course it has the disadvantage too of allowing code to use a banned function.
• Peter Cordes: check for the result becoming negative - signed integer overflow is undefined behaviour in ISO C; you can't safely check for overflow this way unless you compile with gcc -fwrapv or something to define the behaviour as 2's complement wraparound. A common result of the UB is wrapping, but a compiler could optimize away a check if it could prove that both inputs to a multiply were non-negative. Causing UB means nothing at all is guaranteed.

Answer 2

is this code efficient ?

No. Rather than using the unnecessary powe(), lent(),

1. simply scale by 10 each operation; and
2. use - '0'.

.

char sn[20];
//scanf("%s",sn);
if (scanf("%19s", sn) == 1) {
 int z = 0;
 for (int i = 0; sn[i] >= '0' && sn[i] <= '9'; i++) {
 z = z*10 + (sn[i] - '0');
 }
 printf("%d",z);
}

Even better code would detect overflow and maybe use unsigned and/or wider types.

Answer 3

Welcome to code review.

Now my question is, is this code efficient ? or did I use the "long way".

You used the long way.

The solution you provide is not portable to systems that don't use ASCII. Use '0' to '9' instead.

Rather than iterating through each of the ASCII characters do a range check:

 if (a >= '0' && a <= '9')
 {
 return a - '0';
 }

Prefer variable and function names that are longer and more meaningful.

The function scanf() returns a value that is the number of items read, you can use this for error checking input.

A little less vertical spacing would be good. Formatters can also help with keeping consistent code style unlike:

int i=0;
int l = 0;

• jamesqf: "Longer" is not a synonym for "more meaningful".