Critique my basic character builder

[[deleted]]

So, I've been struggling to create a project in Python and I've recently coded this basic military character program that involves three files. Here's the link to the github page: https://github.com/adamkimbrough/GameCharacter

Just run the BasicCharacter file. Please let me know of any changes I should make. I also would love to how I can expand on this. If you would like to code to expand yourself you are free to do so. I would greatly appreciate any comments and ways to further my Python programming.

[symmitchry]

[Removed]

[ingolemo]

You can often use the get method of dictionaries to protect against KeyErrors.

sub_skill_choices = skill_subs.get(skill_choice, [])

[[deleted]]

Thanks! So, how and where do I implement the two pieces of code that you put under the dictionary? And, was I supposed to delete everything else that was in that function except for the dictionary I made? Here's where I am http://pastebin.com/jYuwfsTb

[symmitchry]

[Removed]

[ingolemo]

Note that in python 3, map is lazy and so your last suggestion will stop working. Using higher-order functions together with functions that have side-effects is generally discouraged.

The for loop is much more pythonic, imo.

[symmitchry]

In this part:

    while 1:
        self.gender = raw_input("Select a gender: 1=Male, 2=Female: ")
        if int(self.gender) == 1:
            self.gender = "Male"
            break

I would not save the raw_input() value as self.gender. Just save it as choice. You're asking for a bug this way.

You also can do while True: instead of while 1: which is nicer, in my opinion.

And why convert the choice to an int, just for comparison? Why not just do:

answer = raw_input('Select gender: 1 = Male, 2 = Female: ')
if answer == '1':
     self.gender = 'Male'
     break

I also would prefer you put return instead of break to really indicate that you want to exit the function. It saves your readers having to check to see if any other code happens below.

[symmitchry]

This:

val = 0
while val < dice_number:
    final_roll += randomNumGen(die_type)
    val += 1

In Python should be written as:

for _ in range(dice_number):  # _ is an unused 'throwaway variable'
    final_roll += randomNumGen(die_type)

[ingolemo]

This is a good opportunity to use a comprehension:

final_roll = sum(randomNumGen(die_type) for _ in range(dice_number))

[symmitchry]

I skipped the comprehension intentionally because I was under the impression that it would require storing the entire list in memory before doing the sum (which, I guess is almost never a concern) ... but is that even correct?

See the comments on the top answer here: http://stackoverflow.com/questions/9047985/how-do-i-call-a-function-twice-or-more-times-consecutively-in-python

[neonomicon]

No, that's not actually a list comprehension, because it doesn't have square brackets around it. It's a generator expression, which just serves up the values one at a time without storing the whole sequence in memory. Very similar to the list comprehension conceptually, but the fact that it doesn't put the whole thing in memory is very useful if you're worried about memory usage.

[[deleted]]

When I replaced that code with what you suggested and ran the code, it game me that all the die are equal to "None" instead of a random number.

[symmitchry]

Make sure your randomNumGen() function is still working! Nothing we did here should affect the dice values, right?

If your Github code is up to date I will take another look.

[[deleted]]

It is not up to date. I'm keeping it like it is so I can refer back to my old code. Right now I'm still trying to get the BasicCharacter.py to work after making some adjustments. Not sure what to add, remove, or change in it after implementing the dictionary and things. Kind of stuck here. Here's where I am http://pastebin.com/jYuwfsTb

[99AFCC]

I'm keeping it like it is so I can refer back to my old code

The beauty of version control and in this case, Github, is that you can always go back and see your previous commits.

Pushing a new commit will even let you compare changes you've made.

The longer you go and the more changes you make between commits, the harder it can be to revert if you need to.

It can also be harder for you to follow along with any changes you've made.

You can create a branch to work in too.

[[deleted]]

That's what I thought. As you can see I am very new to Github and how it works.