Вот мое решение для Пришествие Кода 2020, день 4, Часть 1 в C. Я пробовал использовать fscanf или fgets, но не нашел в этом ничего хорошего, поэтому в итоге я использовал getc. Есть ли способ использовать fscanf или fgets, который улучшит этот код? Кроме того, как в целом выглядит код?

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

typedef struct keyValuePair
{
    char key[4];
    char value[20];
} KeyValuePair;

typedef struct passport
{
    KeyValuePair fields[8];
    int numberOfFields;
} Passport;

int main()
{
    FILE* f = fopen("../input.txt", "r");
    if (f == NULL)
    {
        printf("Fail opening filen");
        return -1;
    }

    Passport passports[1000];
    int currentPassport = 0;
    int currentField = 0;

    char key[4];
    char value[20];

    int c;
    while (!feof(f))
    {
        c = getc(f);
        if (c == 'n')
        {
            ++currentPassport;
            currentField = 0;
            continue;
        }
        else
        {
            // read key
            key[0] = (char)c;
            key[1] = (char)getc(f);
            key[2] = (char)getc(f);
            key[3] = ' '; // place null terminator at the end

            // discard ':'
            c = getc(f);

            // read value
            int index = 0;
            while (c != ' ' && c != 'n')
            {
                c = getc(f);
                if (c == EOF) { break; } // at the last line, instead of a new line we have EOF
                else if (c != ' ' && c != 'n') { value[index++] = (char)c; }
            }
            value[index] = ' ';  // place null terminator at the end

            strcpy(passports[currentPassport].fields[currentField].key, key);
            strcpy(passports[currentPassport].fields[currentField].value, value);
            ++currentField;

            ++passports[currentPassport].numberOfFields;
        }
    }

    int numberOfValidPassports = 0;
    for (int i = 0; i <= currentPassport; ++i)
    {
        if (passports[i].numberOfFields == 8) { ++numberOfValidPassports; }
        else if (passports[i].numberOfFields == 7)
        {
            int cidFound = 0;
            for (int j = 0; j < passports[i].numberOfFields; ++j)
            {
                if (strcmp(passports[i].fields[j].key, "cid") == 0)
                {
                    // passport contains cid and has 7 fields, therefore invalid
                    cidFound = 1;
                    break;
                }
            }
            if (!cidFound) { ++numberOfValidPassports; }
        }
    }

    printf("Number of valid passports: %d", numberOfValidPassports);

    fclose(f);
    return 0;
}

Разделите свою программу на несколько функций

Вы можете значительно улучшить читаемость и удобство сопровождения вашей программы, разделив ее на несколько функций, каждая из которых выполняет простую, четко определенную задачу. Например, вы можете уменьшить main() к:

int main()
{
    FILE* f = fopen("../input.txt", "r");
    int numberOfValidPassports = countValidPassports(f);
    fclose(f);
    printf("Number of valid passports: %d", numberOfValidPassports);
};

Так что единственная ответственность main() теперь ввод-вывод: чтение файла и запись результата в stdout. Функция countValidPassports() теперь нужно делать то же самое, что и оригинал main(), но ему больше не нужно беспокоиться об вводе-выводе. Конечно, это новое countValidPassport() Сама функция может быть разделена на несколько функций.

Проверить на наличие ошибок при чтении из открытого файла

Вы только добавили проверку, чтобы убедиться, что файл был открыт правильно, однако при чтении файла также могут возникать ошибки. Хорошая практика — проверить и их наличие, и правильно сообщить о них, распечатав сообщение об ошибке (чтобы stderr) и выходом из программы с ненулевым кодом выхода.

Если произойдет ошибка чтения, ваш код войдет в бесконечный цикл, поскольку вы использовали feof() чтобы проверить конец файла. Если есть ошибка чтения до конца файла, feof() не вернется true. В общем, избегать while(!feof(...)) петля. Просто проверьте результат фактической операции чтения, чтобы проверить, удалось ли она:

while ((c = fgetc(f)) != EOF) {
    ...
}

В fgetc() функция вернет EOF когда он не может прочитать символ, либо из-за ошибки чтения, либо из-за того, что был достигнут конец файла.

Избегайте слишком много предположений о вводе

Ваш код делает много предположений о том, как выглядит ввод. Например, вы предполагаете, что ключи всегда имеют длину 3 символа, а значения — не более 20 символов. Вы также предполагаете, что единственными возможными ключами являются действующие ключи от паспорта. Но что, если вы получите эту строку на входе?

this:isaverylongkeyandvaluepair

Это заставит вас написать pas в конце value массивы. Даже если нет проблемы переполнения буфера, рассмотрите следующий ввод:

eyr:1 iyr:2 byr:3 byr:4 byr:5 byr:6 byr:7 byr:8

Вы бы посчитали это действующим паспортом, но у него есть несколько проблем:

• Не все семь обязательных полей присутствуют
• Некоторые поля повторяются (это разрешено?)
• Год истечения более ранний, чем год выпуска
• Год выпуска раньше года рождения

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

Не храните данные без надобности

Во время синтаксического анализа вы сохраняете копии ключей и значений, которые вы нашли в массиве. passports. Но в конечном итоге в этом массиве ничего не используется, вам нужно только распечатать количество действующих паспортов. Так что это просто пустая трата памяти. Единственное, что вам нужно запомнить, это для текущего паспорта, который вы разбираете, какие ключи вы уже видели. Как только вы закончите разбор одного паспорта, вы можете сразу решить, действителен он или нет, и увеличить numberOfValidPassports если действительно.

Это также решает потенциальную проблему, которая может возникнуть, если входные данные содержат более 1000 паспортов.

С использованием fscanf() или же fgets()

Действительно, стоит подумать об использовании fscanf() или же fgets() при разборе файла. Ваш текущий метод разбора символа за символом несколько неэффективен. В идеале вы должны написать что-то вроде:

while (...) {
    char key[...];
    char value[...];

    if (fscanf(f, " %[^: n]:%s", key, value) == 2) {
        // we read a valid key:value pair
        ...

    }
}

В строке формата " %[^: n]:%s", сначала идет пробел, который «съест» все ведущие пробелы, затем мы сопоставляем строку, которая может содержать любой символ, кроме двоеточия, пробела или новой строки. Затем он ожидает двоеточие, а после него все, что угодно, до следующего пробела.

Однако невозможно отличить таким образом пробелы, разделяющие пары ключ: значение, от пустых строк, разделяющих паспорта. Поэтому вместо этого вы можете использовать fgets() читать целыми строками. Легко проверить, пуста ли строка, и если вы знаете, что она разделяет два паспорта. Если в нем что-то есть, вы можете использовать sscanf() чтобы разобрать его, вот так:

char line[...];
while (fgets(line, sizeof line, f)) {
    if (/* check for empty line */) {
        // Passport separator
        ...
    } else {
        // Scan this line for key:value pairs
        char key[...];
        char value[...];
        char *start = line;
        int n;

        while (sscanf(start, " %[^: n]:%s%n", key, value, &n) == 2) {
             // we read a valid key:value pair
             ...
             start += n;
        }
    }
}

Хитрость здесь заключается в использовании %n чтобы проверить, сколько символов было в паре ключ: значение, чтобы мы могли пропустить ее при следующем вызове sscanf().

Однако: помните о возможных переполнениях буфера. Есть несколько способов справиться с ними, либо используйте спецификаторы ширины поля в строке формата, чтобы сообщить sscanf() чтобы не читать больше символов, чем длина вашего буфера, или, если вы можете использовать расширения POSIX, рассмотрите возможность использования %ms иметь sscanf() выделите для строки достаточно большой буфер. Поскольку вам не нужны значения, вы можете избежать их сохранения, используя подавление присваивания: %*s. Прочтите документацию для sscanf() чтобы увидеть все его возможности.