HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Binary string to integer and integer to binary string

Submitted by: @import:stackexchange-codereview··
0
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.

#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 binstr2int produce an unsigned long of full range returned through a
parameter 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 the
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
do {...} while loop makes sense as it can handle the zero case as any other
number 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 after
creating 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.