Why is the instance variable not getting changed?

Question

class Player:
    def __init__(self, Name, Age, Sex):
        self.Name = Name
        self.Age = Age
        self.Sex = Sex
        
        self.validate(Name,Age,Sex)
        

    @classmethod
    def validate(cls, Name, Age, Sex):
        if (not isinstance(Age,int)) or Age < 0:
            Age = int(input('Please input a valid integer greter than 0'))
            return cls(Name,Age,Sex)
            
        
    def showDetails(self):
        return f'Player name: {self.Name}. Player age: {self.Age}'


a = Player('Nail',-25,'F')
a.showDetails()

Output:

Please input a valid integer greter than 0 20
'Player name: Nail. Player age: -25'

The variable Age is not changing. It remains on its initial state.

Because your `validate` method is a class method, and returns a new instance of Player... that you never use. Instead of a class method, use a regular instance method and modify the instance variables Name, Age, Sex directly. — Gino Mempin, Jan 01 '23 at 23:48
What is the motivation in making `validate` a class method? It is an instance of the class that needs validation. — John Coleman, Jan 01 '23 at 23:49
Having variable names with leading capital letters implies these are classes, which they are not. — tadman, Jan 01 '23 at 23:49
Doing input inside a validation function is also highly irregular. You should return the result of the validation test, `True` or `False`, or perhaps `raise`. You should leave the input part to the caller. The way you're calling this could also lead to going deeper and deeper in the stack, indefinitely. — tadman, Jan 01 '23 at 23:50
`validate()` is a class method, meaning it does _not_ have access to the instance attributes. Why on earth did you make this a class method? — John Gordon, Jan 01 '23 at 23:50
Tip: If you have constraints on the inputs, define an error class you can `raise` and use that instead. — tadman, Jan 01 '23 at 23:51

score 0 · Accepted Answer · answered Jan 02 '23 at 03:57

You almost got it. As some comments point, validate can't have access to the specific member of an instance of the object because it is a class method. You can do instead

def __init__(self, Name, Age, Sex):
    self.Name = Name
    self.Age = self.validate(Age)
    self.Sex = Sex                                                                                  
                                                                                                        
def validate(self, Age):
    while (not isinstance(Age, int)) or Age < 0:
        Age = int(input('Please input a valid integer greter than 0: '))
    return Age

That said, if the validation is as simple as checking inequalities, you could just stick the operation inside of the constructor without the need for an auxiliary function. Something like this:

def __init__(self, Name, Age, Sex):
    while (not isinstance(Age, int)) or Age < 0):
        Age = int(input('Please input a valid integer greter than 0: '))

    self.Name = Name
    self.Age = Age
    self.Sex = Sex

The extra validation function can be useful if it was a standalone/separate function, especially since it has an input prompt. It should be called _before_ instantiating an object. As other comments also pointed out, having an input prompt as part of init is highly irregular. — Gino Mempin, Jan 02 '23 at 04:45

Gino Mempin · Answer 2 · 2023-01-02T11:50:48.207

The main problem with your code is that you are incorrectly using a class method, which doesn't have access to the instance variables that needs to be validated and replaced. Review the related post on the Difference between class and instance methods to understand what a class method is and what it's for.

Furthermore, your code

return cls(Name,Age,Sex)

instantiates a new, different object, different from the original one that was just instantiated with the invalid Age, and, as tadman pointed out in the comments, can lead to a continuous chain of instantiation, validation, instantiation, ... etc. which doesn't even modify the original instance. This is definitely not what you want.

The simple, yet a bit naive solution, would be to replace your class method with an instance method and then use a regular while loop to keep prompting for input until you get a valid Age, similar to what's demonstrated in JustLearning's answer:

In [4]: class Player:
   ...:     def __init__(self, Name, Age, Sex):
   ...:         self.Name = Name
   ...:         self.Age = Age
   ...:         self.Sex = Sex
   ...: 
   ...:         self.validate()
   ...: 
   ...:     def validate(self):
   ...:         while (not isinstance(self.Age, int)) or self.Age < 0:
   ...:             self.Age = int(input('Please input a valid integer greter than 0'))

But, while that certainly works, it's highly unusual and a bit of code smell to have an input prompt as part of instantiating a class. The better practice would be prompting for Age and getting all the other variables before instantiating the class, as part of a separate function. Your class should just act like a container for a Player's details and functions, and this would also make it much easier to write unit tests for the Player class.

Furthermore, doing int(input(...) can possibly raise a ValueError Exception if the input isn't a valid integer. Checking with isinstance would not properly handle non-integer inputs. See Asking the user for input until they give a valid response for better ways to validate.

My alternative solution would be:

In [22]: class Player:
    ...:     def __init__(self, Name, Age, Sex):
    ...:         self.Name = Name
    ...:         self.Age = Age
    ...:         self.Sex = Sex
    ...: 
    ...:     def showDetails(self):
    ...:         return f'Player name: {self.Name}. Player age: {self.Age}'
    ...: 

In [23]: def get_age():
    ...:     while True:
    ...:         try:
    ...:             Age = int(input('Please input a valid integer greater than 0 '))
    ...:         except ValueError:
    ...:             continue
    ...:         if Age < 0:
    ...:             continue
    ...:         else:
    ...:             break
    ...:     return Age
    ...: 

In [24]: Age = get_age()
Please input a valid integer greater than 0 -5
Please input a valid integer greater than 0 -16
Please input a valid integer greater than 0 notinteger
Please input a valid integer greater than 0 22

In [25]: a = Player('Nail',Age,'F')

In [26]: a.showDetails()
Out[26]: 'Player name: Nail. Player age: 22'

If somehow you really still want your class to do some validation, it should just raise some form of an Exception if it gets invalid input, and then let your outer code handle it.

In [32]: class Player:
    ...:     def __init__(self, Name, Age, Sex):
    ...:         self.Name = Name
    ...:         self.Age = Age
    ...:         self.Sex = Sex
    ...:         self.validate()
    ...: 
    ...:     def validate(self):
    ...:         if (not isinstance(self.Age, int)) or self.Age < 0:
    ...:             raise ValueError("Age should be int and >0")

In [36]: try:
    ...:     Age = get_age()
    ...:     a = Player('Nail', Age, 'F')
    ...: except ValueError:
    ...:     print("Cannot instantiate Player, invalid details")

Finally, since this answer is all about best practices, you should follow PEP8 Style Guide for Naming Conventions and use all-lowercase for function parameters and instance variable names:

class Player:
    def __init__(self, name, age, sex):
        self.name = name
        self.age = age
        self.sex = sex

Only use uppercase/CapWords style Age only if Age is a class.

Why is the instance variable not getting changed?

2 Answers2