0

I repeatedly find myself in a position where I'm writing specific django model instance fields into a list for various reasons (exporting to CSV, logging) and I'd imagine the same is true for many other people.

Generating a report could require traversing through foreign keys IF they exist. The larger the report, the more unreadable the code gets as I wrap attribute getter attempts in try/except blocks.

Optional foreign keys are also problems: item.optional_fk.optional_date.method()

for item in django_model_instances:
    try:
          date_created = item.order.date_created.strftime('%Y/%m/%d')
    except AttributeError:
          date_created = ''

    try:
        date_complete = item.order.date_complete.strftime('%Y/%m/%d')
    except AttributeError:
        date_complete = ''

    # perhaps more try/except...

    writer.writerow([
        item.optional_fk.optional_field.strtime('%Y'),
        item.optional_fk.method(),
        item.bar,
        date_created,
        # other attributes...
        date_complete,
        # other attributes...
         ])

As you have more columns to write the code starts to look like a monster.

I like the readability of using eval() wrapped in try/except but I read I should avoid eval like the plague.

Is using eval in Python a bad practice?

  1. There is almost always a better way to do it - trying to find a better way without writing too much code :)
  2. Very dangerous and insecure - the strings are hard coded
  3. Makes debugging difficult - True
  4. Slow - code is for generating reports, it can be slow.

.

def no_exceptions_getter(item, statement):
    try:
        return eval(statement)
    except AttributeError, e:
        log.debug(e)
        return ''

for item in django_model_instances:
    writer.writerow([no_exceptions_getter(item, x) for x in (
        'item.foo',
        'item.bar',
        'item.date_created.strftime("%Y/%m/%d")',
        'item.date_complete.strftime("%Y/%m/%d")',
        'item.optional_foreign_key.foo',
        # more items in a readable list format
        )])

I don't see the security vulnerability, speed or debugging problems being an issue. So my question for you experts out there is: is this an OK use of eval?

Community
  • 1
  • 1
Yuji 'Tomita' Tomita
  • 115,817
  • 29
  • 282
  • 245

3 Answers3

3

Why aren't you just using getattr?

for item in django_model_instances:
    date_created = getattr(item.order, 'date_created', '')
    if date_created:
          date_created = date_created.strftime('%Y/%m/%d')

or a simple wrapper, if this particular pattern is used a lot:

def get_strftime(object, attr):
    value = getattr(object, attr, None)
    if value is None:
        return ''
    return value.strftime('%Y/%m/%d')

writer.writerow([
    item.foo,
    item.bar,
    get_strftime(item.order, 'date_created'),
    get_strftime(item.order, 'date_complete'),
])
Glenn Maynard
  • 55,829
  • 10
  • 121
  • 131
  • Well that still splits the list items so it's equal to try/except in readability. `getattr(item, 'optional_fk.optional_field.blah')` doesn't work so that wasn't an option – Yuji 'Tomita' Tomita Dec 28 '10 at 23:10
  • It's much more readable--if you find yourself writing lists of exception blocks, you're probably using them incorrectly. It's simple enough to navigate a list of attributes with a helper function. – Glenn Maynard Dec 28 '10 at 23:28
0

I assume in your example that date_created might not exist because you're not always passing the same model class to your loop. The solution may then appear to be to put the logic in the object via the model class definition, and write a 'getrow' method function for any classes you might want to do this to. For classes that have a 'date_created' method, they return that, otherwise return '' or None.

Spacedman
  • 92,590
  • 12
  • 140
  • 224
  • What about a general `optional_fk.optional_method()` situation? I just run into this situation a lot where... given some django models, export available data to a CSV. Is the eval() method to be avoided at all costs or is it OK? It seems like I would write a lot more to avoid eval(). – Yuji 'Tomita' Tomita Dec 28 '10 at 23:17
0

How about

def getattrchain(obj, attrStr, fnArgs=None, defaultResult=''):
    try:
        for attr in attrStr.split('.'):
            obj = getattr(obj,attr)
    except AttributeError:
        return defaultResult

    if callable(obj) and fnArgs is not None:
        return obj(*fnArgs)
    else:
        return obj

for item in django_model_instances:
    writer.writerow([getchainattr(item, x, args) for x,args in (
        ('foo', None),
        ('bar', None),
        ('date_created.strftime', [r'%Y/%m/%d']),
        ('date_complete.strftime', [r'%Y/%m/%d']),
        ('optional_foreign_key.foo', None),
    )
])
Hugh Bothwell
  • 55,315
  • 8
  • 84
  • 99