views:

147

answers:

3

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()
+6  A: 

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.

Dave Kirby
+3  A: 

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.

Mike Graham
+5  A: 

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 concern 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")
    
  • return a value from a setter is meaningless while assignment is not an expression, i.e. you cannot check the value of an assignement:

    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):
        retunr 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 an higher control and less coupled code, but maybe this is much more than you need.

mg
This was really helpful. Thank you.
ensnare
@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
@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
@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.
Mike Graham