8
\$\begingroup\$

After completing an introduction to Python tutorial, I wanted to put my basic knowledge of control flow and data structures to use, and felt this was a great choice for my first program.

I created the below to run in Jupyter Notebook / Visual Studio Code terminal. It behaves like an over-the-board version, based on colours and pins.

Without experience of how to structure code I found myself putting everything into functions and used functions within functions, rightly or wrongly, leaving only a small amount of what I've called 'main game code'. Feedback on how I should structure properly, or resources to that effect would be greatly appreciated.

I used no guides nor copied online code, so the code is a bit bloated, I imagine, as a first attempt. I've tried to make it a bit more user friendly running in the terminal so there are a lot of input validations and line spaces as I wanted to actually play this with my housemate and it not be a horrible experience.

Summary of the rules

2 players, playing at least 2 games, each getting a chance to be the codemaker and codebreaker.

Two game modes: Beginner or Mastermind. The latter allows the codemaker to use colours more than once.

Three difficulty settings: Easy, Medium or Hard. The difficulty determines the number of guesses the codebreaker gets.

Points earned, and tallied across all games, by the player acting as codemaker for each guess the codebreaker makes, with a bonus point if the codebreaker fails entirely.


 def getPlayers(): player1 = input("Player 1, you will start as the Codemaker. What is your name? ") print("Player 1 is",player1,"and will start as the Codemaker") print() player2 = input("Player 2, you will start as the Codebreaker. What is your name? ") print("Player 2 is",player2,"and will start as the Codebreaker") return player1, player2 def checkUserInput(userInput): try: int(userInput) return int except (ValueError, TypeError): try: float(userInput) return float except (ValueError, TypeError): return str def getNumberOfGames(): while True: numGames = input("How many games do you want to play? Min 2, Max 8. Remember, an even number of games must be played!") userInputType = checkUserInput(numGames) if userInputType == int: if int(numGames) in range(2,9,2): print() print("You will be playing",numGames,"games") print("----") return numGames break else: print("Your entry is incorrect, please try again") def getMode(): D = {"B":"Beginner","M":"Mastermind"} D2 = {"B":"the Codemaker cannot use the same colour more than once","M":"the Codemaker can use colours more than once"} print("----") print("Select game mode: Enter 'B' for Beginner, or, 'M' for Mastermind") print() while True: mode = str.capitalize(input()) if mode in ("B","M"): #alternative way to write if mode == "B" or mode == "M" print("You have selected",D[mode],"mode! In",D[mode],"mode",D2[mode]) print("----") break else: print(mode,"is not a valid game mode, please select a game mode") print() return mode def getDifficulty(): D = {"E":"Easy","M":"Medium","H":"Hard"} D2 = {"E":12,"M":10,"H":8} print("Select game mode: Enter 'E', 'M' or 'H' for Easy, Medium or Hard") print() while True: difficulty = str.capitalize(input()) if difficulty in ("E","M","H"): print("You have selected",D[difficulty],"difficulty! The Codebreaker will get",D2[difficulty],"chances to crack the code!") print("----") break else: print(difficulty,"is not a valid option, please select a difficulty") print() return difficulty, D2[difficulty] def duplicateColours(L): for x in L: if L.count(x) > 1: return True break else: pass return False def getNewCode(mode): D = {'W':'White','R':'Red','B':'Blue','Y':'Yellow','P':'Purple','O':'Orange','G':'Green','S':'Silver'} L = [] print("Quickly, Codemaker! Enter a new 4 colour code to stop the Codebreaker from breaking into the vault!") print(""" Select from the 8 available colours by typing the first letter of the colour: White -- Enter 'W' Red -- Enter 'R' Blue -- Enter 'B' Yellow -- Enter 'Y' Purple -- Enter 'P' Orange -- Enter 'O' Green -- Enter 'G' Silver -- Enter 'S' """ ) for cbsi in range (1,5): while True: print('Enter colour',cbsi) inputColour = str.capitalize(input()) if D.get(inputColour) is None: print("Codemaker, colour,'",inputColour,"'is not available, please select colour",cbsi,"again") print() else: L.append(inputColour) if mode == "B": if duplicateColours(L) == True: L.pop(len(L)-1) print("Duplicate colours are not allowed in Beginner mode!") print(L) continue print('Colour',cbsi,'is',inputColour) print(L) print() break print("Codemaker, your chosen code is",L,"keep it hidden from the Codebreaker!") print() print("!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!") input("Press any key to scramble the screen!") print(""" *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** """) print("----") return L def checkGuess(L, mode): D = {'W':'White','R':'Red','B':'Blue','Y':'Yellow','P':'Purple','O':'Orange','G':'Green','S':'Silver'} for x in L: if D.get(x) is None: print("Your guess contains colours that aren't available, please try again") return False break else: if mode == "B": if duplicateColours(L) == True: print("Duplicate colours are not allowed in Beginner mode! Please try again") return False break return True def getGuess(guesses): L2 = [] while True: print() playerGuess = input("Codebreaker, what is your guess? Enter 4 colours using the first letter of the colour as your input, separated by commas: ") print() L2 = str.upper(playerGuess).split(",") if len(L2) != 4: print("Your guess does not contain 4 colours") print() else: if checkGuess(L2, mode) == True: print("Guess #",guesses+1) print(L2) break else: pass return L2 def guessCheck(L,L2): D = {'W':'White','R':'Red','B':'Blue','Y':'Yellow','P':'Purple','O':'Orange','G':'Green','S':'Silver'} Ly = ["*","*","*","*"] Lt1 = L.copy() Lt2 = L2.copy() for y in range(len(L2)): if L[y] == L2[y]: Ly[y] = "X" Lt1[y] = "*" Lt2[y] = "*" for y in range(len(L2)): if D.get(Lt2[y]) is not None: if Lt1.count(Lt2[y]) != 0: Ly[y] = "O" Lt1[Lt1.index(Lt2[y])] = "*" Lt2[y] = "*" return Ly def playGame(guesses, currentPlayer): print(""" Available colours: White (W), Red (R), Blue (B), Yellow (Y), Purple (P), Orange (O), Green (G), Silver (S) """) codemakerScore = 0 for x in range(guesses): L2 = getGuess(x) Ly = guessCheck(L,L2) print(Ly," Help: 'X' = correct colour and order, 'O' = correct colour, wrong order, '*' = incorrect guess") codemakerScore += 1 if L2 == L: print() print("!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+") print("Congratulations, Codebreaker! You cracked the code") print("!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+!+") print() break if x == int(guesses)-1: print("Codebreaker... You failed to crack the code. The Codemaker was granted a bonus point") codemakerScore += 1 print("The Codemaker",currentPlayer,"gained",codemakerScore,"points this round") print() return codemakerScore def getCurrentPlayer(player1,player2,numberOfGames): if int(numberOfGames) %2 ==0: currentPlayer = player1 else: currentPlayer = player2 return currentPlayer def updatePlayerScores(currentPlayer,codemakerScore,player1Score,player2Score): if currentPlayer == player1: player1Score += codemakerScore else: player2Score += codemakerScore return player1Score,player2Score def getWinner(player1Score, player2Score): if player1Score > player2Score: return player1 if player1Score == player2Score: return "Draw" else: return player2 #Main game code******************************************** player1, player2 = getPlayers() mode = getMode() difficulty, guesses = getDifficulty() numberOfGames = getNumberOfGames() player1Score = 0 player2Score = 0 for x in range(len(numberOfGames)+1): print() currentPlayer = getCurrentPlayer(player1,player2,x) print() print(currentPlayer,"is the Codemaker") print() L = getNewCode(mode) print() print("Current scores are: ",player1,"=",player1Score," / ",player2,"=",player2Score) codemakerScore = playGame(guesses,currentPlayer) player1Score, player2Score = updatePlayerScores(currentPlayer,codemakerScore,player1Score,player2Score) print("Current scores are: ",player1,"=",player1Score," / ",player2,"=",player2Score) input("Press any key to continue") winner = getWinner(player1Score,player2Score) print() print("After",numberOfGames,"games, the winner is: ",winner,"!") print() print("The final scores were ",player1,"=",player1Score," / ",player2,"=",player2Score) 
\$\endgroup\$

    1 Answer 1

    5
    \$\begingroup\$

    It's a pretty good piece of code for someone just beginning, good job.

    There are a couple of things, as always, that could be improved :

    Making a better use of Python's functionalities

    Languages all have some special functionalities that make the code easier to write/read. It's good to know these because well... they're there to be used.

    1 ) String templating : Instead of writing print("Player 1 is",player1,"and will start as the Codemaker") you should write print(f"Player 1 is {player1} and will start as the Codemaker"). This is valid for all the cases where you mix strings with code.

    2 ) Using tuples : So, for me it was a big thing when I started Python because I came from C# where tuples were... less powerful. There are some places where you have two dictionaries with the same keys, for example :

    def getDifficulty(): D = {"E":"Easy","M":"Medium","H":"Hard"} D2 = {"E":12,"M":10,"H":8} print("Select game mode: Enter 'E', 'M' or 'H' for Easy, Medium or Hard") print() while True: difficulty = str.capitalize(input()) if difficulty in ("E","M","H"): print("You have selected",D[difficulty],"difficulty! The Codebreaker will get",D2[difficulty],"chances to crack the code!") print("----") break else: print(difficulty,"is not a valid option, please select a difficulty") print() return difficulty, D2[difficulty] 

    I think I would write it this way :

    def getDifficulty(): possible_difficulties = {"E":("Easy", 12),"M":("Medium", 10),"H":("Hard", 8)} print("Select game mode: Enter 'E', 'M' or 'H' for Easy, Medium or Hard­\n") while True: difficulty_input = str(input()).upper() if difficulty_input not in possible_difficulties.keys(): print(f"{difficulty_input } is not a valid option, please select a valid difficulty\n") continue chosen_difficulty = possible_difficulties[difficulty_input] print(f"You have selected {chosen_difficulty[0]} difficulty! The Codebreaker will get {chosen_difficulty[1]} chances to crack the code!") return chosen_difficulty 

    There are a couple things to unpack here:

    1 ) I took some liberties with the variable names. Using single letters as variable names is almost always a bad idea.

    2 ) I fused the two dictionaries by having a tuple value that contains the text and the number of chances. You could even use a named tuple instead of a plain tuple to really explain what is the purpose of the two values.

    3 ) I removed some nesting. It's been proven (I believe) that nesting code inside loops and conditions reduces the code clarity. So I chose to first check if the input is invalid, if so, I use the continue keyword to return to the start of the loop. It's a personal preference to check for invalid input, but you could also check for a valid input and return the chosen difficulty :

    def getDifficulty(): possible_difficulties = {"E":("Easy", 12),"M":("Medium", 10),"H":("Hard", 8)} print("Select game mode: Enter 'E', 'M' or 'H' for Easy, Medium or Hard­\n") while True: difficulty_input = str(input()).upper() if difficulty_input in possible_difficulties.keys(): chosen_difficulty = possible_difficulties[difficulty_input] print(f"You have selected {chosen_difficulty[0]} difficulty! The Codebreaker will get {chosen_difficulty[1]} chances to crack the code!") return chosen_difficulty print(f"{difficulty_input } is not a valid option, please select a valid difficulty\n") 

    I don't have time to make a longer review, but I have one last point, don't do this :

    print(""" *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** *********************************************** """) 

    I have to scroll like 4 or 5 mouse wheels to see the end of it and code should almost always be readable above all. If you want to print this, you should replace it with the following : print("*"*3000) or however long you want it to be.

    \$\endgroup\$
    1
    • 1
      \$\begingroup\$print("*"*3000) - brilliant. In truth, the decision to print that many *'s was in place of what I actually wanted to do which was find a way to delete the output from the terminal display in a "This message will self destruct in x seconds" type of thing, but that would have required some googling. Great feedback on the getDifficulty function, aside from not knowing some of the built in functions and data structures that come with experience, your suggestion on structuring the logic differently in the while loop is very helpful.\$\endgroup\$
      – M.White
      CommentedJan 26, 2022 at 19:19

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.