# Stats classes for a text-based adventure

I'm learning Python by creating a text-based adventure.
I've put the stat creation and storage classes into separate files (modules?), and common functions (ValueError handling etc.) into another one as well.
Is this good practice?

import common_functions

# Maximum number of points. Adjust for balance later
statsMax = 26
stopMessage = "No. Stop. Try again."

# Class to store the stats of a character - both Players and NPCs

class statsStore():

def __init__(self):
self.S = 0      # Strength
self.P = 0      # Perception
self.E = 0      # Endurance
self.C = 0      # Charisma
self.It = 0     # Intelligence
self.A = 0      # Agility
self.L = 0      # Luck

# Class to ask players to customise their player stats, and display them.

class stats(statsStore):

def __init__(self):
super().__init__()
# Purely used for counting in the statsList function
self.x = 0

# Stats Functions

def statsFunc(self, stat, Text):
global stopMessage
global statsMax
if self.x == 0:
Limit = statsMax - 10
else:
Limit = statsMax - 20 - self.x
print("You have " + str(statsMax) + " points left.")
while True:
print(Text)
stat = common_functions.errorHandle_int(stat, ">>>")

if stat <= 10 and stat >= 1:
if (statsMax - stat) < Limit:
print(stopMessage)
continue
else:
statsMax = statsMax - stat
return stat
break
else:
print(stopMessage)

# What's Fallout?
def statsList(self):
global statsMax
self.statsDict = {"Strength": self.S,
"Perception": self.P,
"Endurance": self.E,
"Charisma": self.C,
"Intelligence": self.It,
"Agility": self. A,
"Luck": self.L}

self.attributes = [self.S, self.P, self.E,
self.C, self.It, self.A, self.L]

for statsTip, stat in self.statsDict.items():
self.statsDict[statsTip] = self.statsFunc(stat, statsTip)
self.attributes[self.x] = self.statsDict[statsTip]

# I need this block of code to make changes to the individual stats.
# Otherwise, printing self.S would output 0.
self.S = self.attributes[0]
self.P = self.attributes[1]
self.E = self.attributes[2]
self.C = self.attributes[3]
self.It = self.attributes[4]
self.A = self.attributes[5]
self.L = self.attributes[6]
self.x += 1

# print(self.statsDict)
# Printing a list looks neater
print(self.attributes)

# Stats Debug
Player1 = stats()
Player1.statsList()
print(Player1.S)
print(Player1.attributes)


The block of code where I set self.S = self.attribute[0] (and for the other stats as well) seems inefficient, but it doesn't change those variables otherwise, even though it is changed in the dictionary/list.
Is there a cleaner way of implementing this?

Here is the common functions module as well:

import sys
stopMessage = "No. Stop. Try again."

def checkIn_list(List, Check):
global stopMessage
while True:
Check = "".join(
[letter for letter in input(">>>") if str.isalpha(letter)]).lower()
if Check == "exit":
sys.exit()
elif Check not in List:
print(List)
print(stopMessage)
continue
else:
return Check
break

def errorHandle_int(value, message):
global stopMessage
while True:
value = input(message)
if value.title() == "Stop":
sys.exit()
try:
value = int(value)
return value
break
except ValueError:
print(stopMessage)

# test_list = ("yes", "no", "stop")
# test_input = 0
# checkIn_list(test_list, test_input)


General code review is also appreciated!

First things first: your code reads nowhere near Python code. You should read into PEP8 to properly format it.

Second, you should strive to avoid global as hard as you can as it easily make the code error-prone, hard to reason about, and hard to test. In your case, of the two global variables you use, stopMessage is read-only so it is much like a constant where the use of global is unnecessary. The second one, statsMax is trickier. For your simple use-case, it seems to work, but if you were to create a second player, statsMax would be 0 and it would be impossible. Instead, you should make it a parameter of some sort: the __init__ method of the stats class seems a good starting point:

class stats(statsStore):
def __init__(self, max_points=26):
...
self.max_points = max_points


There is also the fragmentation of the source code into two files. Even though it is good to consider at some point to be able to group related functionnalities under a namespace, you have too few functionnalities for it to be usefull. It even lead to the duplication of the stopMessage. Keep it simple for now with a single file.

The design of the stats class is also very fragile: the statsFunc method is not meant to be called outside of statsList and statsList itself have to be called for the stats object to be considered initialized. This is the role of a simple function, maybe a classmethod on the statsStore class itself. Moreover, statsStore is only there to be a collection of values: better use a namedtuple here.

One more thing that intrigues me is how you use some parameters. For instance stat in statsFunc or Check in checkIn_List. It is like you want to declare a variable before use, but you never do anything with the value passed as parameter. You don't need that. Just assign a value to a new name whenever you need a new variable, and don't pass unnecessary parameters around.

Lastly, instead of putting tests at the end of the file, you should wrap them under an if __name__ == '__main__' test so they don't get executed when you import your file (in a Python shell to test it, or in an other module).

Proposed improvements:

import sys
from collections import namedtuple

Statistics = namedtuple('Statistics', 'strength, perception, endurance, charisma, intelligence, agility, luck')

while True:
value = input(prompt_message)
word = ''.join(filter(str.isalpha, value)).lower()
if word in authorized_values:
return word
if word == 'exit':
print('Only one of the following values is allowed:', authorized_values, file=sys.stderr)

while True:
value = input(prompt_message)
try:
return int(value)
except ValueError:
if value.lower() == 'stop':
raise
print('An integer is required!', file=sys.stderr)

message = '[{} points left] {} >>> '.format(max_points, stat_name)
while True:
if 1 <= value <= min(10, max_points):
return value
print('The provided value is out of bounds, try again.')

def build_statistics(max_points=26):
statistics = dict.fromkeys(Statistics._fields)
for statistic in statistics:
if not max_points:
raise ValueError('character exhausted statistics points')
max_points -= value
statistics[statistic] = value
return Statistics(**statistics)

def main():
try:
stats = build_statistics()
except ValueError:
print('Could not build statistics, aborting.', file=sys.stderr)
else:
print('Statistics collected:', stats)

if __name__ == '__main__':
main()

• Sorry for the late comment. From what I see, the values inside are changed by turning it into a dictionary, changing the values, then turning back into a namedTuple. Would I need to do this every time I want to change values inside the namedTuple(such as leveling up certain attributes.) Secondly, if I wanted to incorporate more than just the stats of a player, such as their chosen class, do I need to add more field names to the namedTuple, or create more namedTuples, or should it be in a class? Final question, could I use a dictionary instead of a namedTuple? – Broken Rose Mar 19 '18 at 12:34
• If you need to update statistics, then yes, a regular class is better suited. And as far as putting all other caracteristics of a player in the same class or a Character class that would also contain the statistics… I'd go for the second, but it mostly depend of your use-case, that I’m not aware off. Your best option is to continue building your program and post the next version as a new question. – 301_Moved_Permanently Mar 19 '18 at 12:50

I have a few issues with your code. First, objects should always be valid. When you separate initialization and configuration, you create a seam into which bugs will surely crawl.

Next, names matter. Your naming, and orthography, are somewhat bizarre. I'd suggest that you change your stats and statsStore classes to Character.

I don't see any reason why the two should be separate classes. Instead, I think you need a single class with some helper functions. (Creating a class to "do something" is a Java-ism. Python very much isn't Java.)

Your attributes should be spelled out. Remember that writing code is much more about communicating with the next maintenance guy than with the compiler.

For use cases, I can see a couple. First, you might create a "character class" mechanism, with certain minimum and/or maximum attribute constraints. Second, you might have a "monster" mechanism, where it is interesting to specify some attributes, but the rest can be defaulted. So let's try those two things.

First, the "specify some attributes plus a default" approach:

monster = Character(strength=15, agility=14, default=12)


We can get that by updating the initializer in the base class:

class Character:   # formerly, statsStore

def __init__(self, *, default=10, strength=None, perception=None,
endurance=None, charisma=None, intelligence=None,
agility=None, luck=None):
"""Initialize a character. Set attributes to "default" unless
they are given explicitly.

"""
self.strength = default if strength is None else strength
self.perception = default if perception is None else perception
self.endurance = default if endurance is None else endurance
self.charisma = default if charisma is None else charisma
self.intelligence = default if intelligence is None else intelligence
self.agility = default if agility is None else agility
self.luck = default if luck is None else luck


Now, what about that character class thing, with the minimum/maximum attributes?

Well, there are a couple of ways to address that. First, how do we need it to work? I envision there is a "set your attributes" process, where the user can specify an attribute and a value. This will loop until they are satisfied:

ATTR_NAMES = ('strength', 'perception', 'endurance', 'charisma', 'intelligence', 'agility', 'luck')

def set_attributes(char, points):
"""Interact with the user. Set attributes, up to maximum points.

"""

done = False
while not done:
print("Stats: ", end='')
for name in ATTR_NAMES:
print("{}={}".format(name[:3].upper(), getattr(char, name)),
end=' ')
print("\n\nYou have {} points remaining to increase attributes"
.format(points))

# FIXME: Maybe validate entire character here, to decide if
# 'done' is allowed.

if points == 0:
cmd = input("Enter 'done' or 'NAME=VALUE' to set an attribute: ")

if 'done'.startswith(cmd.lower()):
done = True
continue

else:
cmd = input("Enter 'NAME=VALUE' to set an attribute: ")

(attr, value) = cmd.split('=')
attr = attr.strip()
value = value.strip()

for name in ATTR_NAMES:
if name.startswith(attr.lower()):
attr = name
break
else:
print("'{}' is not a valid attribute name.".format(attr))
print("Valid attribute names are: ", ', '.join(ATTR_NAMES))
continue

try:
value = int(value)
except:
print("Please provide an integer value. '{}' is not valid".format(value))
continue

cur_value = getattr(char, attr)
point_cost = value - cur_value
if point_cost > points:
print("You don't have enough points for that!")
continue

# FIXME: Maybe check attr/value pair here.

setattr(char, attr, value)
points -= point_cost

player = Character()
set_attributes(player, points=26)


What about maximum/minimum values? I'll leave that up to you. You could implement a .check_attributes() method on various classes (Fighter.check_attributes(), Mage.check_attributes(), etc.) or you could pass in a dictionary of min/max tuples, or pass in a validation function and call it each time. I've marked a couple of likely spots with #FIXME where you could do validation.

What about different races? In some games, there are bonuses to stats based on race. You could do that in a couple of ways. The most obvious would be to just increase/decrease the "base" attributes before the point allocation process starts. Instead of coming in with 10,10,10 the character would come in with 10,12,9 or something. Another choice would be to apply them as a bonus after the creation process, but I don't like this because it puts the user in the position of having to remember the bonuses and keep doing math in their head.

What about increasing the costs? Should changing STR=12 to STR=15 only cost three points, or should it start costing more once you go past 14? That's not that hard, it just requires a lookup table. Instead of computing the point_cost as value - cur_value, you would compute the sum of a list of attribute costs, from cur_value + 1 to value + 1:

point_cost = sum(attr_points[value+1:cur_value+1])


(Or, going down, a slightly different slice with -1 step.) You'd have to initialize the attr_points list to whatever the incremental costs were.

• The set_attributes function you suggested requires the user to input in terms of STATNAME = VALUE. Wouldn't this be more prone to bugs than simply asking the user to input in a number for a specified stat? Is there a gameplay element I'm missing here? – Broken Rose Mar 19 '18 at 12:57
• @BrokenRose IMO, yes, there's a gameplay element. I expect that a player might want to "tweak" attributes up or down during creation, rather than expecting them to "just know" what the values should be. – Austin Hastings Mar 19 '18 at 13:27