Stats classes for a text-based adventure

Question

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!

Answer 1

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')


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


def ask_for_integer(prompt_message):
    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)


def ask_for_stats(stat_name, max_points):
    message = '[{} points left] {} >>> '.format(max_points, stat_name)
    while True:
        value = ask_for_integer(message)
        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')
        value = ask_for_stats(statistic, max_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()

Answer 2

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.