Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was wondering if this script in Python could be shortened?

import random

def var():
    strength = 10
    skill = 10
    dice4 = 0
    dice12 = 0
    dice_score = 0
    character_name = ""
    attr_list = [character_name, strength, skill]

    character_name = str(input("Please enter your characters name: "))
    skill_func(strength,skill,dice4,dice12,character_name,dice_score,attr_list)

def skill_func(strength,skill,dice4,dice12,character_name,dice_score,attr_list):
    print(character_name + "'s attributes are being generated! ... ")

    dice4, dice12 = random.randrange(1,4),random.randrange(1,12)

    dice_score = dice12//dice4
    skill+= dice_score

    print("Skill: ", skill)
    strength_func(strength,skill,dice4,dice12,character_name,dice_score,attr_list)

def strength_func(strength,skill,dice4,dice12,character_name,dice_score,attr_list):
    dice4, dice12 = random.randrange(1,4),random.randrange(1,12)

    dice_score = dice12//dice4
    strength+= dice_score

    print("Strength: ", strength)
    file(attr_list, character_name)

def file(attr_list, character_name):
    file = open("attributes.txt", "a")
    file.write(character_name + " - Strength = " + str(attr_list[1]) + ", Skill = " + str(attr_list[2]) + "\n")
    print("Your scores have been wrote to a file. ")


var()
share|improve this question
add comment

1 Answer

up vote 5 down vote accepted

You can start with multi-assignment:

strength = skill = 10
strength, skill = 10, 10
strength, skill = (10,10)
strength, skill = [10]*2

I prefer the first way as long as the thing you are assining is immutable / constant literal. If you are assigning two empty lists to two vars, and they need to be distinct, don't do the first way.

Now ask yourself why do you need to repeat the random number generation and algebraic operations (addition and division) in skill_func and strength_func. I am not going to figure out the reason for you, but if they can be reused, then eliminate the repeated lines from strength_func since skill_func will call strength_func.

If they have to be repeated, you can create another function called get_dice_score just to do the random number generation and return the dice_score so that *_func can call the new function. Example:

def skill_func(...):
   // do stuff
   dice_score = get_dice_score(...)

def strength_func(...):
   // do_stuff
   dice_score = get_dice_score(...)

Finally, both strength_func and skill_func are so similar. They have almost, if not, exactly the same function signature. They do almost the same thing. Can we combine them?

Idea:

def skill_and_strength(strength, skill, ......):

    dice_score1 = get_dice_score(..)
    dice_score2 = get_dice_score(..)
    strength += dice_score1
    skill += dice_score2

This works as long as skill and strength don't depend on each other.

Modularity only makes sense if they do distinct task. If they share most of the logic, you extract the common logic into separate function(s). If they call similar signature and they are usually used together, you can make a general function with the long signature and then "dispatch" to the more specific function.

Idea (may not be applicable):

def say_stuff(function, text):
    return function(text)

def say_hello(name):
    print "Hello, " + name

def say_goodbye(name):
    print "Goodbye, " + name

def main():
    say_stuff(say_hello, "John")
    say_stuff(say_goodbye, "Ben")
    say_stuff(say_goodbye, "Alice")

This works because function and classes are objects! I pass the function object and inside say_stuff I invoke in callable manner. This is one way to dispatch.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.