CODEWITHSUNDEEP
c
Nabmeister
Nabmeister

Reputation: 795

Decimal to Binary conversion not working

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

int myatoi(const char* string) {
  int i = 0;
  while (*string) {
    i = (i << 3) + (i<<1) + (*string -'0');
    string++;
  }
  return i;
}

void decimal2binary(char *decimal, int *binary) {
  decimal = malloc(sizeof(char) * 32);
  long int dec = myatoi(decimal);
  long int fraction;
  long int remainder;
  long int factor = 1;
  long int fractionfactor = .1;
  long int wholenum;
  long int bin;
  long int onechecker;
  wholenum = (int) dec;
  fraction = dec - wholenum;

  while (wholenum != 0 ) {
    remainder = wholenum % 2;  // get remainder
    bin = bin + remainder * factor;  // store the binary as you get remainder
    wholenum /= 2;  // divide by 2
    factor *= 10;  // times by 10 so it goes to the next digit
  }
  long int binaryfrac = 0;
  int i;
  for (i = 0; i < 10; i++) {
    fraction *= 2;  // times by two first
    onechecker = fraction;  // onechecker is for checking if greater than one
    binaryfrac += fractionfactor * onechecker;  // store into binary as you go
    if (onechecker == 1) {
      fraction -= onechecker;  // if greater than 1 subtract the 1
    }   
    fractionfactor /= 10;
  }

  bin += binaryfrac;
  *binary = bin;
  free(decimal);
}

int main(int argc, char **argv) {   
  char *data;
  data = malloc(sizeof(char) * 32);
  int datai = 1;
  if (argc != 4) {
    printf("invalid number of arguments\n");
    return 1;
  }
  if (strcmp(argv[1], "-d")) {  
    if (strcmp(argv[3], "-b")) {
      decimal2binary(argv[2], &datai);
      printf("output is : %d" , datai);
    } else {
      printf("invalid parameter");
    }
  } else {
    printf("invalid parameter");
  }
  free(data);
  return 0;
}

In this problem, myatoi works fine and the decimal2binary algorithm is correct, but every time I run the code it gives my output as 0. I do not know why. Is it a problem with pointers? I already set the address of variable data but the output still doesn't change.

./dec2bin "-d" "23" "-b"

Upvotes: 0

Views: 262

Answers (4)

Sankar Mani
Sankar Mani

Reputation: 178

if(!strcmp(argv[3] , "-b"))

if(!strcmp(argv[3] , "-d"))

The result of the string compare function should be negated so that you can proceed. Else it will print invalid parameter. Because the strcmp returns '0' when the string is equal.

In the 'decimal2binary' function you are allocating a new memory block inside the function for the input parameter 'decimal',

decimal = malloc(sizeof(char) * 32);

This would actually overwrite your input parameter data.

Upvotes: 0

sukumar
sukumar

Reputation: 143

void decimal2binary(char *decimal, int *binary) {
  decimal = malloc(sizeof(char) * 32);
  ...
}

The above lines of code allocate a new block of memory to decimal, which will then no longer point to the input data. Then the line

long int dec = myatoi(decimal);

assigns the (random values in the) newly-allocated memory to dec.

So remove the line

decimal = malloc(sizeof(char) * 32);

and you will get the correct answer.

Upvotes: 0

Adam Liss
Adam Liss

Reputation: 48330

Another coding tip: instead of writing

if (strcmp(argv[1], "-d")) {  
  if (strcmp(argv[3], "-b")) {
    decimal2binary(argv[2], &datai);
    printf("output is : %d" , datai);
  } else {
    printf("invalid parameter");
  }
} else {
  printf("invalid parameter");
}

you can refactor the nested if blocks to make them simpler and easier to understand. In general it's a good idea to check for error conditions early, to separate the error-checking from the core processing, and to explain errors as specifically as possible so the user will know how to correct them.

If you do this, it may also be easier to realize that both of the original conditions should be negated:

if (strcmp(argv[1], "-d") != 0) {  
  printf("Error: first parameter must be -d\n");
else if (strcmp(argv[3], "-b") != 0) {
  printf("Error: third parameter must be -b\n");
} else {
  decimal2binary(argv[2], &datai);
  printf("Output is: %d\n" , datai);
}

Upvotes: 0

Adam Liss
Adam Liss

Reputation: 48330

The line:

long int fractionfactor = .1;

will set fractionfactor to 0 because the variable is defined as an integer. Try using a float or double instead.

Similarly,

long int dec = myatoi(decimal);

stores an integer value, so wholenum is unnecessary.

Instead of

i = (i << 3) + (i<<1) + (*string -'0');

the code will be much more readable as

i = i * 10 + (*string - '0');

and, with today's optimizing compilers, both versions will likely generate the same object code. In general, especially when your code isn't working, favor readability over optimization.

fraction *= 2;  // times by two first

Comments like this, that simply translate code to English, are unnecessary unless you're using the language in an unusual way. You can assume the reader is familiar with the language; it's far more helpful to explain your reasoning instead.

Upvotes: 1

Related Questions