2

My situation is that I am writing a function that allows a user to delete records from a database. For efficiency, the function itself must be able to take a list of arguments for batch deletion. In addition, for simplicity, the list can contain either IDs (which can be strings or ints) or row objects (which are glorified dicts)

If you are curious, the reason behind allowing full-fledged rows is that there are security checks that examine row data to make sure that the user is allowed to delete the rows in question. Allowing rows to be passed in can reduce trips to the database.

Currently, I use the oft-chastised isinstance function in the following way:

def deleteRows(rowsToDelete):
    ids = set()
    rows = []
    for r in rowsToDelete:
        if isistance(r, basestring) or isinstance(r, int):
            ids.add(r)
        else:
            rows.append(r)
    # Some logic that SELECTS based on the data in ids and appends the
    # result into rows...
    # ... then security ...
    # ... then DELETE

I can see why this is dangerous (what if there is a type can be coerced into an id that isn't an int or basestring?), however I am at a loss for a cleaner solution that doesn't involve isinstance in one way or another, or relying on an exception that may or may not be related to my actual code.

The question is, what is the most effective way of doing this in python? Or, alternatively, is babying the caller this much just a recipe for disaster, i.e. I should just require that the parameter be either a list of types that coerce to an int or a list of rows?

zashu
  • 747
  • 1
  • 7
  • 22
  • Are the IDs always numerical? If so, why not just `try` to convert them to an int, and catch the error if it doesn't work? – supercheetah Feb 18 '13 at 23:46
  • @supercheetah: the ID would always be a number. My original fear was: what if `int(r)` could somehow turn my row data into an integer? In that case, the `try` would succeed but the logic would be incorrect. However, after looking more closely at the documentation for `int`, it seems like it doesn't utilize magic functions and will *only* work on numbers or strings. Thus, in this case, I think you might be right! – zashu Feb 18 '13 at 23:51
  • 1
    @supercheetah: Is correct in that you should do an `int` coercion. I would either a) avoid catching the error, or b) (this is better) catch and re-raise the error with relevant information tacked on (i.e. "ids must be castable to int"). This allows users the flexibility to pass in custom data types as long as they implement an `__int__` magic method, but also delivers an informative error message if something goes awry. – Joel Cornett Feb 19 '13 at 00:21

1 Answers1

2

I would recommend not making the interface this flexible. Your code will be simpler to have separate delete_rows(*row_objs) and delete_rows_by_id(*int_ids) the former calling the latter.

Ben Wilber
  • 843
  • 8
  • 15
  • Splitting the function into two (one based on IDs and one based on Rows) actually hugely simplified the functionality overall. – zashu Feb 20 '13 at 21:47