Please comment on my program

[Registered User]

I'm a newbie and have attempted to create a program to convert a string
containing the representation of a number in a specified base to a
decimal integer. Here's the code:

#include <stdio.h>

unsigned todecimal( char *s, int base)
{
unsigned num = 0, mul;
int len=0;
while (*++s != '\0')
++len;
--s;
for (mul=1; len>=0; --len, mul*=base, --s)
{
if (*s >= '0' && *s <= '9')
num+= (*s-'0')*mul;
else if (*s>='A'&& *s<='Z')
num+= (*s-'A'+10)*mul;
else if (*s>='a' && *s<='z')
num+=(*s-'a'+10)*mul;
}
return num;
}

int main()
{
char s[33];
int base;
printf("Enter the number and its base: ");
scanf("%s%d",s, &base);
printf("Decimal value = %u\n", todecimal(s, base) );
return 0;
}

[CBFalconer]

Registered User wrote:

>
I'm a newbie and have attempted to create a program to convert a
string containing the representation of a number in a specified
base to a decimal integer. Here's the code:

#include <stdio.h>

unsigned todecimal( char *s, int base)
{
unsigned num = 0, mul;
int len=0;
while (*++s != '\0')
++len;
--s;
for (mul=1; len>=0; --len, mul*=base, --s)
{
if (*s >= '0' && *s <= '9')
num+= (*s-'0')*mul;
else if (*s>='A'&& *s<='Z')
num+= (*s-'A'+10)*mul;
else if (*s>='a' && *s<='z')
num+=(*s-'a'+10)*mul;
}
return num;
}

int main()
{
char s[33];
int base;
printf("Enter the number and its base: ");
scanf("%s%d",s, &base);
printf("Decimal value = %u\n", todecimal(s, base) );
return 0;
}

Shows promise, but here are some things to consider:

There is no guarantee that the letters 'A'..'Z' are contiguous.
Same for 'a'..'z'. See EBCDIC encoding for one example.

Nothing stops your conversion if an input 'digit' is beyond the
size allowed by base.

Nothing detects overflow.

--
Some informative links:
<news:news.announce.newusers
<http://www.geocities.com/nnqweb/>
<http://www.catb.org/~esr/faqs/smart-questions.html>
<http://www.caliburn.nl/topposting.html>
<http://www.netmeister.org/news/learn2quote.html>
<http://cfaj.freeshell.org/google/>

[Michael Mair]

Registered User schrieb:

I'm a newbie and have attempted to create a program to convert a string
containing the representation of a number in a specified base to a
decimal integer. Here's the code:

#include <stdio.h>

unsigned todecimal( char *s, int base)

"to" followed by a lower case letter belongs as an identifier
to the implementation; i.e. this name might at some time clash
with something the standard library provides.

You do not modify the passed string, so you may as well pass a
const char *.

Something to document early: You check neither s!=NULL nor
2<=base<=36. If the user does not find this, you may get
invalid input and the ensuing crash or returned garbage is
somehow _your_ fault...

{
unsigned num = 0, mul;

Note: I'd rather use unsigned long which is the widest unsigned
integer type in C89 (or unsigned long long for C99).

int len=0;
while (*++s != '\0')

If s is "", then you run off wildly through storage that does
not necessarily belong to you. In addition, you modify a
parameter -- it may be a better idea to create a copy and modify
that.
Either stick with strlen() or amend this to
const char *tmp;
for (tmp = s; *tmp != '\0'; ++tmp) {
++len;
}

++len;
--s;
for (mul=1; len>=0; --len, mul*=base, --s)
{
if (*s >= '0' && *s <= '9')
num+= (*s-'0')*mul;
else if (*s>='A'&& *s<='Z')
num+= (*s-'A'+10)*mul;
else if (*s>='a' && *s<='z')
num+=(*s-'a'+10)*mul;

This breaks if 'Z'-'A' != 25...
A better method is using a digits array:
char digits[] = "0123456789abcdefghijklmnopqrstuvwxyz";
and set the allowed range:
digits[base] = '\0';
and then find the digit using strchr()
char *occurred = strchr(digits, tolower(*s));
if (occurred && *occurred) {
int digit = occurred - s;
....
The other thing is that you do not make sure that you have
no overflow: For num of type unsigned int, this means:
unsigned int add;

if (UINT_MAX / digit < mul) {
/* mul will become too large in the next step and is
* already too large for digit*mul to be representable */
/* Your error handling here */
}
add = digit * mul;
if (UINT_MAX - add < num) {
/* mul will become too large in the next step and is
* already too large for num+digit*mul to be representable */
/* Your error handling here */
}

}
return num;
}

You omitted error handling; there are several types of errors, so
you should decide how you want to flag "everything okay" vs. "something
bad happened".
Bad things that could happen:
s == NULL
base < 2
base strlen(digits)
one of the characters not in digits (where digits[base] == 0)
overflow
Should everything be flagged differently?
How do you want to flag this?
As you have unsigned int values, the obvious candidate for an error
return is UINT_MAX. Alternatively, you can have a look at the
strtoul() function and how it flags "bad" values.

int main()

int main (void)
-- we have that much time.

{
char s[33];
int base;
printf("Enter the number and its base: ");
scanf("%s%d",s, &base);

Always make sure that you did get what you expected when using
*scanf():
if (2 != scanf("%s%d", s, &base)) {
/* Your error handling here */
}

printf("Decimal value = %u\n", todecimal(s, base) );
return 0;
}

Note: As you have an excellent alternate version (strtoul()), you
can test your function in comparison with strtoul(). Maybe you have
to wrap strtoul() to get the same outward behaviour.
When testing, do not forget corner cases such as:
s = NULL, s = ""
base = -1, base = 0, base = 1, base = 2, base = 36, base = 37
s = "/", s = "-1", s = "3" for base = 2, s = "0",
s = string holding UINT_MAX, s = string holding UINT_MAX + 1, ....
Cheers

--
E-Mail: Mine is an /at/ gmx /dot/ de address.

[Frederick Gotham]

Registered User posted:

#include <stdio.h>

unsigned todecimal( char *s, int base)

Take that argument as a pointer to const. I would also suggest making the
actual arguments themselves const.

{
unsigned num = 0, mul;
int len=0;

I would advocate the use of an unsigned integer type for "len".

>
while (*++s != '\0')
++len;
--s;

Highly inefficient on most systems; you'd probably be better off with
something like:

size_t len = strlen(s);

s += len-1;

for (mul=1; len>=0; --len, mul*=base, --s)

If using an unsigned type, change the conditional to: len != -1

{
if (*s >= '0' && *s <= '9')
num+= (*s-'0')*mul;
else if (*s>='A'&& *s<='Z')
num+= (*s-'A'+10)*mul;
else if (*s>='a' && *s<='z')
num+=(*s-'a'+10)*mul;

As mentioned elsethread, the codes for the letters of the alphabet aren't
necessarily in order one after the other.

--

Frederick Gotham

[jaysome]

On Mon, 25 Sep 2006 10:42:47 -0400, CBFalconer <cb********@yahoo.com>
wrote:

>Registered User wrote:

>>
I'm a newbie and have attempted to create a program to convert a
string containing the representation of a number in a specified
base to a decimal integer. Here's the code:

#include <stdio.h>

unsigned todecimal( char *s, int base)
{
unsigned num = 0, mul;
int len=0;
while (*++s != '\0')
++len;
--s;
for (mul=1; len>=0; --len, mul*=base, --s)
{
if (*s >= '0' && *s <= '9')
num+= (*s-'0')*mul;
else if (*s>='A'&& *s<='Z')
num+= (*s-'A'+10)*mul;
else if (*s>='a' && *s<='z')
num+=(*s-'a'+10)*mul;
}
return num;
}

int main()
{
char s[33];
int base;
printf("Enter the number and its base: ");
scanf("%s%d",s, &base);
printf("Decimal value = %u\n", todecimal(s, base) );
return 0;
}

Shows promise, but here are some things to consider:

There is no guarantee that the letters 'A'..'Z' are contiguous.
Same for 'a'..'z'. See EBCDIC encoding for one example.

While it's important to understand that the C Standard does not
guarantee that the character set used to encode 'A'..'Z' or 'a'..'z'
is not contiguous, it's also important to realize that source code
distributed with a particular character encoding is inherently
non-portable. ZIP files containing source code encoded with the ASCII
character encoding are inherently non-portable. True pedantics would
note such a caveat.

Best regards
--
jay

[dcorbit]

[snip]
See snippet functions bascnvrt.c and ltostr.c:
http://c.snippets.org/browser.php