Sign in to follow this  

Improving Readability & Simplicity

Recommended Posts

I'm looking for an easily readable way of modifying the output of a function.

 

I'm trying to make functions for a game where various actions in combat are determined the characters respective skills instead of physical traits or stats.

 

All skills are int variables between 0-100, and the oas (overall action score) variable is the same range. Also, if all skills/variables are the same value, then the oas will be too.  Different variables should contribute a greater percentage to the oas than others, depending on the type of action.

 

I also have a variable that isn't called yet in the function called style(combat style) which will eventually make certain skills count for more or less in determining the oas.

 

My idea is to make style a list value and include a condition that checks to see what values are inside style and modify what values contribute to the oas accordingly.

IE a character using a certain style might have the variable wpn_skl contribute a greater percentage to the value of the oas variable than otherwise.

 

Irregardless, if all variables == X then the oas should also == X.

 

I know a way of doing this. Replacing the float values in the example below with variables.

 

But declaring so many variables in each of my functions would be messy, and the new variables would make the readability worsen a lot.

 

I was hoping somebody could show me a better way.

 

Here is one of the functions I've written. They all work in the same way.

def attack(comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl, style):
    """helps determine effectiveness of attacks"""
    oas = (((wpn_skl * .20) + (wpn_Tskl * .55) + (comb_skl * .20) +\
    (arm_skl * .03) + (arm_Tskl * .02)) \
    / 1)
    return int(oas)

Edited by RidiculousName

Share this post


Link to post
Share on other sites

To reduce the number of variables in the attack function, I think you want a list for your values as well.

def create_skills(comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl):
    return [comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl]

It just folds a list around all skill values, so you can carry them around in a single variable.

 

 

The attack function can then pairwise multiply and sum two lists.

weights = [0.20, 0.20, 0.55, 0.03, 0.02]
skills = create_skills(comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl)

def attack(skills, style):
    total = 0.0
    for i in range(len(WEIGHTS)):
        total = total + skills[i] * weights[i]

    return int(total)

This one works, but all the "[i]" indexing is ugly. Luckily, there is "zip". It takes two lists, and pairwise combines elements from them, eg

zip([1, 2, 3], [10, 20, 30]) gives [(1, 10), (2, 20), (3, 30)]

 

We can use this in the attack function:

def attack(skills, style):
    total = 0.0
    for skl, wght in zip(skills, weights):
        total = total + skl * wght

    return int(total)

That looks much better, doesn't it?

If you don't understand what happens, add a few "print" statements so you can see the values retrieved from the lists, and the computations being done.

 

Python can do more than this, revisit this function after you've read about generators and the "sum" function.

 

 

Now I didn't touch your question about style, but perhaps that's not needed now. This "attack" function uses a list of skill values and a list of weights, and computes the oas from them.

If you modify skill values or weights before running the computation, or even completely swap to a different list of weights, you can affect the oas. Have a go at it!

 

 

One word of warning: Before you modify a list, make sure you make a copy first, if the original list should not change. Eg

skills_copy = skills[:] # Make shallow copy of the list.

Share this post


Link to post
Share on other sites

I would just have an object like "PlayerSkills" or just "Player" that contains the list of skills for a particular character and is passed to the attack function. The contents of attack would be basically the same, but it'd clean up how you call it.

 

Basically, the strength of an attack is based on the style, and who's doing it, and this makes that much clearer. I'd recommend this over Albeth's solution, because of that clarity, but you can still his technique to simplify the implementation of your attack function, if useful.

Share this post


Link to post
Share on other sites

I would just have an object like "PlayerSkills" or just "Player" that contains the list of skills for a particular character and is passed to the attack function. The contents of attack would be basically the same, but it'd clean up how you call it.

I agree, further improvement or integration is definitely useful.

The OP is however still using the "a separate variable for each value" idea, which makes the "list index represents a value" idea a major leap forward already.

 

Besides not being sure that the class concept was known by the OP, a second problem with classes is that the logical approach for beginners in classes is

class Skills:
    def __init__(self, comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl):
        self.comb_skl = comb_skl
        self.wpn_skl  = wpn_skl
        self.wpn_Tskl = wpn_Tskl
        self.arm_skl  = arm_skl
        self.arm_Tskl = arm_Tskl

which doesn't solve the "too many variables" problem. To solve this nicely in Python you'd need an ordered container accessible both by numeric index and by name. Interestingly, this doesn't exist in Python afaik. I have been programming Python for almost 20 years, and this is the first time I realized that, it's funny how that works :)

At this moment, I am really wondering why I never saw this as a problem in all those years.

 

 

I considered dictionaries, but they have basically the same problem as the class above.

 

The solution you aim at, is like

class Skills:
    def __init__(self, comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl):
        self.skills = [comb_skl, wpn_skl, wpn_Tskl, arm_skl, arm_Tskl]

    def attack(self, style):
        strength = ... # Compute attack strength with self.skills
        return strength

I agree with you it's more confined, but also more advanced.

 

However, even with a class wrapped around it, at the core, it's still list manipulation. Whether you do that inside or outside a "class" wrapper doesn't change the core solution. Dropping the entire "class" thing ensures the OP will understand it even if he/she hasn't seen classes, and it reduces the number of concepts to deal with, so focusing on list manipulation is easier (which is a leap already, in particular as I threw "zip", "iteration over list", and "unpacking a tuple while iterating" in too).

 

 

 

The weakest point in my solution is that something like "skills[2]" doesn't say what you get from the skills.

You can improve that by adding names for the index, like

COMB_SKL_INDEX = 0
WPN_SKL_INDEX = 1
WPN_TSKL_INDEX = 2
ARM_SKL_INDEX = 3
ARM_TSKL_INDEX = 4

print(skills[WPN_TSKL_INDEX]) # Much simpler to see what value you pull from the skills.

I added that, but in the end, I didn't actually need that, it didn't fit in the answer and I deleted it again.

Share this post


Link to post
Share on other sites

I think of more importance than the implementation of a function is having a the interface of this function be clear. So the problem is less too many variables and more too many function arguments. An object the more elegant solution to that.

 

Now I do agree that you describe an elegant and extensible way to implement this attack function. Having the skill weights separated from the computation like that is a flexible design. But If I used it I would have a segment that converted the player object to a list at the start of the function.

 

With regards to indexing and accessing by names, what you usually want to do is not index, but iterate. A dictionary in python does allow you to do this by allowing you to iterate over all the keys to a dictionary. So an alternate design here is to have a dictionary/map of skills to player skill value(or an object as described above), and a dictionary of skills to weight in the combat calculation; and iterate over both. Something like this:

skill_weights= {'combat': 0.20, 'weapon': 0.20, 'arm': 0.55}

def attack(player, style):
    total = 0.0
    for skill in skill_weights.items():
        total += player[skill] * weights[skill]

    return int(total)

(Warning: code for demonstration purposes only; I have not verified that it works or even parses)

Edited by King Mir

Share this post


Link to post
Share on other sites

(Warning: code for demonstration purposes only; I have not verified that it works or even parses)
In this case, your code does fail, ".items()" returns tuples (key, value), and then you get key errors all over the place.

 

Otherwise, Sounds like a good alternative.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this