Add Hangman exercise in C track

[MerveilleK1]

This is the Hangman game in C. It is based on other versions already available on Exercism, notably Python and Java. This is the first push and contains the C code structure, plus 3 basic tests.

[ryanplusplus]

Hey @MerveilleK1, is this still a work in progress? Are you looking to get feedback on your approach so far?

[MerveilleK1]

Hey @MerveilleK1, is this still a work in progress? Are you looking to get feedback on your approach so far?

Hey @ryanplusplus , yeah, there are still some tests to add, to cover all the functionnalities of the game. But yeah it would be nice to get a first feedback on the approach, Thanks.

[siebenschlaefer]

Hi @MerveilleK1, I'm not a maintainer on the C track but here are some notes:

• When you want to implement an exercise you can use our configlet tool. Download it with the fetch-configlet script in the bin/ directory of this repository and execute the command bin/configlet create --practice-exercise hangman from the root of the repository. It will create the directory exercises/practice/hangman and populate it with almost all required files.
• The name of the directory should match the name in the problem-specifications repository.
  Please rename the directory hang to hangman.
• The test file for all our other exercises is named test_<slug>.c (e.g. see the high-scores exercise).
  Please rename hangman_test.c to test_hangman.c.
• hangman.h and hangman.c look like a complete solution. The files hangman/hangman.h and hangman/hangman.c should be the initial stubs that the students see when they start the exercise. The example solution should be stored in the directory hangman/.meta/, with the filenames example.h and example.c.
• The makefile does not match the makefile from the other exercises.
  Please copy one one of them. No changes should be necessary.
• This repository should not contain compiled programs.
  Please remove test_runner.exe
• Your code uses four spaces per indentation level but this repository uses three spaces. (Yes, I do not like that either.)
  Please re-indent your code.

hangman.h:

• I'm not sure which parts of this file are the initial stub that the students see when they start solving this exercise. For this section of my notes I'm treating the whole hangman.h as the stub.
• Do you really want STATUS_WIN, STATUS_LOSE, and STATUS_ONGOING to be strings?
  A couple of distinct values where each of them has a name, to me that sounds like a job for an enum.
• Providing the definition of the struct Hangman in the stub of the .h file makes some IMHO important decisions: That this should not be an opaque type, which members it has, what their types are, whether word and masked_word should be arrays or should point to separately allocated strings.
  I'd rather leave that to the students. How about just typedef struct Hangman Hangman; instead in the stub of the .h file?
• Related: The stub has declarations of two "getter functions" get_masked_word() and get_status() but not for the remaining_guesses. The tests access the game->remaining_guesses directly.
  For consistency I'd suggest getter functions for all properties that should be accessible from the outside, or for none of them.
• guessed_letters is a pointer to an array of int. That's not wrong but I'm curious: Wouldn't bool be sufficient and express the intent more clearly? Also, couldn't it be a regular array of length 26?
• From the declaration of the function create_game() it's not clear whether it can be assumed that the argument word is alive during the lifetime of the Hangman instance or whether create_game() should copy this string.
  These types of ambiguities cause problems in practice and make the task unclear for our students. I'd suggest adding a comment.

hangman.c

• strdup() is not part of the C99 standard. I think we will get a compiler error with our compiler flags -std=c99, -pedantic, and -Werror.
• guess_letter() does terminate the program if the game is not ongoing.
  I'd prefer to handle this in some other way, e.g. by returning early and effectively ignoring the function call, or by returning an error code. Then the behavior can be tested with our testing framework, and (in a hypothetical real program) the caller could decide how they want to handle this error.
• guess_letter() passes a char to tolower(). This tolower() comes with a catch: It takes an int and its behavior is undefined if its argument is neither representable by an unsigned char nor equal to EOF. You should never pass a char or a signed char that might be negative to tolower() but always cast it to unsigned char first (see also cppreference).
• This solution calls strlen() in the condition of the for loops. Not all compiler are smart enough to figure out that the result of the function call will be the same throughout the loop, they might call strlen() over and over. For short strings that's probably fine, but for longer strings you would get a significant slowdown through the quadratic runtime complexity. Since some students view our example solutions as "good code" I'd prefer to avoid these strlen() calls in condition of loops.

hangman_test.c

• Please rename to test_hangman.c (see above).
• Header files from the standard library should be #include-d with angle brackets. Please replace "string.h" with <string.h>. Or remove this include completely, I don't think it's needed.
• The test functions should be defined as static, otherwise our compiler flags -Wmissing-declarations and -Werror will report an error.
• All three test functions (and the commented-out fourth test function) clean up at the end by calling destroy_game().
  You could consider making game a (static) global variable, setting it to NULL in setUp() and calling destroy_game() in tearDown().
• Exercism promotes Test-Driven Development (TDD). When you download exercises to solve them on your computer only the first test is enabled, the other tests are disabled by the TEST_IGNORE() macro. Therefore the second test should start with a line TEST_IGNORE(); // delete this line to run test (including this comment), all following tests should start with the line TEST_IGNORE();
• The generic website for the Hangman exercise shows that this exercise has been added to seven tracks: C#, F#, Java, OCaml, Powershell, Python, and Tcl. You could look at their test files for additional test cases.

[siebenschlaefer]

Dear maintainers,

This exercise was originally intended to be implemented and solved with Functional Reactive Programming (FRP)
(see the blurb and the description.)

This makes the exercise hard to translate to non-FP languages.
The C# version uses the "ReactiveX" library, the Java version uses "RxJava".
Powershell, Python, and Tcl have amended the instructions to explicitly ignore FRP.

Sadly, I'm a beginner at best in functional programming languages and I know nothing about functional reactive programming (FRP). I don't know how FRP would look in C and how this exercise could be translated faithfully.

Should we follow the example of the Python, PowerShell, and Tcl track and implement this exercise in a procedural style?
Or do you have an idea how we could keep the exercise closer to FRP?

[ryanplusplus]

Thanks @siebenschlaefer from the thorough review! You addressed all of the things that I noticed and several more.

Should we follow the example of the Python, PowerShell, and Tcl track and implement this exercise in a procedural style?
Or do you have an idea how we could keep the exercise closer to FRP?

I'm also not an expert in FRP or functional programming, but I have a little bit of experience. The FRP-style react exercise is included in this track and in that exercise the emphasis is on implementing FRP building blocks as opposed to solving a real problem. I think react is pretty clearly the most difficult exercise in the track. Expecting an FRP solution for hangman would require similar effort in creating the FRP building blocks and then additional effort in applying them to solve a hangman puzzle. I suspect that in practice people would submit procedural solutions or give up in frustration. My vote is to add the instruction appends to explicitly ignore FRP like in Powershell, Python, and Tcl.

[wolf99]

Hi @MerveilleK1
Are you OK to adjust per the comments from @siebenschlaefer or would you like some help?