CODEWITHSUNDEEP
python
Violet
Violet

Reputation: 505

A better way to do this?

I am writing a python function with the following:

class myObj(object):
    def __init__(self, args):
        # there is code here
    def newO(self, name, description):
        if type(name)==str:
            self.oname.append(name)
            self.o.append(description)
        elif type(name)==list:
             for n in name:
                self.oname.append(n)
                self.o.append(description)

However I am led to believe this is not the best way to accomplish this. Whats a better way? I want to be able to call newO with a string or a list.

Upvotes: 1

Views: 88

Answers (1)

Josh Lee
Josh Lee

Reputation: 177574

  1. Never check type(x) == foo, use isinstance(x, foo) — the former will break if there is a subclass, for instance.

  2. You appear to be maintaining parallel lists. If the order matters, it might make more sense to use a list of tuples instead, so self.values.append((name, description)). If the order does not matter, a dictionary would be better: self.values[name] = description (with the assumption that you don't want duplicate names).

  3. Single letter variables are a no-no.

  4. If you want to call newO with a string or a list, what would the disadvantage be of splitting it into two functions? Perhaps (and I might be wrong) you come from a language with compile-time polymorphism, where a string and a list would be dispatched differently. In reality they are treated as different functions, so in a dynamic language you might split them up like this:

    def add(self, name, description):
    ...
def add_many(self, name_list, description):
    for name in name_list:
        self.add(name, description)

    It's possible that you don't know a priori whether you have a string or a list, but I hope that's not the case.

  5. Is the case of a single string really different from the case of a list of strings? Why not just use a one-element list, to remove the conditional? You could use varargs as well, to make the syntax more bulletproof:

    def add_names(self, description, *name_list):
    for name in name_list:
        self.add(name, description)

Upvotes: 4

Related Questions