trjhtr

Rules

• 7 mistakes allowed.


• Hangman rules.

Disclaimer

I don't have a figure (drawing) of the hangman as this project took way longer than I thought I would take. It took like 9 hours during 3 weeks.

General mistakes (I could detect but can't program away):

1. Poor way of generating the random word to be guessed.




2. I have dummy code that isn't used ( I don't know how to remove this without going line for line )




3. I'm used to MVC frameworks but my display functions seem to need some calculating code, because things get difficult when I keep them in a separate function.

Reflection

I am doing more of these little assignments and realized my "good code" and "fast/dirty code" both are poor and don't differ that much.

Where does one learn how to categorize your code

Does anyone know a book that like explains which kind of code needs to be in which kind of function? As I feel I'm reinventing the wheel. Like how do you guys feel that code should be in another place?

#!/bin/bash

words[1]="twitter"
words[2]="facebook"
words[3]="myspace"
words[4]="youtube"
words[5]="amazon"
random_nr=6
correct_letters[1]=""
mistake_letters[1]=""
imploded_letters[1]=""
geuss_counter=1
correct_counter=0
mistakes=0
place=1

# setters
set_word() 
 until [[ $random_nr -lt 6 ]] && [[ $random_nr -gt 0 ]]; do 
 random_nr=$RANDOM;
 word=$words[$random_nr]
 done


set_word_length() 
 word_length_incl_zero=$#word
 word_length=$((word_length_incl_zero+1))


# views
display_result() 
 for i in $(seq 1 $word_length); do
 if [[ $i -ne $word_length ]]; then
 position=$((i-1))
 if [[ $word:position:1 == $user_input ]]; then
 correct_counter=$((correct_counter+1))
 correct_letters[$correct_counter]=$user_input

 continue_game
 fi
 fi
 done

 echo "sorry you're wrong"

 mistakes=$((mistakes+1))
 mistake_letters[$mistakes]=$user_input


display_welcome_message() 
 echo "GALGJE"
 echo
 echo "Hello, so happy to see you playing my handmade game"
 echo "Basicly you need to guess every letter of the word below"
 echo "Before things go badly with the guy on the left"
 echo


display_dashes() 
 letters_not_guessed=0
 for i in $(seq 1 $word_length); do
 if [[ $i -ne $word_length ]]; then
 position=$((i-1))
 check_if_already_guessed
 if [[ $found -eq 1 ]]; then
 echo -n $word:position:1
 else
 letters_not_guessed=$((letters_not_guessed+1))
 echo -n "-"
 fi
 fi
 done
 echo
 echo


check_if_already_guessed() 
 found=0
 for i in "$correct_letters[@]"
 do
 if [ "$i" == "$word:position:1" ] ; then
 found=1
 fi
 done


display_asking_for_a_letter() 
 if [[ $letters_not_guessed == 0 ]]; then
 echo "you've won"
 exit
 fi
 read -sn 1 user_input
 echo


display_mistakes() 
 if [[ $mistakes -lt 7 ]]; then
 for i in $(seq 1 $#mistake_letters[@]); do
 if [[ $mistakes -eq 1 ]]; then
 more_mistakes_than_one=""
 else
 more_mistakes_than_one="s"
 fi

 if [[ $i -eq 1 ]]; then
 echo -n "wrong ($mistakes) letter$more_mistakes_than_one: "
 echo -n $mistake_letters[$i]
 else
 echo -n ", $mistake_letters[$i]"
 fi
 done
 echo
 else
 end_game
 fi


display_correct_letters() 
 if [[ $mistakes -lt 7 ]] && [[ $correct_counter -lt $word_length ]]; then
 for i in $(seq 1 $#correct_letters[@]); do
 if [[ $correct_counter -eq 1 ]]; then
 more_correct_than_one=""
 else
 more_correct__than_one="s"
 fi

 if [[ $i -eq 1 ]]; then
 echo -n "right ($correct_counter) letter$more_correct_than_one: "
 echo -n $correct_letters[$i]
 else
 echo -n ", $correct_letters[$i]"
 fi
 done
 echo
 echo
 else
 end_game
 fi


continue_game() 
 display_mistakes
 display_correct_letters
 geuss_counter=$((geuss_counter+1))
 main


end_game() 
 echo "sorry you've lost"
 exit


get_first_time_guess() 
 first_time_guess=1

 for letter in "$correct_letters[@]"
 do
 if [ "$letter" == "$user_input" ] ; then
 first_time_guess=0
 fi
 done

 for letter in "$mistake_letters[@]"
 do
 if [ "$letter" == "$user_input" ] ; then
 first_time_guess=0
 fi
 done


main() 
 #visuals
 display_dashes
 display_asking_for_a_letter

 #if this letter is already guessed the user
 #shouldnt be penalized for it
 get_first_time_guess
 if [[ $first_time_guess -eq 1 ]]; then
 display_result
 fi

 continue_game


set_word
set_word_length
display_welcome_message
main

Code organization and naming

It's good that you organized your code into functions,
and that you set everything in motion after all the function definitions,
using a few simple terms that drive your program:

set_word
set_word_length
display_welcome_message
main

It's a pity that the simple terms that drive the program are not human-friendly, and not natural.
Better names and some rethinking can help:

• set_word -> My first reaction reading this is "what word?", and "set to what?". If you play this game with a friend, would you ask him to "set word"? Probably you'd phrase it more like "pick a random word". In a program, the name init_random_word would sound quite natural to me.




• set_word_length -> Again, my first reaction is "set to what?" More importantly, the purpose of this unclear, I have to read the implementation to understand. The fact that you need a word_length variable is a low-level implementation detail, and as such it would be better to not require this function call at the top-level. It would be better to move this step inside the init_random_word function, directly (the code itself) or indirectly (call to the function that performs this step).




• display_welcome_message -> This is good as it is, perfectly clear what it will do and why.




• main -> A more natural and intuitive name would be run_hangman_game. This and the other calls could actually be in a function called main, and you could call main to setup and run the game.

As to your specific question about code organization:

Does anyone know a book that like explains which kind of code needs to be in which kind of function? [...] Like how do you guys feel that code should be in another place?

I recommend the book Code Complete. It's not a direct answer to your question, but it's my favorite fundamental book for programmers (especially the chapter on Abstract Data Types), and I think it will guide you on the path of developing good programmer instincts. As a more direct answer, I suggest to read up on SOLID principles, especially the Single Responsibility Principle is relevant for designing reusable, testable program elements.

Picking a random word

This is an extremely poor implementation of picking a random word from an array:

until [[ $random_nr -lt 6 ]] && [[ $random_nr -gt 0 ]]; do 
 random_nr=$RANDOM;
 word=$words[$random_nr]
done

One big problem is that RANDOM returns a random value between 0 and 32767.
It can be a lot of wasted cycles until you get a number between 1 and 5.
The common approach is to get a number between 0 and n using modulo:

num=$((RANDOM % n))

The same thing, but in a slightly more natural and readable form in Bash:

((num = RANDOM % n))

Be careful, because this formula gets a random number between 0 and n,
not 1 and n.
The current program needs a random number between 1 and 5,
because words[0] is not assigned, only words[1] ... words[5].
You could adjust the formula to account for that:

((num = 1 + RANDOM % (n - 1)))

But a much simpler way will be to change the program,
to require a number in the range of 0 and n,
by assigning values in words starting from element 0 instead of 1.
This is also the most natural way to work with arrays with 0-based indexes as in Bash, and in most programming languages.

Note that we can assign all elements at once,
which is shorter, and most importantly,
we don't need to worry about writing the correct indexes:

words=("twitter" "facebook" "myspace" "youtube" "amazon")

And then we can get a random word from this array with:

word=$words[RANDOM % $#words[@]]

This is safe, fast, and simple.
We can also eliminate the random_nr variable and the set_word_length function,
they are simply no longer needed.

Arithmetic context in Bash

As mentioned earlier, instead of this:

mistakes=$((mistakes+1))

This is a simpler syntax:

((mistakes = mistakes + 1))

Notice that within the arithmetic context ((...)) the $ is optional in variable names (except some special variables such as the positional variables $1, $2, ... and a few others).

And this example can be further simplified to either of these forms:

((mistakes += 1))
((mistakes++))

Iterating over letters of a string

Avoid seq as much as possible, it's not available in many environments,
and a native Bash alternative exists, in the form of counting loops.
For example, instead of this:

for i in $(seq 1 $word_length); do

You can write in native Bash:

for ((i = 1; i <= word_length; i++)); do

(Notice that I didn't write $ in front of i and word_length,
since the expressions are within an arithmetic context ((...)),
the $ are unnecessary.)

Now let's fix the loop in the code:

for i in $(seq 1 $word_length); do
 if [[ $i -ne $word_length ]]; then
 position=$((i-1))
 if [[ $word:position:1 == $user_input ]]; then
 correct_counter=$((correct_counter+1))
 correct_letters[$correct_counter]=$user_input

 continue_game
 fi
 fi
done

Not only seq is inferior to native Bash counting loops,
it also forced you to add a conditional $i -ne $word_length in the loop body,
and then you also needed to compute position as $((i-1)).
All this complexity can go away if you rewrite with a counting loop:

for ((i = 0; i < $#word; i++)); do
 if [[ $word:i:1 == $user_input ]]; then
 ((correct_counter++))
 correct_letters[$correct_counter]=$user_input

 continue_game
 fi
done

Notice that I didn't use the word_length variable,
it's no longer necessary here.
I suggest to apply the same technique to eliminate it from the rest of the program.

Code organization 2

Now that I've reached the implementation of the display_result function,
I see that it calls a continue_game function from its loop,
which will call main (among other functions).
These recursive calls generate a deep stack,
which is difficult to understand and follow.

A better way is to reorganize the program without such recursive calls.
Use a loop that takes user input,
checks the result,
tracks the program state,
and either continues to take user input or terminates with the final result (user won or lost).

Minor things

• The program is full of typos and grammar mistakes. It's good to pay attention to details.




• imploded_letters and place are defined but not used. Delete them.




• correct_counter and mistakes should not be necessary. With minor changes, you can get these values from $#correct_letters[@] and $#mistake_letters[@], respectively. Note also that it would be better to initialize these arrays as correct_letters=() and mistake_letters=(), and to append values to them with correct_letters+=($user_input).




• The flags of echo are not portable, such as -n. Without flags, echo is fine. If you really want to suppress the newline character, a possible alternative is printf, if you don't mind that it's not POSIX. If possible, it's best to find a way to avoid both, for example by designing the user interface in such a way that it doesn't need to suppress the newline anywhere.