2

I'm trying to write a short class to represent an object stored on a remote server. I expect setting attributes of the object locally without setting them on the remote server will generally be a bad idea; it'll mean the local object no longer represents the remote one, without even being aware of that fact.

I had planned on using code like the below to handle this. It lets you shoot yourself in the foot if you want to set item.name, but assumes users will be sensible and not do so.

class Item(object):
    def __init__(self):
        self.name = get_item_name()

    def set_name(self, name):
        try:
            set_item_name(name)
        except:
            handle_error()
            raise
        else:
            self.name = name

However, I recently came across the @property decorator, which let me write the below:

class Item(object):
    def __init__(self):
        self._name = get_item_name()

    @property
    def name(self):
        return self._name

    @name.setter
    def name(self, value):
        try:
            set_item_name(value)
        except:
            handle_error()
            raise
        else:
            self._name = value

This has the advantage of still allowing attribute access using item.name, but also handling item.name = blah assignment.

However, while the second block of code offers (what seems to me to be) nicer behaviour, it hides a function call in attribute setting, and given the Zen's "Explicit is better than implicit" and the general idea that Python classes should not hide things from their users, does this make the second block of code less Pythonic?

me_and
  • 15,158
  • 7
  • 59
  • 96
  • The `try: something`/`except: raise` is completely redundant. You can dispense with it entirely. If an exception is raised in set_item_name it'll never reach the `self._name = value` line. – Martijn Pieters Jul 06 '12 at 09:33
  • @Martijn: That's what you get for trying to post minimal code :) rather than dumping the entire thing. The real thing has more exciting error handling there; I just wanted to make it clear that there was more than a simple function call going on there. – me_and Jul 06 '12 at 09:48
  • @me_and, Can you please explain why do you mean by "... it hides a function call in attribute setting..."? Check this topic: http://stackoverflow.com/questions/6618002/python-property-versus-getters-and-setters it is a good discussion about properties in Python and why and how to use them properly. – Zaur Nasibov Jul 06 '12 at 09:52
  • 1
    @BasicWolf: I mean when someone writes `item.name = blah`, it looks like what they're doing is a simple attribute set, but there's a function call hidden under the covers. I'll take a look at that link shortly… – me_and Jul 06 '12 at 10:09

1 Answers1

3

Writing code in OO languages is all about hiding things. When you write a method doSomethingComplex(), you basically create a new word in the language of your application which hides some complex operation from the developer who has to use the method.

While explicit is usually better than implicit that doesn't mean always. In your case, the problem is not in hiding the method call in the setter but in the fact that you're hiding the network. As soon as you try to hide the real, physical network from the user of a method, you need to make sure you handle the following cases correctly:

  1. The remote side suddenly dies
  2. The network drops packets (timeout on local side)
  3. The remote side throws an exception (you must somehow get it to the local code but it might be meaningless locally)
  4. The remote side successfully processes the call but the network drops the answer (so you did succeed but you don't know)

No matter how hard you try, this hidden complexity will leak through. As a simple example, developers using this code will have to handle network exceptions and timeouts (as opposed to any other method).

So the break here is less that you call a method in a setter but that you introduce a completely new contract.

What is a better solution? Make the network call explicit. That means more obvious effort for users of the code but they will always know what they're up against (instead of wondering why this setter behaves so oddly).

Suggested implementation: Use the command pattern to modify the state of these objects. The command needs the ID of the object to modify (so you don't have to keep a local copy), the field to change and the new value.

Update the local cached copy when the remote side replies "success". If the remote side returns an error, drop the cached copy so it gets fetched again.

This will create a lot more code but this code will be pretty simple and obvious. When you try to hide the remote side, you will have less code but it will be more complex and obscure.

PS: Never assume that people are sensible over an extended period of time.

Aaron Digulla
  • 321,842
  • 108
  • 597
  • 820