用C语言编写的Hangman v2

Question

这是对我的以前的代码审查的后续，我已经将我在这个代码修订版中收到的反馈和其他一些改进结合起来了。

我希望您对这段代码的反馈，特别是在config.h中放置项目范围内的常量的做法，使用函数指针打印单词状态的count_missing_letters方法，以及D2循环，在main.c中，我现在继续处理无效的输入和使用空格字符。

是否有更简洁的方法来编写count_missing_letters，它完成了易于使用和不重复其代码的任务。我选择保留一个函数，以便单循环一次执行两次不同的检查，函数指针的使用将每次迭代中发生的事情解耦--不确定这是否为“惯用”C。

下面是源文件和CMakeLists文件(其中包括启用了许多运行时Clang消毒液)。或者，可以使用以下方法轻松编译代码：cc *.c -o hangman && ./hangman

main.c

/**
 *                      * Hangman in C *
 * O(1) lookup using pointers to 26 letters which each have a
 * state. A letter is either empty, or the letter itself.
 * I was inspired by seeing many other Hangman implementations which
 * relied on a multiple layers of iteration, this aims to be 'simple'
 * and 'idiomatic', by using a different approach.
 *
 * @version 2.0
 * @date 1/18/19
 * @author Faraz Fazli
 */

#include 
#include 
#include 
#include 
#include "rng.h"
#include "utils.h"
#include "config.h"

// Returns length of an array
#define len(x) (((sizeof(x)))/(sizeof((x)[0])))

int main() {
    char letters[ALPHABET_SIZE];
    int tries = 0;
    rng_init();
    memset(letters, HIDDEN_LETTER, ALPHABET_SIZE);
    size_t total_elems = len(words);

    char *word = words[rng_to(total_elems)];
    size_t word_len = strlen(word); // excludes NUL
    size_t word_size = word_len + 1; // includes NUL

    char **word_to_guess = malloc(word_size * sizeof(*word_to_guess));

    // Link letters in word to 'letters' array
    for (size_t i = 0; i < word_len; i++) {
        word_to_guess[i] = &letters[dst_from_a(word[i])];
    }

    size_t num_prev_missing = word_len;
    count_missing_letters(word_to_guess, print_char);
    fputs("\nPick a letter: ", stdout);

    int chosen_letter;
    while ((chosen_letter = getchar()) != EOF) {
        // Consume newline and other white-space characters
        if (isspace(chosen_letter)) {
            continue;
        }

        if (!isalpha(chosen_letter)) {
            puts("Please enter a valid letter.");
            continue;
        }

        chosen_letter = tolower(chosen_letter);
        size_t letter_pos = dst_from_a(chosen_letter);
        if (letters[letter_pos] != (char) HIDDEN_LETTER) {
            puts("Please pick a different letter");
            continue;
        }

        letters[letter_pos] = (char) chosen_letter;

        size_t num_missing = count_missing_letters(word_to_guess, print_char);
        if (num_missing == num_prev_missing) {
            tries++;
        }
        num_prev_missing = num_missing;

        if (num_missing == 0) {
            puts("-> YOU WIN!");
            break;
        }

        puts("");
        int tries_left = TOTAL_TRIES - tries;
        print_hangman(tries_left);
        if (tries_left > 0) {
            printf("\nTries Remaining: %d\n", tries_left);
            fputs("Pick a letter: ", stdout);
        } else {
            puts("No tries left! Game Over!");
            break;
        }
    }
    free(word_to_guess);
}

config.h

#ifndef HANGMAN_CONFIG_H
#define HANGMAN_CONFIG_H
/**
 * Use enum to replace "magic numbers" instead of #define or const
 * Ref: Practice of Programming, p.21
 */
enum {
    ALPHABET_SIZE = 26,
    TOTAL_TRIES = 10,
    HIDDEN_LETTER = '_',
};

static char *words[] = {"racing", "magic", "bow", "racecar"};

#endif //HANGMAN_CONFIG_H

utils.c

#include 
#include 
#include "config.h"

void print_hangman(int tries_left) {
    if (tries_left > 7) {
        return;
    }
    switch (tries_left) {
        case 7:
            puts("┏━━━╤━");
            puts("┃┃    ");
            puts("┃┃");
            puts("┃┃");
            puts("┃┃");
            puts("┻┻━━━━━━━");
            break;
        case 6:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃");
            puts("┃┃");
            puts("┃┃");
            puts("┻┻━━━━━━━");
            break;
        case 5:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃  | ");
            puts("┃┃    ");
            puts("┃┃    ");
            puts("┻┻━━━━━━━");
            break;
        case 4:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃ ╲| ");
            puts("┃┃    ");
            puts("┃┃    ");
            puts("┻┻━━━━━━━");
            break;
        case 3:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃ ╲|╱");
            puts("┃┃    ");
            puts("┃┃    ");
            puts("┻┻━━━━━━━");
            break;
        case 2:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃ ╲|╱");
            puts("┃┃  | ");
            puts("┃┃    ");
            puts("┻┻━━━━━━━");
            break;
        case 1:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃ ╲|╱");
            puts("┃┃  | ");
            puts("┃┃ ╱  ");
            puts("┻┻━━━━━━━");
            break;
        case 0:
            puts("┏━━━╤━");
            puts("┃┃  O ");
            puts("┃┃ ╲|╱");
            puts("┃┃  | ");
            puts("┃┃ ╱ ╲");
            puts("┻┻━━━━━━━");
            break;
    }
}

void print_char(char char_to_print) {
    putchar(char_to_print);
    putchar(' ');
}

size_t count_missing_letters(char **word_to_guess, const void(on_each(char))) {
    size_t num_missing = 0;
    while (*word_to_guess) {
        if (on_each != NULL) {
            on_each(**word_to_guess);
        }
        if (**word_to_guess++ == HIDDEN_LETTER) {
            num_missing++;
        }
    }
    return num_missing;
}

size_t dst_from_a(int letter) {
    return (size_t) abs(letter - 'a');
}

utils.h

#ifndef HANGMAN_UTILS_H
#define HANGMAN_UTILS_H

#include 

/**
 * Prints "hangman" ascii art
 * @param tries_left - must be <= 7 to display
 */
void print_hangman(int tries_left);

/**
 * Prints character, followed by a space
 * @param char_to_print
 */
void print_char(char char_to_print);

/**
 * Prints the state of each letter and counts the number of missing letters
 * Optionally calls a function with each character read
 * @param word_to_guess - word being guessed (array of pointers)
 * @param on_each - optional function to call on each iteration
 * @return underscore count
 */
size_t count_missing_letters(char **word_to_guess, void(on_each(char)));

/**
 * Returns the distance from 'a'
 * @param letter 'a' to 'z' (must be lower case)
 * @return 0 through 25
 */
size_t dst_from_a(int letter);

#endif //HANGMAN_UTILS_H

rng.c

#include "rng.h"
#include 
#include 

void rng_init(void) {
    srand((unsigned int) time(NULL));
}

size_t rng_to(size_t max) {
    return (unsigned) rand() / ((unsigned) RAND_MAX / max + 1u);
}

rng.h

#ifndef HANGMAN_RNG_H
#define HANGMAN_RNG_H

#include 

/**
 * Initializes Random Number Generator
 * Note: RNG is based on the current time and thus does not produce secure values.
 * This is intentional, as the RNG is solely used to select a random current word.
 */
void rng_init(void);

/**
 * Helper method for Random Number Generation
 * @param max - max number
 * @return between 0 to max
 */
size_t rng_to(size_t max);

#endif //HANGMAN_RNG_H

CMakeLists.txt

# Improved version adapted from https://codereview.stackexchange.com/a/210770/78786
cmake_minimum_required(VERSION 3.13)
project(Hangman C)

add_executable(${CMAKE_PROJECT_NAME} main.c utils.c utils.h rng.c rng.h config.h)

set(CMAKE_C_COMPILER clang)
target_compile_features(${CMAKE_PROJECT_NAME} PRIVATE c_std_99)
target_compile_options(${CMAKE_PROJECT_NAME} PRIVATE
        $<$:
        -Weverything
        -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi
        -flto
        -fvisibility=default>)
target_link_options(${CMAKE_PROJECT_NAME} PRIVATE
        $<$:
        -fsanitize=undefined,integer,implicit-conversion,nullability,address,leak,cfi
        -flto>)

Answer 1

这个代码一点也不坏，我喜欢新的ASCII艺术。以下是关于如何进一步改进它的一些想法：

不要把读者搞糊涂，

通过将事物组合在一个enum中，它确实消除了“神奇的数字”，但它也会误导读者，使他们认为这些项目是相关的。在这种情况下，实际上有三个独立的常数，它们唯一的关系就是它们都在这个游戏中使用。为此，我将使用const，并对每种类型使用适当的类型，因为ALPHABET_SIZE可能是size_t、HIDDEN_LETTER a char等。

考虑重新处理接口

目前，关注点并没有太大的分离。main程序知道程序的每一个部分。这是可行的，但更好的做法是把事情分开一点。我将config.h重命名为dictionary.h，并有一个名为get_random_word的函数：

const char *get_random_word() {
    static const char *words[] = {"racing", "magic", "bow", "racecar"};
    static const size_t word_count = sizeof(words)/sizeof(words[0]);
    return words[rng_to(word_count)];
}

这样就不需要main拥有宏，也不需要它知道任何关于随机数生成代码的信息。

考虑不同的数据结构

代码依赖于许多相关的数据结构，letters、words和word_to_guess。我会稍微改变一下他们的用途。首先，我会完全隐藏words，如上面所示。现在，实际的基本字母表是隐含的，而不是显式的。它假定字母表由ALPHABET_SIZE连续字符组成，从'a'开始。这适用于英语和ASCII编码，但不适用于EBCDIC编码，也不适用于其他语言，如西班牙语、法语或德语。相反，我建议可以有一个与前面提到的alphabet关联的显式dictionary.h字符串。可能是const。其次，我们可以使用一个长度相同的bool数组来跟踪字母中的哪个字母已经猜到了。这将是唯一需要在游戏中修改的数据结构。

示例

如果我们将字典的内容与字典隔离开来，它可能如下所示：

const char *dict_init () {
    static const char *alphabet = "abcdefghijklmnopqrstuvwxyz";
    rng_init();
    return alphabet;
}

const char *get_random_word() {
    static const char *words[] = {"racing", "magic", "bow", "racecar"};
    static const size_t word_count = sizeof(words)/sizeof(words[0]);
    return words[rng_to(word_count)];
}

现在，在main内部，我们可能有以下内容：

int main() {
    const char *alphabet = dict_init();
    bool *guessed = calloc(strlen(alphabet), sizeof(bool));
    int tries = 10;
    const char *word = get_random_word();
    size_t word_len = strlen(word);
    bool playing = true;

    while (playing) {
        display_word(word, word_len, alphabet, guessed);
        fputs("\nPick a letter: ", stdout);
        int chosen_letter;
        for (chosen_letter = tolower(getchar()); isspace(chosen_letter); chosen_letter = tolower(getchar())) 
        { }
        if (chosen_letter == EOF) {
            playing = false;
            continue;
        }
        const char *target = strchr(alphabet, chosen_letter);
        if (target == NULL) {
            puts("Please enter a valid letter.");
            continue;
        }
        if (guessed[target - alphabet]) {
            puts("Please pick a different letter");
            continue;
        }
        guessed[target - alphabet] = true;
        // is this letter in the word to be guessed?
        if (strchr(word, *target) != NULL) {
            if (display_word(word, word_len, alphabet, guessed)) {
                printf("\nTries Remaining: %d\n", tries);
            } else {
                puts("-> YOU WIN!");
                playing = false;
            }
        } else {  // guessed letter not in target word
            playing = print_hangman(--tries);
        }
    }
    free(guessed);
}

注意，我使用了strchr来查看字符是否在字母表中。如果还有猜测，我还修改了print_hangman以返回true，并添加了以下函数：

size_t display_word(const char *word, const size_t word_len, const char *alphabet, const bool *guessed) {
    size_t hidden = word_len;
    for (size_t i=0; i < word_len; ++i) {
        bool revealed = guessed[strchr(alphabet, word[i]) - alphabet];
        if (revealed) {
            putchar(word[i]);
            --hidden;
        } else {
            putchar('_');
        }
        putchar(' ');
    }
    return hidden;
}

就运行时性能而言，这并不是非常有效，但这并不重要，因为对于一个人工玩家来说，它已经足够快了。

Answer 2

小想法：

Repetitive调用

这是另一种想法，而不是一项建议。

避免重复调用puts()并仍然维护代码"art“的一种方法是使用字符串文本连接。

        puts(
            "┏━━━╤━\n"
            "┃┃\n"
            "┃┃\n"
            "┃┃\n"
            "┃┃\n"
            "┻┻━━━━━━━");

注意，优化编译器可以通过任何方式将原始puts()连接在一起。

代码可以将这8个字符串放入字符串const char *art[8] = {...};数组中，然后使用art[tries_left]而不是switch。

与这样的样式问题一样:按照您组的编码标准编写代码。

Type命名

与其在代码中使用unsigned int和unsigned，不如使用其中之一。