A simple base-converter program

[sugaray]

hi, i wrote a simple base-conveter utility for practice, it seems
works fine right now, hope any gurus out there can give me some
suggestions to, critics of, optimisations on this program or my
programming practice. the code are mainly written in C, but need to be
compiled using a C++ compiler because of the using of builtin Boolean
type. it has been tested under VC7 and GCC 3.3.1. thanx in advance.

/////////////////////// code start /////////////////////
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

typedef enum {DEC,OCT,HEX,UNKNOWN} BASE; // test for the base of the
user input

static char *TrimLeadingZero(char *s) {

char *tmp=s;
while(*tmp=='0') tmp++;
return tmp;
}

static bool isHexChar(char c) {

return ((c>='a' && c<='f')||(c>='A' && c<='F')); // whether a valid
hex char
}

static bool isHexString(char *s) {

char *tmp=s;
size_t l,i;

if((l=strlen(s))==0) // expect no empty strings
return false;

for(i=0;i<l;++i)
if(!isHexChar(tmp[i]) && !isdigit(tmp[i])) // neither a valid hex
char nor a digit
return false;

return true;
}

static int Hex2Dec(char hex) {

if(isupper(hex))
return hex-'7'; // upper hex chars to integers
else if(islower(hex))
return hex-'W'; // lower hex chars to integers
else return hex-'0'; // digit chars to their corresponding integers
}

static int HexString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

int i,t;
int n=0;
size_t l=strlen(tmp);

for(i=l-1;i>=0;--i) {
t=Hex2Dec(*tmp);
if(t==0) {
tmp++;
continue; // jump out if is 0
}
n+=t*(int)pow(16,i);
++tmp;
}

return n;
}

static bool isOctChar(char c) {

return (c>='0' && c<='7'); // whether a valid octal char
}

static int Oct2Dec(char c) {

return c-'0'; // octal to it's corresponding decimal
integer
}

static bool isOctString(char *s) {

char *tmp=s;

if(strlen(s)==0)
return false;

while(*tmp!='\0') {
if(!isOctChar(*tmp))
return false;
++tmp;
}

return true;
}

static int OctString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

size_t l=strlen(tmp);
int i,t;
int n=0;

for(i=l-1;i>=0;--i) {
t=Oct2Dec(*tmp);
if(t==0) {
++tmp;
continue;
}
n+=t*(int)pow(8,i);
tmp++;
}
return n;
}

static BASE Base(char *nstr) {

char input[64];
strcpy(input,nstr);

if(input[0]=='0') // case for hex or octal
if(input[1]=='x' || input[1]=='X') // hex
return HEX;
else return OCT; // octal
else if(isdigit(input[0]) && input[0]!='0') // decimal
return DEC;
else return UNKNOWN; // unknown base
}

static void MakeAFuss(void) {

fprintf(stderr,"Hey pal, what the hell's that ?\n");
}

int main(int argc,char **argv)
{
BASE base;
char tmp[64]={0};
int integer;

if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

base=Base((char *)argv[1]);

switch(base) {
case DEC:
printf("%#x\t%#o\n",atol(argv[1]),atol(argv[1]));
break;
case OCT:
strcpy(tmp,&argv[1][1]); // strip the oct-base header '0'
if(isOctString(tmp)) {
integer=OctString2Int(tmp);
printf("%u\t%#x\n",integer,integer);
}
else MakeAFuss();
break;
case HEX:
strcpy(tmp,&argv[1][2]); // strip the hex-base header '0x' or '0X'
if(isHexString(tmp)) {
integer=HexString2Int(tmp);
printf("%u\t%#o",integer,integer);
}
else MakeAFuss();
break;
default:
MakeAFuss();
}

return 0;
}
////////////////////// code end /////////////////////////

[Allan Bruce]

Just a very quick glance of your code brings up 2 questions to me. See
below

<snip>

static char *TrimLeadingZero(char *s) {
why are you using static functions?

<snip>
if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

you should not 'return 1' as this is not portable, you should instead
include <stderr.h> and use EXIT(EXIT_FAILURE);
Allan

[Mark A. Odell]

"Allan Bruce" <al*****@TAKEAWAYf2s.com> wrote in
news:bu**********@news.freedom2surf.net:

Just a very quick glance of your code brings up 2 questions to me. See
below

<snip>

static char *TrimLeadingZero(char *s) {
why are you using static functions?

Why wouldn't you? Always maintain the tightest scope possible. For this
program the OP does not need to export any function to any other module.
Thus, all functions, except main, should be static. Export *only* what is
required by other modules.
<snip>

if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

you should not 'return 1' as this is not portable, you should instead
include <stderr.h> and use EXIT(EXIT_FAILURE);

You stdlib.h where EXIT_FAILURE is defined (there is no stderr.h in ISO
C).

http://www.acm.uiuc.edu/webmonkeys/b...uide/2.13.html

--
- Mark ->
--

[Richard Bos]

"Allan Bruce" <al*****@TAKEAWAYf2s.com> wrote:

Just a very quick glance of your code brings up 2 questions to me. See
below

static char *TrimLeadingZero(char *s) {

why are you using static functions?

Because it's wiser? I never bother to do this myself, but I'd prefer it
if internal linkage were the default.

if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

you should not 'return 1' as this is not portable, you should instead
include <stderr.h> and use EXIT(EXIT_FAILURE);

ITYM <stdlib.h> and exit(EXIT_FAILURE)? There's no such thing as
<stderr.h> or EXIT(), but EXIT_FAILURE and exit() are in <stdlib.h>.
BTW, you can return EXIT_FAILURE from main() just as well.

Richard

[Allan Bruce]

> ITYM <stdlib.h> and exit(EXIT_FAILURE)? There's no such thing as

<stderr.h> or EXIT(), but EXIT_FAILURE and exit() are in <stdlib.h>.
BTW, you can return EXIT_FAILURE from main() just as well.

Richard

Oh dear I am having an off day today!
Allan

[Jack Klein]

On 21 Jan 2004 04:42:49 -0800, ru****@sohu.com (sugaray) wrote in
comp.lang.c:

[snip]

/////////////////////// code start /////////////////////
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

typedef enum {DEC,OCT,HEX,UNKNOWN} BASE; // test for the base of the
user input

static char *TrimLeadingZero(char *s) {

char *tmp=s;
while(*tmp=='0') tmp++;
return tmp;
}

static bool isHexChar(char c) {

return ((c>='a' && c<='f')||(c>='A' && c<='F')); // whether a valid
hex char
}
Apparently you are posting C++ code to comp.lang.c. Don't do that.
There is no such thing as "bool" in C unless you have a conforming C99
implementation, which I have been informed Visual C++ 7 is NOT, and
even then it requires including <stdbool.h>, which you have not.

Also this function can be performed much better by combining two
standard functions from <ctype.h>:

return isxdigit(c) && !isdigit(c);
static bool isHexString(char *s) {

Don't post C++ code to comp.lang.c.

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

[Barry Schwarz]

On 21 Jan 2004 04:42:49 -0800, ru****@sohu.com (sugaray) wrote:

hi, i wrote a simple base-conveter utility for practice, it seems
works fine right now, hope any gurus out there can give me some
suggestions to, critics of, optimisations on this program or my
programming practice. the code are mainly written in C, but need to be
compiled using a C++ compiler because of the using of builtin Boolean
type. it has been tested under VC7 and GCC 3.3.1. thanx in advance.

/////////////////////// code start /////////////////////
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

typedef enum {DEC,OCT,HEX,UNKNOWN} BASE; // test for the base of the
user input

static char *TrimLeadingZero(char *s) {

char *tmp=s;
while(*tmp=='0') tmp++;
return tmp;
}

static bool isHexChar(char c) {
Is there some reason you don't like the standard function isxdigit()?

return ((c>='a' && c<='f')||(c>='A' && c<='F')); // whether a valid
hex char
There is no requirement in the C standards for a through f and/or A
through F to be sequential. It would be legal for the character set
to have the character + between a and b.
}

static bool isHexString(char *s) {

char *tmp=s;
size_t l,i;

if((l=strlen(s))==0) // expect no empty strings
return false;

for(i=0;i<l;++i)
if(!isHexChar(tmp[i]) && !isdigit(tmp[i])) // neither a valid hex
char nor a digit
return false;

return true;
}

static int Hex2Dec(char hex) {

if(isupper(hex))
return hex-'7'; // upper hex chars to integers
This only works for ASCII characters. It doesn't even come close for
EBCDIC.
else if(islower(hex))
return hex-'W'; // lower hex chars to integers
else return hex-'0'; // digit chars to their corresponding integers
}

static int HexString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

int i,t;
int n=0;
size_t l=strlen(tmp);

for(i=l-1;i>=0;--i) {
If you put the tmp++ in the third clause of the for statement, you can
get rid of both increment statements below.
t=Hex2Dec(*tmp);
if(t==0) {
tmp++;
continue; // jump out if is 0
}
n+=t*(int)pow(16,i);
++tmp;
}
You could get rid of the call to pow completely (it has to be
expensive in terms of time) by evaluating the hex string from left to
right. Something like
for (s = TrinLeadingZero(s); *s ; s++)
n = n*16 + Hex2Dec(*s);
which also allow you to get rid of i, t, tmp, and l.

return n;
}

static bool isOctChar(char c) {

return (c>='0' && c<='7'); // whether a valid octal char
The 10 decimal characters are guaranteed to be sequential so this will
work.
}

static int Oct2Dec(char c) {

return c-'0'; // octal to it's corresponding decimal
integer
}

static bool isOctString(char *s) {

char *tmp=s;

if(strlen(s)==0)
return false;

while(*tmp!='\0') {
if(!isOctChar(*tmp))
return false;
++tmp;
}

return true;
}

static int OctString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

size_t l=strlen(tmp);
int i,t;
int n=0;

for(i=l-1;i>=0;--i) {
t=Oct2Dec(*tmp);
if(t==0) {
++tmp;
continue;
}
n+=t*(int)pow(8,i);
tmp++;
}
return n;
}

static BASE Base(char *nstr) {

char input[64];
strcpy(input,nstr);

if(input[0]=='0') // case for hex or octal
if(input[1]=='x' || input[1]=='X') // hex
return HEX;
else return OCT; // octal
else if(isdigit(input[0]) && input[0]!='0') // decimal
The second half of the if must always be true because this is the else
to the very first if above.
return DEC;
else return UNKNOWN; // unknown base
}

static void MakeAFuss(void) {

fprintf(stderr,"Hey pal, what the hell's that ?\n");
}

int main(int argc,char **argv)
{
BASE base;
char tmp[64]={0};
int integer;

if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

base=Base((char *)argv[1]);
argv is a char**. Therefore, argv[1] must be of type char. Why the
cast?

switch(base) {
case DEC:
printf("%#x\t%#o\n",atol(argv[1]),atol(argv[1]));
atol returns a long. %x and %o both require the corresponding
argument to be int. You need some casts.
break;
case OCT:
strcpy(tmp,&argv[1][1]); // strip the oct-base header '0'
if(isOctString(tmp)) {
integer=OctString2Int(tmp);
printf("%u\t%#x\n",integer,integer);
}
else MakeAFuss();
break;
case HEX:
strcpy(tmp,&argv[1][2]); // strip the hex-base header '0x' or '0X'
if(isHexString(tmp)) {
integer=HexString2Int(tmp);
printf("%u\t%#o",integer,integer);
}
else MakeAFuss();
break;
default:
MakeAFuss();
}

return 0;
}
////////////////////// code end /////////////////////////

<<Remove the del for email>>

[sugaray]

Barry Schwarz <sc******@deloz.net> wrote in message news:<bu**********@216.39.134.8>...

On 21 Jan 2004 04:42:49 -0800, ru****@sohu.com (sugaray) wrote:

hi, i wrote a simple base-conveter utility for practice, it seems
works fine right now, hope any gurus out there can give me some
suggestions to, critics of, optimisations on this program or my
programming practice. the code are mainly written in C, but need to be
compiled using a C++ compiler because of the using of builtin Boolean
type. it has been tested under VC7 and GCC 3.3.1. thanx in advance.

/////////////////////// code start /////////////////////
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

typedef enum {DEC,OCT,HEX,UNKNOWN} BASE; // test for the base of the
user input

static char *TrimLeadingZero(char *s) {

char *tmp=s;
while(*tmp=='0') tmp++;
return tmp;
}

static bool isHexChar(char c) {

Is there some reason you don't like the standard function isxdigit()?

return ((c>='a' && c<='f')||(c>='A' && c<='F')); // whether a valid
hex char

There is no requirement in the C standards for a through f and/or A
through F to be sequential. It would be legal for the character set
to have the character + between a and b.

}

static bool isHexString(char *s) {

char *tmp=s;
size_t l,i;

if((l=strlen(s))==0) // expect no empty strings
return false;

for(i=0;i<l;++i)
if(!isHexChar(tmp[i]) && !isdigit(tmp[i])) // neither a valid hex
char nor a digit
return false;

return true;
}

static int Hex2Dec(char hex) {

if(isupper(hex))
return hex-'7'; // upper hex chars to integers

This only works for ASCII characters. It doesn't even come close for
EBCDIC.

else if(islower(hex))
return hex-'W'; // lower hex chars to integers
else return hex-'0'; // digit chars to their corresponding integers
}

static int HexString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

int i,t;
int n=0;
size_t l=strlen(tmp);

for(i=l-1;i>=0;--i) {

If you put the tmp++ in the third clause of the for statement, you can
get rid of both increment statements below.

t=Hex2Dec(*tmp);
if(t==0) {
tmp++;
continue; // jump out if is 0
}
n+=t*(int)pow(16,i);
++tmp;
}

You could get rid of the call to pow completely (it has to be
expensive in terms of time) by evaluating the hex string from left to
right. Something like
for (s = TrinLeadingZero(s); *s ; s++)
n = n*16 + Hex2Dec(*s);
which also allow you to get rid of i, t, tmp, and l.

return n;
}

static bool isOctChar(char c) {

return (c>='0' && c<='7'); // whether a valid octal char

The 10 decimal characters are guaranteed to be sequential so this will
work.

}

static int Oct2Dec(char c) {

return c-'0'; // octal to it's corresponding decimal
integer
}

static bool isOctString(char *s) {

char *tmp=s;

if(strlen(s)==0)
return false;

while(*tmp!='\0') {
if(!isOctChar(*tmp))
return false;
++tmp;
}

return true;
}

static int OctString2Int(char *s) {

char *tmp;

tmp=TrimLeadingZero(s);

size_t l=strlen(tmp);
int i,t;
int n=0;

for(i=l-1;i>=0;--i) {
t=Oct2Dec(*tmp);
if(t==0) {
++tmp;
continue;
}
n+=t*(int)pow(8,i);
tmp++;
}

return n;

}

static BASE Base(char *nstr) {

char input[64];
strcpy(input,nstr);

if(input[0]=='0') // case for hex or octal
if(input[1]=='x' || input[1]=='X') // hex
return HEX;
else return OCT; // octal
else if(isdigit(input[0]) && input[0]!='0') // decimal

The second half of the if must always be true because this is the else
to the very first if above.

return DEC;
else return UNKNOWN; // unknown base
}

static void MakeAFuss(void) {

fprintf(stderr,"Hey pal, what the hell's that ?\n");
}

int main(int argc,char **argv)
{
BASE base;
char tmp[64]={0};
int integer;

if(argc!=2) {
fprintf(stderr,"Usage:\n\t%s <number>\n",argv[0]);
return 1;
}

base=Base((char *)argv[1]);

argv is a char**. Therefore, argv[1] must be of type char. Why the
cast?

switch(base) {
case DEC:
printf("%#x\t%#o\n",atol(argv[1]),atol(argv[1]));

atol returns a long. %x and %o both require the corresponding
argument to be int. You need some casts.

break;
case OCT:
strcpy(tmp,&argv[1][1]); // strip the oct-base header '0'
if(isOctString(tmp)) {
integer=OctString2Int(tmp);
printf("%u\t%#x\n",integer,integer);
}
else MakeAFuss();
break;
case HEX:
strcpy(tmp,&argv[1][2]); // strip the hex-base header '0x' or '0X'
if(isHexString(tmp)) {
integer=HexString2Int(tmp);
printf("%u\t%#o",integer,integer);
}
else MakeAFuss();
break;
default:
MakeAFuss();
}

return 0;
}
////////////////////// code end /////////////////////////

<<Remove the del for email>>

Wow, amazing, thanx Barry!

[Mark A. Odell]

ru****@sohu.com (sugaray) wrote in
news:ad**************************@posting.google.c om:

[please learn to snip]

<<Remove the del for email>>

Wow, amazing, thanx Barry!

--
- Mark ->
--