Simple Python Console Game With Inefficient Code Flow

Question

I made a simple program as an exercise in improving my code flow in Python, which does the following:

1. asks user to input their name
2. presents them with a choice to either print their name in reverse, or print the number of characters
3. after the output is printed, the user is asked if they want to "play again". if not, exits

Have I done anything especially goofy? Any low hanging fruit? Best practice I'm unaware of?

from sys import exit
def userinput():
 print('Enter your name:')
 n = input()
 print('Hello, ' + n + ', would you rather (R)everse or (C)ount the letters of your name?')
 a = input()
 return (a,n)
def choosef(choice,myname):
 while choice not in ['R','C']:
 print("Only 'R' or 'C' are valid. Try again:")
 choice = input()
 if choice == 'R':
 print(''.join(reversed(myname)))
 else:
 spaces = 0
 for c in list(myname):
 if c == ' ':
 spaces = spaces + 1
 print('Total # Characters: ' + str(len(myname) - spaces))
 playagain()
def playagain():
 print("Would you like to play again, (Y)es or any other character to exit?")
 yn = input()
 if yn == 'Y':
 ui = userinput()
 choosef(ui[0],ui[1])
 else:
 exit()
ui = userinput()
choosef(ui[0],ui[1])

• IEatBagels: @AryanParekh after only 11 hours, I don't see why OP would be pressed to do this
• ZeroDadeCoolMurphy: @BCdotWEB I had originally posted this on Stack Overflow but it was migrated here. Because the two forums serve different purposes, I don't understand how a post can maintain a fitting title after such migrations. There is a recommended edit to change the title to "Simple Beginner Python Console Game". But the fact the code happened to make a "console game" is completely irrelevant to the guidance I was seeking. I was hoping to draw the attention of people who are especially knowledgeable about code structure, not console games. Can you help me understand a bit better?
• L. F.: Don't worry about the migration - just keep in mind to use a descriptive title if you happen to post again on Code Review in the future.
• ZeroDadeCoolMurphy: @BCdotWEB - I read your comment and the link before I responded. To clarify, I'm less confused about title guidelines on CodeReview than I am about the rationale for migrating my post from StackExchange to CodeReview in the first place. From your link: "If your code does not have a goal, then it is likely that...you are asking about best practices in general...such questions are off-topic for Code Review." In my post, I explicitly asked about any "best practice I'm unaware of?" Should I request a different migration (e.g. Software Engineering), or just accept the less relevant title edit?

Answer 1

Try to forget everything that you know about the code, and assume that you aren't the author. Imagine yourself to be a third person reading the code, he/she knows nothing about it and just reading it for the first time.

ui = userinput()
choosef(ui[0],ui[1])

Alright, userinput() must take some input and put it into ui. Makes sense, but what is choosef()?.

Naming convention

Use meaningful names for functions and variables. take this example

x = 3.14
y = 24

what is x and y to the reader? Is that the price of something?

pi_value = 3.14
number_of_hours = 24

Ahhh okay, now I know exactly what 3.14 means. So If I use it somewhere the reader will also know what I exactly mean. This is an important part of writing clean code.

Another aspect is the style. There are a few ways I can write user input*

user_input()
USERINPUT()
UserInput()
userinput()

Which one should I follow?

To have a consistent naming convention, python code follows the PEP-8 naming convention. When I say follows, I mean that it is recommended that you should follow it too as other python libraries also use this. It makes the code look cleaner.

Things like functions follow : lower_snake_case

Classes follow: CamelCase

You can read the link for more information.

Taking input in Python

print('Enter your name:')
n = input()

Clearly, you want to display a message to the user before he enters something. This is why the input() function has something called input prompt.

name = input("Enter your name: ")

You can display the message between the (). This removes the additional line. Also note that I changed n to name for the reasons I mentioned above.

Formatting strings

From your code, I can see that you concatenated strings with + to form meaningful sentences. That works but there is a huge problem when you want to use variables of different types.

name = "Eric"
age = 14
job = "Comedian"
print("Hello " + name + "You are " + age + " years old and you are a " + comedian)

TypeError: can only concatenate str (not "int") to str

Simply use Python 3's f-strings. Place the letter 'f' before "

print(f"Hello {name}, you are {age} years old ")

Cleaner.

return value from userinput()

What you are currently doing is returning a tuple of choice and name. It's better that you keep them as they are because later on, you slice from it just to get them again. This means you directly do

return name, choice
# Calling the function
name, choice = userinput()

---

Code structure

Couple of points

• Do not ask "Do you want to play again? " in the play_again() function. The reason you called that function should be because the user wants to play again. Move that to a playgame() function which will call user_input() every time the user wants to play the game, and break from the loop only if the user enters 'n' or "no"

• spaces = spaces + 1 can be simplified into spaces += 1

• Move the part where you count the number of characters into a separate function which will return an integer. So your play_game() function doesn't do anything other than play the game. When you need the characters , number_of_char = character_len( name ).

• Use enum for clarity.

An improved version of the code

from enum import Enum
class PrintChoices(Enum):
 number_of_char = 'c'
 reversed_name = 'r'
 exit_choice = 'e'
def find_num_of_char(name):
 return len(name) - name.count(' ') 
def reverse_name(name):
 return name[::-1]
def user_input():
 name = input("Enter your name: " )
 choice = input("Would you like to (r)everse your name\n or would you like to print the number of (c)haracters or (e)xit?: ")
 return name, choice
def clear_screen():
 print(chr(27) + "[2J")
def play_game():
 while True:
 clear_screen()
 name, choice = user_input()
 if choice == PrintChoices.reversed_name.value:
 print(reverse_name(name))
 input("Press any key to continue...")
 elif choice == PrintChoices.number_of_char.value:
 print(find_num_of_char(name))
 input("Press any key to continue...")
 elif choice == PrintChoices.exit_choice.value:
 break
 else:
 input("Invalid input, Press any key to continue...")
play_game()

Note: I have also added print(chr(27) + "[2J") to clear the screen.

• ZeroDadeCoolMurphy: Wow, your answer was more instructive than hours I've spent watching YouTube videos and reading blog posts on Python. Really appreciate the thoroughness....super insightful for me.
• user228914: I am so happy that you found it useful
• Cireo: As a nitpick, perhaps len(name) - name.count(" ") over len(name.replace(" ", ""), no need for a new string
• abdusco: I'd move print(chr(27) + "[2J") into a separate function clear_screen because it wasn't clear until I read your explanation
• user228914: @KenY-N yeah i don't know why I added the continue there, It doesn't serve any purpose. About the input thing, for me, it works with any key, and not just ENTER

Answer 2

You can have a better structure. Here is detailed explanation. Data Flow Errors which you should not do in real world:

• Here, your function calls don't make sense. Whenever you define and use functions, define them in such a way that they don't call each other i. e. there should be tree structure. Main function will control all. (See Below Code to get glimpse.)
• Your script can have better structure. See Below Code.

*** PROTIP: If you want to make your code more maintainable, break it into modules and put all of them in different files and import them ***

def take_name_input():
 print('Enter your name:')
 n = input()
 return n
def ask_choice():
 print('Hello, ' + n + ', would you rather (R)everse or (C)ount the letters of your name?')
 a = input()
 return a
def playgame(myname):
 while True:
 choice = ask_choice()
 if choice == 'R':
 print(''.join(reversed(myname)))
 elif choice == 'C':
 for c in list(myname):
 if c == ' ':
 spaces = spaces + 1
 print('Total # Characters: ' + str(len(myname) - spaces))
 else:
 break
name = take_name_input()
playgame(name)

Answer 3

I think the structure is quite convoluted, with calls to input() and choosef() scattered in multiple places/levels, thus the program is not super readable and not scalable (if you had to handle more user inputs).

A more readable approach would be a main loop like - in pseudo code

while True:
 Get user input from 3 choices R / C / E(xit)
 if input = E:
 exit
 else if input = R:
 handle R case
 else 
 handle C case

Also in python there is a shorter way to reverse a string with slicing syntax: "abc"[::-1]

• ZeroDadeCoolMurphy: Looking back at my original program, it is looking more like an exercise in deliberate code obfuscation than an honest attempt at structuring a simple program. I suck.
• Spratty: @ZeroDadeCoolMurphy - don't despair; we all have to learn. I'd been a professional programmer for years when I switched from COBOL to Visual Basic and I honestly think I'd be torn between shuddering and laughing out loud if I went back and looked at my first efforts in VB now (and it was production code too! Bonus ego-hit!); we've all been there. We learn and improve, and eventually you'll be back here helping others with exactly the same sorts of issues you're having now. If your code actually did what you wanted you're already ahead of an awful lot of programmers I've worked with.
• ZeroDadeCoolMurphy: Yep, my original language is 'q" (from kdb+), which has it's own very distinct idiomatic approach to clean programming. The switch to Python has been trying at times.

Answer 4

There are two area's where your code could improve further.

First you can run tools like flake8 or pylint to check your code for 'violations' of PEP8. PEP8 is a Python style guide that is accepted as the de facto style for Python code. I won't focus on this since this mostly an automatic process and a somewhat personal choice.

Secondly you can make some improvements to your code with regards to naming and documentation. This is what we call the readability of your code. I've taken a shot at this myself and this is what I (personally) find to be better code.

One thing you can improve is removing all magic from your code. In the two lines below you set a variable ui and then pass the first and second element to another function. If I see these two lines, I have no idea what you're doing.

ui = userinput()
choosef(ui[0],ui[1])

This can be simplified and made more readable by selecting better variable names and (in this case at least) using tuple unpacking:

action, name = get_user_input()
handle_user_action(action, name)

The example above can be applied throughout your code. I always judge my code (and the code I review) with the question: "If I show this block of code to someone, is the intention of the code clear to the person reading it, without requiring knowledge of the entire program?"

• ZeroDadeCoolMurphy: I actually did try to implement your last suggestion, but I must have screwed up the syntax because it kept giving me an error. Thanks for getting me back on the right track.