0

(The following is python3-related (if that matter).)

This this the code I've written (simplified) :

class MyClass:

    def __init__(self):
        self.__some_var = []

    @property
    def some_var(self):
        return self.__some__var

    @some_var.setter
    def some_var(self, new_value):
        if hasattr(new_value, '__iter__'):
            self.__some_var = new_value
        else:
            self.__some_var.append(new_value)

I want to replace when setting if their is several "values" (i.e if new_value is an iterable of, in my case, non-iterable objects) and appending if their is only one "value". I'm concerned about the performance of hasattr so I wonder if I shouldn't use this setter instead :

    @some_var.setter
    def some_var(self, *args):
        if len(args) > 1:
            self.__some_var = args
        else:
            self.__some_var.append(args)

Thank for your attention !

thomas
  • 1,855
  • 4
  • 17
  • 19
  • Premature optimization anyone? Do a simple benchmark and see if the difference is even measurable. If anything, I expect `hasattr` to be faster. – intgr Nov 19 '09 at 14:12
  • @thomas, in your first form, are you **sure** that you want to "replace" when `new_value` is, say, `"thomas"`? Strings have `__iter__`, and are sequences, yet almost invariably in code we want to treat them as scalars instead. (The second form has drastically different semantics, of course, so it will always "replace", even when called with a list). – Alex Martelli Nov 19 '09 at 15:50
  • In fact, their a test just before the affectation if the new_value's values() element has some method, you can just pass object that behave specifically to this function, or it raise an error. – thomas Nov 20 '09 at 15:11

3 Answers3

4

There's no "performance cost" of hasattr that matters. It's fast enough that you would have a hard time measuring it.

Please do not use __ (double underscore) for your own attributes. It's confusing to the rest of us.

It's usually best to use the collections Abstract Base Class membership for this kind of thing.

if isinstance( arg, collections.Sequence ):
    self._some_var = list(arg)
else:
    self._some_var.append( arg )

This gives you something that will likely work better because it expresses the semantics a little more clearly.

S.Lott
  • 384,516
  • 81
  • 508
  • 779
  • I believe, and I might misunderstand, that isinstance is a bad practice in general, and that Python's way is duck-typing. Furthermore I once had issues with isinstance and issubclass just because of different importation so, I prefer to avoid them. – thomas Nov 19 '09 at 14:22
  • 1
    In your case, you need to know whether it's a collection or not. len('aaa') > 1, too. – Aaron Digulla Nov 19 '09 at 14:28
  • 1
    Since you're making a distinction based on type (is it a sequence or not a sequence), then you have little choice but to ask `isinstance`. The alternative is to define your own subclass of collections.Sequence with your own method for updating MyClass -- that's the proper "duck typing" solution. Rather more complex than checking for subclass of Sequence. – S.Lott Nov 19 '09 at 14:36
2

An alternative might be duck typing:

Here I assume you want to evaluate (=make a list) out of the iterable, since it might be an iterator that you need to expand immediately to save.

@some_var.setter
def some_var(self, new_value):
    try:
        self.__some_var = list(new_value)
    except TypeError:
        self.__some_var.append(new_value)

Here we expect list() to raise ValueError if new_value is not iterable. As a response to your comment on S.Lott's answer, I don't think using hasattr is really duck typing style either.

u0b34a0f6ae
  • 48,117
  • 14
  • 92
  • 101
0

In terms of efficiency:

  • hasattr() would have a worst-case of O(1) (constant).
  • append() is also a worst-case of O(1) (constant).
AJ.
  • 27,586
  • 18
  • 84
  • 94
  • 1
    I suspect a small mixup. Hasattr is in principle a dict lookup (or more than one), so it's `O(1)`. – u0b34a0f6ae Nov 19 '09 at 14:30
  • @kaizer.se right - I was thinking that the entire list was being inspected to see if an element was already in the list. That would be O(n) worst-case if the element was not in the list, or if it was at the end of the list. I re-read the code, and what he's actually doing is checking to see if new_value is a collection itself. Makes more sense now. – AJ. Nov 19 '09 at 16:29