HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Moody game characters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
moodycharactersgame

Problem

I've been doing some self-imposed exercises to get accustomed to Python and below is my code.

import random
from random import randint
from random import randrange
from mainlist import legendaryitems
from mainlist import waystodie
from mainlist import powerup
from mainlist import mood
from mainlist import ponder
import time

class Player(object):
    def __init__(self, name):
        self.name = name
        self.lifecondition = True
        self.hitpoints = 100
        self.items = []
        self.waystodie = []
        self.mood = ""

    def death(self):
        self.lifecondition = False

    def ways_to_die(self, death):
        self.waystodie.append(death)

    def  add_items(self, item):
            self.items.append(item)

    def change_hp(self, hp):
        self.hitpoints = hp

    def change_mood(self, mood):
        self.mood = mood

    def __repr__(self):
        return repr(self.name)

def addplayers(alist):
    players = []
    for item in alist:
        item = Player("%s" % item)
        players.append(item)
        print "%s enters the game with %s HP of life and feels pretty much alive" %  (item.name, item.hitpoints, )
    return players      

def player_mood(playerlist, moodlist):
    for item in playerlist:
        mood = random.choice(moodlist)
        item.change_mood(mood)

playerlist = ["Player1", "Player2", "Player3", "Player4", "Player5", "Player6", "Player7", "Player8", "Player9", "Player10"]

players = addplayers(playerlist)
player_mood(players, mood)

print players[1].mood
print players[2].mood

player_mood(players, mood)

print players[1].mood
print players[2].mood


My intention is to create actions depending on the mood of characters. Basically it's gonna be a "There can be only one!" game. The reason of my post is not so that someone else can write the code for me but for feedback on how to write more clean, efficient code following Python's philosophy at the same time.

I am calling the instances through the players list b

Solution

Note that you can significantly simplify the import statements (two of which you never even use):

import random
import time

from mainlist import legendary_items, mood, ponder, powerup, ways_to_die


I have also arranged and renamed them as recommended by the style guide.

The methods in your class aren't consistently named. For example, ways_to_die and add_items are named differently but do similar things; based on what they actually do I would have named them add_way_to_die and add_item, although as the lists they add to are public attributes you could just let the user append to them without the convenience functions. Similarly, death should probably be named kill - instance methods generally contain verbs, as they do things to the instance.

The change_hp and change_mood methods are more consistently named, but also redundant; why not let the user do e.g. player.mood = 'happy' rather than player.change_mood('happy')? If you need to control e.g. the range of valid values later, you can make it a property:

class Player(object):

    VALID_MOODS = ['happy', 'sad']

    @property
    def mood(self):
        return self._mood  # note "private by convention" leading underscore 

    @mood.setter
    def mood(self, mood):
        if mood not in self.VALID_MOODS:
            raise ValueError('Invalid mood: {}'.format(mood))
        self._mood = mood


playerlist (should be player_list, as above) can be built much more efficiently with a list comprehension, factoring out addplayers entirely:

player_list = [Player("Player{}".format(n+1)) for n in range(10)]


this makes it easier to change the number of players, and harder to accidentally leave one of the numbers out (e.g. [..., 'Player4', 'Player6', ...]) or make a typo (e.g. [..., 'Player4', 'Players5', 'Player6', ...]). If you still want to introduce them, you can easily do so:

for player in player_list:
    print ("{0.name} enters the game with {0.hit_points} HP of life " 
           "and feels pretty much alive").format(player)


Note the use of Python's implicit concatenation of strings to keep the line length under 80 characters, and str.format rather than the more old-fashioned % formatting.

You could also get rid of player_mood (which is not very well-named), for example by:

  • adding mood as an initialisation parameter (def __init__(self, name, mood):) and calling random.choice in the list comprehension;



  • adding mood as an optional parameter (def __init__(self, name, mood=None):) and calling random.choice in the __init__ method if mood is None:; or



  • providing a convenience function Player.set_random_mood to the class (this could be combined with 2.: if mood is None: self.set_random_mood()).



Python indexing is 0-based, so:

print players[1].mood
print players[2].mood


will actually show the mood attributes of 'Player2' and 'Player3'. Not a deal-breaker, but you should be aware of it!

You should generally write docstrings for your modules, classes, methods and functions; this provides useful information to anyone using your code (including you!)

Code Snippets

import random
import time

from mainlist import legendary_items, mood, ponder, powerup, ways_to_die
class Player(object):

    VALID_MOODS = ['happy', 'sad']

    @property
    def mood(self):
        return self._mood  # note "private by convention" leading underscore 

    @mood.setter
    def mood(self, mood):
        if mood not in self.VALID_MOODS:
            raise ValueError('Invalid mood: {}'.format(mood))
        self._mood = mood
player_list = [Player("Player{}".format(n+1)) for n in range(10)]
for player in player_list:
    print ("{0.name} enters the game with {0.hit_points} HP of life " 
           "and feels pretty much alive").format(player)
print players[1].mood
print players[2].mood

Context

StackExchange Code Review Q#86631, answer score: 4

Revisions (0)

No revisions yet.