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

Translation of Number Systems

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

Problem

My code converts a number from one number system to another (from binary to 36 and back). Do you think my formatting is successful or not?

```
#include
#include
#include
#include
#include
//#include

#define _CRT_SECURE_NO_WARNIN
#define MAXMEMORY 10

const int EXAMPLE_ARRAY = 36; // Длина шаблонного массива

int Degree(int from,int deg) { // Функция для нахождения степени числа
int i = 0;
long int stepen = 1;
for(i = 0; i 10 в уже в безбуквенном виде в число 10 СС
--size;
int i = 0;
int temp = 0;
int result = 0;
int deg = from;
while(size >= 0) {
deg = Degree(from, i);
result += (str[size] * deg);
++i;
--size;
}
printf("Число в 10 СС %d\n",result); // Вывод итогового числа, в 10 СС
return result;
}

int ConvertToNum(char str, int size) { // Заменяем буквы в числе на цифры
int k = 0;
--size;
int temp = (int)calloc(size + 1, sizeof(int)); // Заменяем память типа char на int
char abc[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ" ;
while(size >= 0) {
for(k = 0; k = 0) {
c = str[i];
str[i] = abc[c]; // В str должен прийти код буквы из ASCII
i--;
}
return;
}

void ConvertIt(int from, char *number1, int to, int memorysize, int length , int guest) { //u - длина числа guest - переменная в 10 СС из какой-либо другой
int ResultInDec = 0;
int number = 0; // Против переполнения
long long int numcopy = 0;
if(guest == 0) {
number = atoi(number1);
numcopy = number;
} else {
number = guest;
}
long int temp_numb = 0;
long int result = 0;
int i = 0;
int t = 0;
int k = 0;
if(from == 10) {
int numb = (int)calloc(memorysize, sizeof(int)); // Выделяю память для перевода числа, с минимальным размером.
while(number >= to) { // Делим число в 10 СС на число конечной СС и сохраняем остатки от деления в массив. Если памяти мало, то увел. в 2 раза

Solution

Working from the top down:

Your function Degree is a bit...oddly named. It's also equivalent to pow from math.h. Unless you have a good reason not to, I'd simply use pow, which likely uses exponentiation by squaring, which will run in \$O(log \ n)\$ instead of \$O(n)\$.

ConvertToDec is a bit inefficient in that it recalculates Degree(from, i) on every loop iteration. This is eventually doing \$O(n^2)\$ work. It also has an unused variable temp. This could be written more simply by starting deg at 1, and simply multiplying it by from each time through the loop:

int ConvertToDec(int *str, int from, int size) { 
    --size;
    int result = 0;
    int deg = 1;
    while(size >= 0) {
        deg *= from;
        result += (str[size] * deg);
        --size;
    }
    printf("Число в 10 СС %d\n",result); // Вывод итогового числа, в 10 СС 
    return result;
}


ConvertToNum allocates heap memory which it returns to the caller. This should absolutely be documented somewhere, as it's very easy to leak memory. Generally, you should probably have the caller pass in a pointer to memory that they have allocated, so:

void ConvertToNum(char* str, int size, int* num)


Further, the char* passed in shouldn't be modified, so should be marked const:

void ConvertToNum(const char* str, int size, int* num)


Further, your char[] abc never changes. Instead of stack allocating it every time this function is called, you should just make it static const:

static const char[] abc = "....";


In DoMyLetters, you don't need to return from a void function. You also initialise i with 0, only to immediately change its value to size; may as well simply just declare it as int i = size;.

Your ConvertIt function is long, too long in fact. I can see a number of cases, these should be broken up into their own functions (which should probably be static, and only allow ConvertIt to call them). If you're using a C99 compliant compiler, you might want to consider using variable length arrays instead of calloc (although this seems to be written with only C89 in mind, so this may not be possible).

This loop:

for(i = 0; i <= k; i++) {
    numb2[i] = numb[i]; 
}


Could be replaced by a memcpy. The next loop:

for(i = 0; i <= (k / 2); i++) { // Переворачиваем число т.к оно сохранилось наоборот
    t = numb2[i];
    numb2[i] = numb2[k - i];
    numb2[k - i] = t;
}


should use a swap function instead of doing this inline.

Your final printf call in ConvertIt is using the wrong specifier:

printf("Число в 10 СС %d\n", result); // Вывод итогового числа, в 10 СС


Here result is a long int, and so should be %ld. You also have another unused variable in this function, numcopy.

You have a memory leak on both error paths in your main function: up the top, you have:

char *number = (char*)calloc(maxmemory, sizeof(char));


In both:

if((EXAMPLE_ARRAY  from)) {


and

if(0 == length)


you return before freeing this memory (of course the OS will clean it up for you, but it's still good to remember these things).

In main, you're declaring c to be a char, and then using c = getchar(), however, getchar() actually returns an int. This can play havoc with the end of file indicator EOF. Again, potentially not a problem for a program reading from stdin, but you should still be aware of the issues it can cause.

Finally, I'd be remiss if I didn't point out that you can do everything here using the library function strtol - I assume this code is for an assignment or for learning purposes, so reinventing the wheel is fine. However, if this is not the case, stick to using standard library code rather than rolling your own.

Code Snippets

int ConvertToDec(int *str, int from, int size) { 
    --size;
    int result = 0;
    int deg = 1;
    while(size >= 0) {
        deg *= from;
        result += (str[size] * deg);
        --size;
    }
    printf("Число в 10 СС %d\n",result); // Вывод итогового числа, в 10 СС 
    return result;
}
void ConvertToNum(char* str, int size, int* num)
void ConvertToNum(const char* str, int size, int* num)
static const char[] abc = "....";
for(i = 0; i <= k; i++) {
    numb2[i] = numb[i]; 
}

Context

StackExchange Code Review Q#70503, answer score: 4

Revisions (0)

No revisions yet.