Reusing validation logic in setter and constructor the right way

Question

I have a class with a property with a custom setter to perform validation. I would like to be able to pass the property as a constructor parameter too, and call the setter from the constructor to reuse the validation logic:

class Part(object):
    def __init__(self, pn):
        # self._pn = None
        self.pn = pn

    @property
    def pn(self):
        return self._pn

    @pn.setter
    def pn(self, value):
        if type(value) != str:
            raise Exception("Part number must be a string")
        self._pn = value  # warning here

On the last line, PyCharm gives this warning:

Instance attribute _pn defined outside __init__

If I uncomment the self._pn = None in the constructor then the warning goes away, but I don't like it, because it's pointless code: if an exception is raised, an object won't be created anyway, so what's the point setting an initial value?

Is this is a shortcoming of PyCharm / tooling, or am I doing it wrong? Is there a better way to achieve the same, without warnings, without pointless code, in a DRY way?

PyCharm is not wrong here. The warning is telling you that you're creating a variable, in runtime, on the object. The warning is supposed to alert you that you might have misspelled a variable's name, since this is something that *might* happen when writing python or you. I actually think the way you're writing this class is indeed not the right way to write python, since *EXPLICIT* is better than *IMPLICIT*. Remember, variables that start with `_` mean "only use if you know what you're doing." — Mahmoud Abdelkader, Nov 23 '14 at 00:27
@MahmoudAbdelkader if you turn your comment into an answer, I will upvote it — janos, Nov 24 '14 at 16:13

score 1 · Answer 1 · answered Nov 25 '14 at 03:07

PyCharm is not wrong here. The warning is telling you that you're creating a variable, in runtime, on the object.

The intention of this warning is to alert you that you might have misspelled a variable's name, since this is something that might happen when writing python.

I actually think the way you're writing this class is indeed not the right way to write python, since EXPLICIT is better than IMPLICIT (type "import zen" into your local python interpreter).

Remember, variables that start with _ mean "only use if you know what you're doing."

Javier Castellanos · Answer 2 · 2014-11-22T08:06:25.147

0

This has the same behavior and shouldn't trigger the warning.

class Part(object):
    def __init__(self, pn):
        self.pn = pn

    @property
    def pn(self):
        return getattr(self, '_pn')

    @pn.setter
    def pn(self, value):
        if type(value) != str:
            raise Exception("Part number must be a string")
        setattr(self, '_pn', value)

edited Nov 22 '14 at 08:06

answered Nov 22 '14 at 07:21

Javier Castellanos

9,346
2
15
19

No, I don't want to define a default value. – janos Nov 22 '14 at 07:26
then what is the purpose of having the offending line? – Javier Castellanos Nov 22 '14 at 07:38
The purpose of the offending line = the purpose of the setter, is to update the value of `pn` later. – janos Nov 22 '14 at 07:43
Why are you trying to set up _pn to none and then immediately to the passed argument? Why this behavior? – Javier Castellanos Nov 22 '14 at 07:45
Notice that the line is commented out. I'm trying to *avoid* that. I explained this in the question. – janos Nov 22 '14 at 07:49
Sorry, I thought it was the other way around. What if you use setattr? – Javier Castellanos Nov 22 '14 at 07:59

score 0 · Answer 3 · edited May 23 '17 at 12:08

0

There isn't any thing syntactically wrong with what you're doing. I'd suppress this particular warning.

edited May 23 '17 at 12:08

Community

1
1

answered Nov 22 '14 at 07:24

generalpiston

911
7
11

I'd like a stronger justification before I go on and suppress warnings. – janos Nov 22 '14 at 07:30
Python interpreters should parse the entire class before it can be instantiated. The '@' sign before methods implies \@property and \@pn.setter are decorators, which means they will also be parsed before instantiation occurs. This means your setter will be available. Essentially, PyCharm hasn't picked this up. I'd file a bug report if possible. – generalpiston Nov 22 '14 at 07:40

Reusing validation logic in setter and constructor the right way

3 Answers3

Linked