Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Design considerations

#Design considerations ThereThere are a few higher level design considerations that will make your life a lot easier if you decide to make a more complicated game or go to a GUI approach. ##Input/output One

Input/output

One thing I see is that you don't have a clear separation of the input/output and the game logic. This will make it considerably harder to move to say a GUI approach in the future.

Lack of functions

##Lack of functions There'sThere's a noticeable lack of functions here. An especially good example is the menu handler, with the large change of else-if statements:

Mutable global state

##Mutable global state ThisThis can turn into a particularly nasty maintenance burden as your codebase grows in size. It makes the flow of the code harder to follow and reason about. This is because having global state that can be change from anywhere makes it much harder to track down which parts of the code can change other parts of the code.

Python usage

#Python usage InIn some places you are comparing against None and other "truthiness" values. This is not a recommend practice. For example

#Design considerations There are a few higher level design considerations that will make your life a lot easier if you decide to make a more complicated game or go to a GUI approach. ##Input/output One thing I see is that you don't have a clear separation of the input/output and the game logic. This will make it considerably harder to move to say a GUI approach in the future.

##Lack of functions There's a noticeable lack of functions here. An especially good example is the menu handler, with the large change of else-if statements:

##Mutable global state This can turn into a particularly nasty maintenance burden as your codebase grows in size. It makes the flow of the code harder to follow and reason about. This is because having global state that can be change from anywhere makes it much harder to track down which parts of the code can change other parts of the code.

#Python usage In some places you are comparing against None and other "truthiness" values. This is not a recommend practice. For example

Design considerations

There are a few higher level design considerations that will make your life a lot easier if you decide to make a more complicated game or go to a GUI approach.

Input/output

One thing I see is that you don't have a clear separation of the input/output and the game logic. This will make it considerably harder to move to say a GUI approach in the future.

Lack of functions

There's a noticeable lack of functions here. An especially good example is the menu handler, with the large change of else-if statements:

Mutable global state

This can turn into a particularly nasty maintenance burden as your codebase grows in size. It makes the flow of the code harder to follow and reason about. This is because having global state that can be change from anywhere makes it much harder to track down which parts of the code can change other parts of the code.

Python usage

In some places you are comparing against None and other "truthiness" values. This is not a recommend practice. For example

Fixed code mistake
Source Link
shuttle87
  • 2k
  • 14
  • 19
def create_new_character(character_data):
 """Create a new character"""
 new_character = Character(*chardata)
 new_character.stats()
 return new_character

if menu_choice = "new":
 new_character = create_new_character(["Foo", 300])
 print ("New character created!")
def create_new_character(character_data):
 """Create a new character"""
 new_character = Character(*chardata)
 new_character.stats()
if menu_choice = "new":
 new_character = create_new_character(["Foo", 300])
 print ("New character created!")
def create_new_character(character_data):
 """Create a new character"""
 new_character = Character(*chardata)
 new_character.stats()
 return new_character

if menu_choice = "new":
 new_character = create_new_character(["Foo", 300])
 print ("New character created!")
Source Link
shuttle87
  • 2k
  • 14
  • 19

#Design considerations There are a few higher level design considerations that will make your life a lot easier if you decide to make a more complicated game or go to a GUI approach. ##Input/output One thing I see is that you don't have a clear separation of the input/output and the game logic. This will make it considerably harder to move to say a GUI approach in the future.

For example:

class Character(object):
 def battle(self,exp):
 self.exp = self.exp + exp
 self.lvl = self.exp/100
 print ("Successful battle! {} experienced gained!".format(exp))
 self.stats()

You have 2 things going on here that really should be separate. You have the battle aspect that updates information about the character and you have some output handling. As it currently stands there is an implicit assumption in parts of your code that the game is a terminal based game because of where the input/output occurs.

These types of situations are much more nicely handled if you split the input/output out from the game logic.

So in this case:

new_character.battle(100)

Might become

old_exp, old_lvl = new_character.exp, new_character.exp
new_character.battle(100)
print ("Successful battle! {} experienced gained!".format(new_character.exp - old_exp))

Now if you move to a GUI approach you just change the print here to something else. This is a lot easier than having to track down the print statements scattered throughout the code.

##Lack of functions There's a noticeable lack of functions here. An especially good example is the menu handler, with the large change of else-if statements:

if menu_choice == "new":
 chardata = ["Foo", 300]
 new_character = Character(*chardata)
 print ("New character created!")
 new_character.stats()

I would start breaking out things like this into functions:

def create_new_character(character_data):
 """Create a new character"""
 new_character = Character(*chardata)
 new_character.stats()
if menu_choice = "new":
 new_character = create_new_character(["Foo", 300])
 print ("New character created!")

Again notice how I am separating out the input/output here from the other functionality. Step 1: create the character then step 2: deal with the output. I would do a similar refactoring with other cases in this chain of else-if's.

##Mutable global state This can turn into a particularly nasty maintenance burden as your codebase grows in size. It makes the flow of the code harder to follow and reason about. This is because having global state that can be change from anywhere makes it much harder to track down which parts of the code can change other parts of the code.

I would strongly recommend creating a class that manages the state of the game which contains the main game loop. This will pay off whenever you have issues you have to debug.

#Python usage In some places you are comparing against None and other "truthiness" values. This is not a recommend practice. For example

while loop_menu == True:
if menu_display == "new" and menu_error == None:

can be written as:

while loop_menu:
if menu_display == "new" and not menu_error:

I notice you quit the program by breaking out of the main game loop. If there is code after the thread loop you might not immediately quit here. If your intent is to quite immediately you can use something like:

elif menu_choice == "quit":
 print ("Quitting program...")
 import sys
 sys.exit()

This guarantees your code exits here.

lang-py

AltStyle によって変換されたページ (->オリジナル) /