patterncMinor
Binary string to integer and integer to binary string
Viewed 0 times
andstringbinaryinteger
Problem
I am working through K&R for review and I thought that I would write some of my own routines for conversion of binary strings to int and int to binary string. I realize this is reinvetning the wheel; I am just doing this for fun. Also, I am attempting to follow this coding style CodingStyle.txt. Any and all critiques are welcomed.
Edit 1:
Okay, here is the revised code. Most of the suggestions I agree with. I did retain filling the buffer from end to beginning because it seems like a logical flow to me and requires one less variable to deal with.
#include
#include
#include
/*
Parameters:
s - String with a maximum of 63 binary digits
Return:
- Decimal value of the binary string pointed to by s
Error:
- Negative returned on error check errno
*/
signed long binstr2int(char *s)
{
signed long rc;
for (rc = 0; '\0' != *s; s++) {
if (rc > (LONG_MAX/2)) {
errno = ERANGE;
return -1;
} else if ('1' == *s) {
rc = (rc * 2) + 1;
} else if ('0' == *s) {
rc *= 2;
} else {
errno = EINVAL;
return -1;
}
}
return rc;
}
/*
Parameters:
num - The number to convert to a binary string
s - Pointer to a memory region to return the string to
len - Size in bytes of the region pointed to by s
Return:
- Pointer to the beginning of the string
Error:
- NULL returned on error check errno
*/
char *int2binstr(unsigned long num, char *s, int len)
{
if (len >= 1) {
if (0 == len) {
errno = ERANGE;
*s = '\0';
return s;
} else if (num & 1) {
s[--len] = '1';
} else {
s[--len] = '0';
}
}
}
return s + len;
}Edit 1:
Okay, here is the revised code. Most of the suggestions I agree with. I did retain filling the buffer from end to beginning because it seems like a logical flow to me and requires one less variable to deal with.
- The do while suggestion was spot on. The for loop was unusual and the do while consumed one of the conditionals making the code easier to read.
- The names of the functions have been revised
Solution
It makes sense for the two functions to be symmetrical - ie that you can call
them in turn to swap between integer and string representations. So I would
make
parameter and have the function return the status (0/-1):
Note that
In
string - this is what all other C string functions do (or at least I'm
ignorant of any that do not). Also, your code is a bit awkward in
that it treats 0 separately from all other numbers. This is a case where a
number and test for zero once it has done it:
Here you can see that the tests for a short input string and for
creating it.
them in turn to swap between integer and string representations. So I would
make
binstr2int produce an unsigned long of full range returned through aparameter and have the function return the status (0/-1):
int binstr2int(const char *s, unsigned long &num);Note that
s can be const.In
int2binstr I think the characters should be moved to the beginning of thestring - this is what all other C string functions do (or at least I'm
ignorant of any that do not). Also, your code is a bit awkward in
that it treats 0 separately from all other numbers. This is a case where a
do {...} while loop makes sense as it can handle the zero case as any othernumber and test for zero once it has done it:
char* int2binstr(unsigned long num, char *s, size_t size)
{
long len = (long) size - 1;
do {
if (len >= 1) != 0);
long n = (long) size - len - 1;
memmove(s, s + len, n);
s[n] = '\0';
return s;
}Here you can see that the tests for a short input string and for
num == 0 are handled naturally. Also I moved the string into place aftercreating it.
Code Snippets
int binstr2int(const char *s, unsigned long &num);char* int2binstr(unsigned long num, char *s, size_t size)
{
long len = (long) size - 1;
do {
if (len <= 0) {
errno = ERANGE;
return NULL;
}
s[--len] = (num & 1) ? '1' : '0';
} while ((num >>= 1) != 0);
long n = (long) size - len - 1;
memmove(s, s + len, n);
s[n] = '\0';
return s;
}Context
StackExchange Code Review Q#43256, answer score: 6
Revisions (0)
No revisions yet.