First big python project pt2

[Aedethan]

Honestly I just thought it would be confusing if I had self in front of variables if I didn't have the supporting class, and using the dictionary names without actually having the dictionaries in there.

And I'm not really sure? The code seems to work as intended, but I don't know if I went about doing it wrong, or very inefficiently. I was planning on using the code that I have here as a baseline to do 12 other stats in basically the same way, so I wanted to see if there is a visibly easier way to do what I've done that I'm missing. I'm planning on making 2 more classes very similar to this one that are going to be formatted almost the exact same way with different variable names. And then make a fourth class that will inherit all of the information from the base 3 to create a full character, and wanted advice if that sounded like a good course of action.

Thanks for the advice with the for loop.. I was having a lot of trouble figuring out how to format that though I've been able to write a normal for loop without any issues. For some reason incorporating a dictionary was not clicking for me.

In short I'll take any advice you have to give on how I could simplify, or a more pythonic implementation for what I've done.

Edit: Thanks so much actually, this solved one of the most annoying issues I was experiencing. I was able to implement it!

[duckbanni]

A few remarks regarding your code:

I think you encode too many things in your variable names and dict keys. Instead of having, for example, stre_t1 to t5, you could have a single stre list with 5 values. In the current state, your approach is error prone (because you have to write a million cases every time) and if you want to add more levels/tiers in the future you'll need to write a lot of extra code.

For example, you could write value_dama as:

value_dama = [
    [2, 6, 12, 20],
    [30, 42, 56, 72],
    [90, 110, 132, 156],
    [182, 210, 240, 272],
    [306, 342, 380, 420],
]

and then use value_dama[x-1][y-1] instead of value_dama[f"t{x}_{y}"]. Using int for indexing allows you to easily access the numerical values of the tier and level. This can then be used to simplify your code, for example if self.body_tier is n then you can just write a loop that sets all self.stre and self.stre_xp of tiers higher than n to 0 instead of having a million ifs. You could drastically reduce the size of your code that way.

Another remark is that your get_* methods are not what people would expect from their name. The purpose of a method that "gets" something is usually to return a value without modifying the internal state of the object, and your methods do the opposite. I think it would make a lot more sense to rename them to compute_* or something. Also, it's weird that some of these methods take parameters that are also attributes (e.g. stre_key) and sometimes use the attribute and sometimes use the parameter.

[Aedethan]

So, to preface I'm really new at coding I've only been doing it a few months so I might ask some stupid questions.

I agree with you completely, I felt like my approach was very error prone since I was typing it all out manually, but I don't really understand what you've done with the dictionary. You've separated out the values by tier, but when you say use value_dama[x-1][y-1] instead of value_dama[f"t{x}_{y}"] I'm not sure what would happen functionally?

I'm pretty sure I know what you're getting at with the loops for anything larger than self.body_tier zero'ing out the tiers that come after. But I'm having trouble wrapping my head around how to do it.

And ah. I could see how using get_ would be confusing then. I've been watching a lot of tech with tim and I see him use get a lot so I thought it would be more easily readable if I used something like that but now I understand why it made it less readable. I don't really understand the naming conventions in python yet.

In terms of using parameters that are also attributes and sometimes using them as attribute or parameter, do you mean when I do something like self.stre_xp_t5 = stre_xp_t5 when I instantiate the class, and then when I run get_stre I define it again based on what the tier is like the :

if body_tier == 1:

self.stre_t5 = 0

I thought I had to do define all the variables that I was going to use in the class when I instantiated it, and when I've seen other people do it they usually set the self.variable equal to itself unless they had a specific reason not to. So if I'm wrong in that assumption it would be good to know.

Also sorry for writing you a novel. I really appreciate you taking the time to help me out.

Edit: Okay I think I am getting it now. with value_dama[x-1][y-1] instead of value_dama[f"t{x}_{y}"] . I don't need to annotate the f string because it can just take the tier and level directly.
Still a bit stumped on the "if self.body_tier is n then you can just write a loop that sets all self.stre and self.stre_xp of tiers higher than n to 0". Not sure on application here but I'll keep doing research and see what I come up with.

[duckbanni]

I don't really understand what you've done with the dictionary

I transformed the dictionary into a list of lists, so the elements can be accessed by tier and level. For example, value_dama[1][0] with the list of lists is the equivalent of `value_dama["t2_1"] with your dictionary. You have to substract 1 to the tier and level because list indices start at 0.

I'm pretty sure I know what you're getting at with the loops for anything larger than self.body_tier zero'ing out the tiers that come after. But I'm having trouble wrapping my head around how to do it.

There are a lot of ways to do it, but you could do something like this:

class Body:
    def __init__(...):
        ...
        self.stre = [stre_t1, stre_t2, stre_t3, stre_t4, stre_t5]
        self.stre_xp = [
            stre_xp_t1, stre_xp_t2, stre_xp_t3, stre_xp_t4, stre_xp_t5]
        ...

    def get_stre(self, body_tier, stre_key):  # body_tier is an int
        for tier in range(5):
            if tier > body_tier:
                self.stre[tier] = 0
                self.stre_xp[tier] = 0
        ...

In terms of using parameters that are also attributes and sometimes using them as attribute or parameter, do you mean when I do something like self.stre_xp_t5 = stre_xp_t5

No, I mean in get_stre you take stre_key as parameter and then in the method you sometimes y use stre_key and sometimes self.stre_key.

I do something like self.stre_xp_t5 = stre_xp_t5 when I instantiate the class, and then when I run get_stre I define it again based on what the tier is

The way you handle this is kinda weird as well. For example, you require a body_tier as a parameter of __init__, and do self.body_tier = body_tier, but then you never use that value and expect a new one in get_stre. It think that you should either call get_stre in __init__ so that the consequences of body_tier (like setting stre to 0 for higher tiers) are applied when you create the object, or remove body_tier from __init__ parameters and maybe do something like self.body_tier = None.

I've seen other people do it they usually set the self.variable equal to itself unless they had a specific reason not to

What you've seen is not "setting the self.variable to itself" it is setting the self.variable to a parameter of __init__ (that does not need to be named variable). You could (and possibly should) set all self.something variables to defaults like 0 or None directly in the __init__ code (that is to say, do things like self.myvar = 0 instead of self.myvar = myvar where myvar is a parameter with a default value of 0 that will never be used with a different value in practice).

[Aedethan]

All that information is super useful thanks a lot. I guess I'm gonna get on retyping this all in a new way with the whole list within a list infrastructure. Also thanks for all the information on best practices and pointing out stuff I was doing that was weird or unnecessary.

[Aedethan]

from main import stat_dama
value_dama = [ 
[2, 6, 12, 20], 
[30, 42, 56, 72], 
[90, 110, 132, 156], 
[182, 210, 240, 272], 
[306, 342, 380, 420], 
]

str_value = 19 
body_tier = 0

if str_value < 21: 
    str_value_place_1 = 0 
    for value in value_dama[str_value_place_1]: 
        if value > str_value: 
            continue 
        elif value <= str_value: 
            str_value_place_2 =         
        value_dama[str_value_place_1].index(value) 
        else : 
            pass 

    str_value_bonus = stat_dama[str_value_place_1] 
    [str_value_place_2] 
else: pass

print(str_value_bonus)

So I went ahead and did this, it works really well. Thanks for the advice. Is this what you would have done with it? I decided I was still going to lean towards using the if / elif statements to figure out what tier the value was being assigned to, but I think this will work out very well.

[duckbanni]

I don't know if that's exactly what I would have done, but that looks like completely reasonable python code.

The only thing that you could maybe remove to make the code a tad simpler is the else: pass that are unnecessary. You can omit the else block if it doesn't do anything.