patterncMinor
itoa with "method of complements" for negative numbers in ANSI C
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 {
```
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
The code uses
Put
When declaring a variable or function, you should put the
Eliminate unused variables
Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code,
Be careful with signed versus unsigned
In the
Practice safe programming
The
Order the functions
The
Eliminate spurious semicolons
The code currently contains this loop:
As the comment says, the final semicolon is not needed and should be eliminated.
Simplify your statements
The
should be simplified to just this:
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:
Use the required
#includesThe 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 firstWhen 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 neededAs 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 neededreturn 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.