1

In this answer it is shown how to automatically set as attribute the kwargs/args passed to a class __init__

For example, one could do:

class Employee(object):
    def __init__(self, *initial_data, **kwargs):
        # EDIT
        self._allowed_attrs = ['name', 'surname', 'salary', 'address', 'phone', 'mail']

        for dictionary in initial_data:
            for key in dictionary:   
                if key in self._allowed_attrs:  # EDIT
                    setattr(self, key, dictionary[key])
        for key in kwargs:
            if key in self._allowed_attrs:  # EDIT
                setattr(self, key, kwargs[key])

In my case I already know in advance the arguments I am going to pass, so I am considering this solution only to have a less repetitive and shorter code.

Is this considered good practice? Which are the pro/cons of this solutions against manually initialise each attribute? Is there any other preferable approach?

EDIT: As the first comments/answers (rightly) focus on sanitizing arguments or listing arguments, I think this can be solved quite easily in this framework.

Community
  • 1
  • 1
FLab
  • 7,136
  • 5
  • 36
  • 69
  • 2
    From the [Zen of Python](https://www.python.org/dev/peps/pep-0020/): `Explicit is better than implicit.`. You are requiring the caller to bundle the `*args` into a dictionary. – AChampion Apr 12 '17 at 12:26
  • Thanks for the comment, but I guess I would weight that against 'Beautiful is better than ugly' and 'Readability Counts': having e.g. 25 lines of 'self.something = kwargs['something']' looks nor beautiful or readable to me, but that's clearly personal taste – FLab Apr 12 '17 at 12:30
  • I would disagree with readability and I would point out `Sparse is better than dense`, `hard to explain` etc. On balance I would not favour this approach. – AChampion Apr 12 '17 at 12:33
  • `for dictionary in initial_data` smells fishy. `*args` is a list of arbitrary objects passed as arguments, not a list of dicts. So this will crash unless all the non-keyword arguments passed are dicts. – ivan_pozdeev Apr 19 '17 at 12:36

4 Answers4

3

Question: ... less repetitive and shorter code

Your example code needs, 9 Lines and 28 Keywords.

class Employee(object):
    def __init__(self, name, surname, salary, address, phone, mail):
        self.name = name
        self.surname = surename
        self.salary = salary
        self.address = address
        self.phone = phone
        self.mail = mail

This default one needs, 6 Lines and 19 Keywords. Summary, your example needs more not "shorter code". I can't see any "repetitive ... code" in the default one, all assignments are done once.

Compared these two lines, doing the same. Control which args can passed:

self._allowed_attrs = ['name', 'surname', 'salary', 'address', 'phone', 'mail']  

with

def __init__(self, name, surname, salary, address, phone, mail):

The second one need less effort and do it all in one.
No if key in self._allowed_attrs: necessary, as python does it for you.


In a real project, I would use something like this

class Employee(object):
    def __init__(self, person, salary=None):
        self.id = unique_id()
        self.person = person
        self.salary = salary

All person related data are to be summarized in object person.


Conclusion:
For your given example class Employee i would never use (*args, **kwargs).
(*args, **kwargs) arguments are only usefull if one can't predict which args are passed.

stovfl
  • 14,998
  • 7
  • 24
  • 51
1

It makes it impossible to figure out what kind of arguments the class expects.

If someone (or you in a few months' time) wants to create an Employee in their code, they look at the arguments of the constructor to see what they should pass (maybe by hand, or maybe an IDE automatically shows them). Your code achieves little except hiding this.

RemcoGerlich
  • 30,470
  • 6
  • 61
  • 79
1

Prior discussion: Python decorator to automatically define __init__ variables, Python: Is it a good idea to dynamically create variables?

Pros:

  • Reduces code duplication

Cons:

Community
  • 1
  • 1
ivan_pozdeev
  • 33,874
  • 19
  • 107
  • 152
0

It is perfectly ok to use the language introspection capabilities to reduce teh ammoutn of repetion and typing you have to do.

Of course, it is better if you are taking care to handle the attributes correctly, and even sanitize contents - so the best thing is to have either a decorator for the __init__ method, or the equivalent of it in the base-class __init__ that will do everything that is wanted: check if the passed parameters are ok for the specific class, and then use setattr to set their values within the instance.

I think the less magic way is to have a convention in your class hierarchy to declare the wanted parameters as class attributes. In that way, you could use those class attributes to document the expected parameters and their type, and keep the __init__ signature as *args, **kwargs and have your base-class init handle them all.

SQLAlchemy Base models do that - you specify the class attributes as special "instrumented attributes" and they are automatically assigned when called in __init__.

A simpler way would be:

_sentinel = object()

class Base(object):
    def __init__(self, *args, **kwargs):
        for attr_name, class_attr in self.__class__.__dict__.items():
            if isinstance(class_attr, type) and kwargs.get(attr_name, _sentinel) != _sentinel:
                attr_value = kwargs[attr_name]
                if not isinstance(attr_value, class_attr):
                    raise TypeError("Parameter {} is expected to be of type {}".format(attr_name, class_attr))
                setattr(self, attr_name, attr_value)


class Person(Base):
    name = str
    age = int
    phonenumber = Phone
    ...

This would require all parameters to the class to be passed as named parameters - but all of them would automatically be assigned to instance attributes, it would work, be documentable and safe. If you want to get even better, just define some fancy descriptor class to be your class attr value.

jsbueno
  • 99,910
  • 10
  • 151
  • 209