Как улучшить код на Си(сделать "красивше")?

Question

[timur102]

Задача:

Главный вождь племени Абба не умеет считать. В обмен на одну из его земель вождь другого племени предложил ему выбрать одну из трех куч с золотыми монетами. Но вождю племени Абба хочется получить наибольшее количество золотых монет. Помогите вождю сделать правильный выбор! 
INPUT.TXT
В первой строке входного файла INPUT.TXT записаны три натуральных числа через пробел. Каждое из чисел не превышает 10100. Числа записаны без ведущих нулей. 
OUTPUT.TXT
В выходной файл OUTPUT.TXT нужно вывести одно целое число — максимальное количество монет, которые может взять вождь.

#include <stdio.h>

#define N 103
char* maxNum(char *n1, char *n2);
int length(char s[]);
int main()
{
	FILE *in;
	FILE *out;
	int i,j;
	char c;
	char num1[N];
	char num2[N];
	char num3[N];
	char *nums[3] = {num1,num2,num3}; 
	in = fopen("input.txt","r");
	for (i=0;i<3; ++i){
		j = 0;
		while(c=fgetc(in)){
			if ( c == '\n' || c == ' ' ){
				(*(nums+i))[j] = '\0';
				break;
			}			
			(*(nums+i))[j] = c;
			j++;
			
		}
	}
	fclose(in);
	out = fopen("output.txt","w");
	fprintf(out, "%s",maxNum(*nums, maxNum(*(nums+1),*(nums+2))));
	fclose(out);
	//printf("%s", maxNum(*nums, maxNum(*(nums+1),*(nums+2))));
	return 0;
}

int length(char s[]){
	int i;
	for(i=0; i<N,s[i]!='\0';i++)
		;
	return i;
}

char* maxNum(char *n1, char *n2)
{
	int i;
	if (length(n1)>length(n2))
		return n1;
	if (length(n2)>length(n1))
		return n2;
	for (i=0; i<N;i++){
		if (n1[i]>n2[i])
			return n1;
		if (n2[i]>n1[i])
			return n2;
	}
}

Теперь вопрос: как сделать код меньше, "красивше". Может я где-то не те типы данных использовал. Подскажите, пожалуйста.

Answer 1

[jcmvbkbc]

char num1[N];
  char num2[N];
  char num3[N];
  char *nums[3] = {num1,num2,num3};

А чего не nums[3][N]?

while(c=fgetc(in))

Когда ты думаешь условие while перестанет выполняться?

if ( c == '\n' || c == ' ' )

Я бы добавил проверку на конец файла: if ( c == '\n' || c == ' ' || c == EOF)

(*(nums+i))[j]

А чего не nums[i][j]?

int length(char s[]){

Есть такая готовая функция strnlen из string.h

maxNum не обрабатывает случай когда аргументы равны.

Comments

[timur102]

maxNum не обрабатывает случай когда аргументы равны.

Такого случая в задаче быть не может

[jcmvbkbc]

Такого случая в задаче быть не может

timur102, во-первых из условия этого не следует, а во-вторых функция должна что-то возвращать в любом случае. Включи предупреждения компилятора, что ли.

[CityCat4]

Такого случая в задаче быть не может

Очень даже может. Ведь ничто не мешает их туда записать. В полном соответствии с законами Мерфи, вещь, которая не может быть никогда, случится при первом же запуске программы :)
У множества программ половина кода - это проверка на всевозможные ошибки и дебажная диагностика. Рассчитывать на то, что программа получает только правильные данные - неверно. Нужно предполагать, что пользователь дебил и сделает все настолько неправильно, насколько можно :)

[timur102]

CityCat4, Он еще захочет переполнение буфера сделать. Хах. В этом минус олимпиад. Там не надо писать защиты от дураков.

[Roman]

timur102,

if (length(n1)>length(n2))
    return n1;
  if (length(n2)>length(n1))
    return n2;

((length(n1) > length(n2)) ? n1: n2;

[jcmvbkbc]

((length(n1) > length(n2)) ? n1: n2;
Роман, когда длины равны что будешь делать?

[Roman]

jcmvbkbc, я для примера.
наверно так:

char* maxNum(char *n1, char *n2)
{
  int l = strlen(n1);
  int r = strlen(n2);
  if(l == r)
  {
    return (strcmp(n1, n2) > 0) ? n1 : n2;
  }
  else
  {
    return (l > r) ? n1 : n2;
  }
}

[jcmvbkbc]

Роман, strcmp можно заменить на memcmp.

Answer 2

[Станислав Бушуев]

Улучшить можно следующими способами:

• знанием и использованием стандартных функций
  
• проверкой результатов и обработкой ошибок
  

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

#define MAX_NUM_LENGTH 100

const char* chose(const char* left, const char* right)
{
	const size_t len_left = strlen(left);
	const size_t len_right = strlen(right);
	if (len_left > len_right)
		return left;
	if (len_right > len_left)
		return right;
	if (strcmp(left, right) < 0)
		return right;
	return left;
}

const char* get_max(const char* n1, const char* n2, const char* n3)
{
	return
	    chose(
	        chose(n1, n2),
	        chose(n2, n3));
}

int main()
{
	FILE* in = NULL;
	FILE* out = NULL;
	char n1[MAX_NUM_LENGTH + 1];
	char n2[MAX_NUM_LENGTH + 1];
	char n3[MAX_NUM_LENGTH + 1];
	in = fopen("input.txt", "r");
	if (!in) {
		perror("Cannot open file 'input.txt'");
		return 1;
	}
	if (3 != fscanf(in, "%100[0-9] %100[0-9] %100[0-9]", n1, n2, n3)) {
		perror("Wrong input format");
		fclose(in);
		return 1;
	}
	const char* max_num = get_max(n1, n2, n3);
	out = fopen("output.txt", "w");
	if (!out) {
		perror("Cannot open file 'output.txt'");
		return 1;
	}
	fprintf(out, "%s", max_num);
	fclose(in);
	fclose(out);
	return 0;
}

Comments

[Станислав Бушуев]

Причесал ещё немного:

• убрал лишнее сравнение
  
• добавил пропущенную проверку вызова библиотечной функции
  
• убрал явное ограничение на длину считываемой строки
  
• упростил логику управления выделенными ресурсами
  

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

#define FAIL(msg)    \
do                   \
{                    \
    perror(msg);     \
    goto cleanup;    \
} while (0)

static const char* choose(const char* left, const char* right)
{
	const size_t len_left = strlen(left);
	const size_t len_right = strlen(right);
	if (len_left > len_right)
		return left;
	if (len_right > len_left)
		return right;
	if (strcmp(left, right) < 0)
		return right;
	return left;
}

static const char* get_max(const char* n1, const char* n2, const char* n3)
{
	return
	    choose(
	        n1,
	        choose(n2, n3));
}

int main()
{
	FILE* in = NULL;
	FILE* out = NULL;
	char* n1 = NULL;
	char* n2 = NULL;
	char* n3 = NULL;
	const char* max_num = NULL;

	in = fopen("input.txt", "r");
	if (!in)
		FAIL("Cannot open file 'input.txt'");

	if (3 != fscanf(in, "%m[0-9] %m[0-9] %m[0-9]", &n1, &n2, &n3))
		FAIL("Wrong input format");

	max_num = get_max(n1, n2, n3);

	out = fopen("output.txt", "w");
	if (!out)
		FAIL("Cannot open file 'output.txt'");

	if (fprintf(out, "%s\n", max_num) < 0)
		FAIL("Output to 'output.txt' fail");

	fclose(in);
	fclose(out);
	free(n1);
	free(n2);
	free(n3);
	return 0;

cleanup:
	if (in)
		fclose(in);
	if (out)
		fclose(out);
	if (n1)
		free(n1);
	if (n2)
		free(n2);
	if (n3)
		free(n3);
	return 1;
}

Answer 3

[light24]

Нужно найти свой стиль оформления кода, и стараться его придерживаться
рекомендую на будущее С. Макконнелл "Совершенный Код"
---------------------------------------------------------------------------------------------
отступы после математических знаков ( +, -, *, /, =), после запятой, после проверок ( ==, !=, <=, <, >, >=), логические отступы - после каждой задачки, переменные нужно объявлять как можно ближе к месту использования

#include <stdio.h>
#include <stdlib.h>     /* atoi */

int main()
{
  char nums[3][N];

  // читаем
  FILE *in = fopen("input.txt", "r");
  for (int i = 0; i < 3; ++i)
  {
    int j = 0;

    char c;
    while(c = fgetc(in))
    {
      nums[i][j++] = c;

      if (c == '\n' || c == ' ')
      {
        nums[i][j] = '\0';
        break;
      }
    }
  }
  fclose(in);
  
  // находим
  int maxNums = 0;
  for (int i = 0; i != sizeof(nums); ++i)
  {
    const int curNum = atoi(nums[i]);
	if (curNum <= maxNums)
		continue;
		
	maxNums = curNum;
  }

  // выводим
  FILE *out = fopen("output.txt", "w");
  fprintf(out, "%s", maxNums);
  fclose(out);

  // printf("%s", maxNum(*nums, maxNum(*(nums+1),*(nums+2))));

  return 0;
}