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

itoa with "method of complements" for negative numbers in ANSI C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodwithnumbersansiforcomplementsnegativeitoa

Problem

I have enhanced my itoa implementation, taking the advise from my previous simple itoa implementation in addition to handling multiple radixes and negative numbers other then just base 10. Looking for general feedback.

```
const static size_t complemnt_digit_count[] = {
0, 0, 32, 20, 16, 14, 12, 12, 11, 10,
-1, 9, 9, 9, 9, 8, 8, 8, 8, 8,
8, 8, 7, 7, 7, 7, 7, 7, 7, 7,
7, 7, 7, 7, 7, 7, 6};

//-------------------------------------------------------------

size_t
charcnta(int i, int base)
{
int tmp;
short digit_count;

if (i = '0' && i[index] = 'a' && i[index] max_char_index) {
return NULL;
}
}

// Calculate the (n - 1)'s complement.
for (index = 0; index = '0' && i[index] = 'a' && i[index] = 0; --index) {
if (i[index] >= '0' && i[index] = 'a' && i[index] = base) { // We have to carry
i[index] = '0';
carry = 1;
} else {

Solution

I see a number of things that may help you improve your code.

Use the required #includes

The code uses strlen which means that it should #include . It also needs stdlib.h, limits.h and more. The code is incomplete without the appropriate #includes.

Put static first

When declaring a variable or function, you should put the static keyword first. See this question for details on why.

Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code, carry_bit and complement_char are never used. My compiler also tells me that. Your compiler is probably also smart enough to tell you that, if you ask it to do so.

Be careful with signed versus unsigned

In the ncomp function, len is declared to be size_t type, but index is an int. They should probably both be size_t because they are compared within the for loops.

Practice safe programming

The ip_itoa function is passed a pointer to a buffer but not the length of that buffer, making it all too easy to create a buffer overflow. Pass the length as well and check it.

Order the functions

The ncomp function is called by ip_itoa and so either the ncomp should have a declaration above ip_itoa or the entire function should be moved above ip_itoa.

Eliminate spurious semicolons

The code currently contains this loop:

while (tmp) {
    tmp /= base;
    ++digit_count;
};   // <-- that semicolon is not needed


As the comment says, the final semicolon is not needed and should be eliminated.

Simplify your statements

The sizeof operator always returns 1 for sizeof(char) so statements like this:

return digit_count * sizeof(char);


should be simplified to just this:

return digit_count;


Fix the bugs

The program gives very strange output for negative numbers. For example, it says that -9 base 17 = "ggggggg8" but that corresponds to a value of 6975757432 which does not appear to be correct. Also, there are problems with other numbers. For example, it reports that 42 base 15 = "2<" which is clearly not correct. Here is the test program I used:

void checkConversion(int test_val, int base, char *buff) 
{
    char *test_val_str = ip_itoa(test_val, buff, base);
    long check_val = strtol(test_val_str, NULL, base);
    printf("%s: %d base %d = %s (check val = %ld)\n", 
            ((check_val == test_val) ? "OK " : "BAD"), 
            test_val, base, test_val_str, check_val);
}

#define VAL_COUNT 7
static const int vals[VAL_COUNT] = { -9, 100, 88, 0, -1, +1, 42};

int main() 
{
    int test_val;
    char *test_val_str;
    char *buff = malloc(100);
    for (int base = 2; base < 36; ++base) {
        for (int i = 0; i < VAL_COUNT; ++i) {
            checkConversion(vals[i], base, buff);
        }
    }
    free(buff);
}

Code Snippets

while (tmp) {
    tmp /= base;
    ++digit_count;
};   // <-- that semicolon is not needed
return digit_count * sizeof(char);
return digit_count;
void checkConversion(int test_val, int base, char *buff) 
{
    char *test_val_str = ip_itoa(test_val, buff, base);
    long check_val = strtol(test_val_str, NULL, base);
    printf("%s: %d base %d = %s (check val = %ld)\n", 
            ((check_val == test_val) ? "OK " : "BAD"), 
            test_val, base, test_val_str, check_val);
}

#define VAL_COUNT 7
static const int vals[VAL_COUNT] = { -9, 100, 88, 0, -1, +1, 42};

int main() 
{
    int test_val;
    char *test_val_str;
    char *buff = malloc(100);
    for (int base = 2; base < 36; ++base) {
        for (int i = 0; i < VAL_COUNT; ++i) {
            checkConversion(vals[i], base, buff);
        }
    }
    free(buff);
}

Context

StackExchange Code Review Q#122442, answer score: 2

Revisions (0)

No revisions yet.