General Advice

Hi,

I recently wrote the following program to convert decimal numbers into binary numbers, the program works fine, however, my question is based around the prebit() function:
Is this good practice? I started off with a reverse() function, but wanted something a bit more elegant, I now wonder if there are any potential problems with this method?

Thanks

Paul

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define  MAX_DIGITS  16


int powi(int number,int powers);
void prebit(char *bin_value,int digit);


int main(int argc,char *argv[]) {

    int  dec_value = atoi(argv[1]);
    char bin_value[MAX_DIGITS+1] = "";
    int  digit = 0;
    int  bit = 0;

    for (digit=0; digit<MAX_DIGITS; digit++) {

        bit = dec_value & powi(2,digit);

        if (bit == 0) {
           prebit(bin_value,0);
        } else {
           prebit(bin_value,1);
        }

    }

    printf("%s\n",bin_value);

    return 0;

}


int powi(int number,int powers) {

    int times = 0;
    int power = 1;

    for (times=1; times<=powers; times++) {
        power = power * number;
    }

    return power;

}


void prebit(char *bin_value,int digit) {

    char tmp_value[MAX_DIGITS+1] = "";
    char bit[2] = "";

    sprintf(bit,"%d",digit);

    /*
    **             +---+---+---+---+              +---+---+---+---+
    ** bin_value = | 1 |   |   |   |  tmp_value = | 0 |   |   |   |  After strcpy(tmp_value,bit)
    **             +---+---+---+---+              +---+---+---+---+
    ** bin_value = | 1 |   |   |   |  tmp_value = | 0 | 1 |   |   |  After strcat(tmp_value,bin_value)
    **             +---+---+---+---+              +---+---+---+---+
    ** bin_value = | 0 | 1 |   |   |  tmp_value = | 0 | 1 |   |   |  After strcpy(bin_value,tmp_value)
    **             +---+---+---+---+              +---+---+---+---+
    */

    strcpy(tmp_value,bit);
    strcat(tmp_value,bin_value);
    strcpy(bin_value,tmp_value);

}

That's a "very slow" method for converting decimal to binary. You have nested loops and you keep recreating/copying the binary string. Something like this would be much faster:

Code:

#include <stdio.h>
#include <stdlib.h>

#define MAX_DIGITS (sizeof(int) * 8)

int main(int arc, char *argv[])
{
  int dec_value = atoi(argv[1]);
  char bin_value[MAX_DIGITS + 1], *b = bin_value;
  int shift;

  for(shift = MAX_DIGITS - 1;shift >= 0;--shift)
    *b++ = ((dec_value >> shift) & 1) + '0';
  *b = '\0';

  puts(bin_value);
  return 0;
}

Output:

Code:

$ dectobin 9073745
00000000100010100111010001010001

Many Thanks for the advice, I'm still learning C so this was a very good example of how powerful C is and just how easy it is to get things wrong.

I have just one question: Why did you increase the MAX_DIGITS to 32?

~P

Why did you increase the MAX_DIGITS to 32?

int is generally a 4-byte (32 bit) value. The maximum number of binary digits needed to represent it is 32.