Не обижайтесь, но ваш код ужасен, как вы и подозревали:)
Но это возможность узнать, почему и как вы можете это исправить, поэтому я объясню подробно:
В вашем коде есть 4 основные проблемы.
Две из них ломают программу и приводят к тому, что она печатает мусор или вылетает из-за ошибки сегментации, в зависимости от расположения памяти при запуске вашей программы.Два других позволят вашей программе напечатать правильный ответ, но сделают его ужасно неэффективным, тратя впустую память и циклы процессора.
Если это задание, по которому вы получаете оценку, вы потеряете счет, а если вы учитесь для себя,это плохая практика программирования.
Проблема 1:
Как уже отмечалось в комментариях, вы всегда должны NULL завершать строки в C. Если вы этого не сделаете, функциинапример, printf
, который их использует, будет печатать содержимое памяти до тех пор, пока они не встретят случайное значение NULL или пока ваша программа не выйдет из строя.
Итак, во внешнем цикле for
вы должны добавить строку, подобную этой:1016 *
for (j = 0; j < 5; i++) {
//code to do the cypher
encrypted_string[j][5] = '\0'; //add terminator to string
}
В качестве альтернативы, вы можете инициализировать массив так, как показывал вам Bwebb:
encrypted_string[5][6] = { '\0' };
Это заранее поместит нули во все ячейки массива.Но, если вы используете этот метод, будьте осторожны, чтобы не перезаписать последнюю ячейку строки в одном из ваших циклов!
Проблема 2:
Также указал Bwebb, выувеличивайте alphabet_count
и numbers_count
вместе, но используйте numbers_count
для индексации меньшего массива.
Это означает, что для любой буквы от k до z ваша программа будет обращаться к памяти, к которой она не должна обращаться при выполнении второгоif
check.
Однако эта проблема исчезнет сама собой, если вы исправите следующие две проблемы и правильно структурируете свой код.
Проблема 3:
Вы тратите память на два больших массива: массив букв и массив чисел.
Они вам не нужны!Поскольку вы правильно рассчитали шифр, вы уже должны знать, как буквы и цифры представлены в компьютере, поэтому вам не нужно сравнивать вашу строку с содержимым этих массивов по одному.
Вы знаете буквыявляются непрерывным диапазоном, поэтому просто используйте один if
.
Задача 4:
Вы тратите время на цикл while
.
Это частьта же проблема, что и в задаче 3 - вам не нужно зацикливаться!Просто сделайте одно сравнение, чтобы увидеть, находится ли текущий символ в диапазоне букв, и если это не так, сделайте еще одно сравнение, чтобы увидеть, находится ли он в диапазоне чисел.
Ваш код будет работать правильно, если вы исправитетолько проблемы 1 и 2, но если вы исправите 3 и 4, ваш код будет короче и легче для чтения и понимания, поэтому вероятность появления других ошибок будет гораздо меньше.
Вот как это сделатьэто:
//this is the inner loop, the outer loop over j stays the same
for (i = 0; i < 5; i++) {
//check if this is a letter - a range between a and z inclusive:
if (encrypted_string[j][i] >= 'a' && encrypted_string[j][i] <= 'z') {
encrypted_string[j][i] += 7; //encrypt through shift by 7
//rotate to beginning of alphabet if needed:
if (encrypted_string[j][i] > 'z') encrypted_string[j][i] -= 26;
} else if (/* do the same thing for numbers) {
...
}
}
Обратите внимание, что я разделил ваш расчет шифрования на две строки: это также облегчает чтение и понимание.
Вы все еще можете использовать свой исходный расчет:
encrypted_string[j][i] = ((((login_ids[j][i] - 'a') + 7) % 26) + 'a');
Но удобочитаемость - важный аспект любого хорошего кода.