0

This is my current implementation:

def update_obj(obj_id, field1=None, field2=None):
    obj = ... # some code to get object by obj_id
    if field1 is not None:
        obj.field1 = field1   
    if field2 is not None:
        obj.field2 = field2

update_obj(1, field1='new value')
update_obj(2, field2='new value')
update_obj(3, field1='new value', field2='another new value')   

I try do the evaluation in one line, like:

obj.field1 = field1 if field1 is not None else ob.field1

However, the obj.field1 = obj.field1 is quite redundant and obj.field1 = field1 if field1 is not None is not a valid statement. Any suggestion to make it simpler and cleaner? I'm using python 2.7.

user3778914
  • 383
  • 1
  • 3
  • 12
  • 2
    I think your current code is perfectly idiomatic and you should keep it like it is. – RemcoGerlich Jan 20 '15 at 09:19
  • I'm afraid , you're using the best method – Juan Diego Godoy Robles Jan 20 '15 at 09:30
  • 1
    if it happens a lot maybe consider writing a func `set_if_None` – Udy Jan 20 '15 at 09:31
  • Consider changing the design. Making dozens of arguments that duplicate each field is rather awkward [and thus considered poor design](http://stackoverflow.com/questions/28443527/python-decorator-to-automatically-define-init-variables/28444093#28444093). Maybe accepting a `dict` and/or using `__setitem__`. Structuring the set of args and splitting the "god object" into multiple ones accordingly as suggested on the link may also be a good idea. – ivan_pozdeev Oct 05 '15 at 00:25

2 Answers2

1

If it's code duplication that bothers you, you may use reflection, e.g.:

obj.__dict__.update(n,v for n,v in locals() if n in 'field1','field2' and v is not None)
ivan_pozdeev
  • 33,874
  • 19
  • 107
  • 152
  • This is cool line! But it might not be that easy to read and maintain as the number of my fields of object is actually quite a lot. Thanks for sharing. – user3778914 Jan 20 '15 at 09:42
0

I'd use or.

obj.field1 = field1 or or obj.field1

It doesn't remove the redundancy, but it is a little more concise.

jlb83
  • 1,988
  • 1
  • 19
  • 30
  • 1
    It doesn't work when `field1` has a value that evaluates to boolean false, such as `0`, `[]`, etc. – interjay Jan 20 '15 at 09:11
  • Absolutely, if you want this to work for "False" values, then the OP's original code is probably what I'd use. – jlb83 Jan 20 '15 at 09:22