43

What is the proper way to do error-checking in a class? Raising exceptions? Setting an instance variable dictionary "errors" that contains all the errors and returning it?

Is it bad to print errors from a class? Do I have to return False if I'm raising an exception?

Just want to make sure that I'm doing things right. Below is some sample code:

@property
def password(self):
    return self._password

@password.setter
def password(self,password):
    # Check that password has been completed
    try:
        # Check that password has a length of 6 characters
        if (len(password) < 6):
            raise NameError('Your password must be greater \
                             than 6 characters')

    except NameError:
        print 'Please choose a password'
        return False

    except TypeError:
        print 'Please choose a password'
        return False                                                                                                                                

    #Set the password
    self._password = password

    #Encrypt the password
    password_md5 = md5.new()
    password_md5.update(password)
    self._password_md5 = password_md5.hexdigest()
Improve this question
2
  • 1
    Too many things named password in that code. (the first function, the second function, and the argument to the second function.) pyflakes will be your friend. Commented Mar 26, 2010 at 20:51
  • @keturn: it is a pattern shown in the docs for property() function. Look at the code example in the docs that uses .setter decorator. Inside the setter method password is a local variable (parameter). Other names are in a different namespace (they belong to the class). Commented Jul 5, 2017 at 15:51

3 Answers 3

Reset to default
70

Your code is out of a context so is not obvious the right choice. Following some tips:

  • Don't use NameError exception, it is only used when a name, as the exception itself said, is not found in the local or global scope, use ValueError or TypeError if the exception concerns the value or the type of the parameter;

  • Don't print error messages. Raise meaningful exceptions with a meaningful error message:

    raise ValueError("password must be longer than 6 characters")

  • Returning a value from a setter is meaningless while assignment is not an expression, i.e. you cannot check the value of an assignment:

    if (user.password = 'short'): ...

  • Just raise an exception in the setter and let the code that set the property handle it.

Example:

class Test:

    minlen = 6

    @property
    def password(self):
        return self._password

    @password.setter
    def password(self, value):
        if not isinstance(value, basestring):
            raise TypeError("password must be a string")
        if len(value) < self.minlen:
            raise ValueError("password must be at least %d character len" % \
                                 self.minlen)
        self._password = value

Look also at this forms handling library, there the validators , here an example, are entities in their own: they can be set dynamically with higher control and less coupled code, but maybe this is much more than you need.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

@mg, Great post (and great initials!) overall, but I'm concerned about the example at the end. You seem to have made up a password decorator, the usage of which is pretty odd; do you mean @property and to name the first method password? Also, you'll have to use new-style classes if you want to use properties, i.e. change the first line of the example to class Test(object): or inherit from some other new-style class. (Even if you weren't using properties yet, you should use new-style classes.)
@Mike Graham: damn, tiredness could play bad jokes. you are right for the property naming, the exact form is the one used originally by ensare, i prefer the old good password = property(...) but i would be coherent with ensare without really knowing the new 2.6 syntax. The second observation is not true: properties can be used with old style classes
@mg, Oops, I was remembering wrong the fact that classes that implement the descriptor protocol (like property) must be new-style classes. There are still other subtle advantages to using new-style classes whenever possible.
NB for Python 3 basestring has been replaced by str
"you cannot check the value of an assignment" you can using the walrus operator := starting with Python 3.8
10

The standard way of signalling an error in python is to raise an exception and let the calling code handle it. Either let the NameError & TypeError carry on upwards, or catch them and raise an InvalidPassword exception that you define.

While it is possible to return a success/fail flag or error code from the function as you have done, it is not recommended - it is easy for the caller to forget to check the return value and have errors get lost. Besides you are returning a value from a property setter - this is meaningless in Python since assignments are not expressions and cannot return a value.

You should also never print a message for the user in your exception handling - what if you later want to use the function or class in a GUI program? In that case your print statement will have nowhere to print to. Logging an error to a logfile (using Python's logging module) is often helpful for debugging though.

Improve this answer

Comments

4

Generally, you should indicate errors that propagate by using exceptions. If you discover an error by something you just checked and you can deal with it immediately, there is no need to raise an exception.

In the particular case of a setter, of example, returning False or anything else won't help. Setting instance variables you have to check is very suboptimal, since then you could miss an error on accident.

print is not usually a good response to an error. In this case, it sounds like you want to tell the end user they need to use a different password. It sounds like you should call a method that causes the webpage with the form to explain to the user what went wrong; you could call the method that does that in your class or raise an exception that will propagate and eventually be caught and used for that purpose. (This is general advice. I don't know enough about Pylons to tell you how it wants you to do this.)

You should not raise your own NameError exceptions. NameError prettymuch always indicates a typo in your program, and as such you usually do not want to catch it. By catching it, you introduce unnecessary uncertainty into a program. This looks like it might be something more like ValueError or a subclass thereof (class InvalidPasswordError(ValueError): pass).

I do not understand why you check TypeError. You should always understand what would have caused an exception you caught. If you do in this case, that's great; I can't figure out what error would raise TypeError that you could sanely deal with by prompting the user.

Your technique of receiving the password in plaintext and storing its md5 hash is not very secure. You should look into something like AuthKit that could make this process more secure and abstracted.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.