views:

167

answers:

4

A function should select rows in a table based on the row name (column 2 in this case). It should be able to take either a single name or a list of names as arguments and handle them correctly.

This is what I have now, but ideally there wouldn't be this duplicated code and something like exceptions would be used intelligently to choose the right way to handle the input argument:

def select_rows(to_select):
    # For a list
    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in to_select:
            table.selectRow(row)
    # For a single integer
    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() == to_select:
            table.selectRow(row)
+4  A: 

I would do just this:

def select_rows(to_select):
    # For a list
    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in to_select:
            table.selectRow(row)

and expect that the argument will always be a list - even if its just a list of one element.

Remember:

It is easier to ask for forgiveness than permission.

Andrew Hare
+1... much easier to maintain just one set of code to performs a task, and more pythonic; let it explode if someone calls it in defiance of the docs. If a function is truly needed that accepts a single integer as an argument, make a second one called 'def select_row(to_select)' and have it package 'to_select' as a list, then call select_rows.
Jarret Hardie
+4  A: 

You could redefine your function to take any number of arguments, like this:

def select_rows(*arguments):
    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in arguments:
            table.selectRow(row)

Then you can pass a single argument like this:

select_rows('abc')

multiple arguments like this:

select_rows('abc', 'def')

And if you already have a list:

items = ['abc', 'def']
select_rows(*items)
Steef
+1 Like this approach better than Andrew Hare's... Problem could be if you needed to pass more arguments to the same function, not just the list/single argument. But you could either have those before, or use keyword arguments, i.e. **kwargs.
Jaime
+2  A: 

Actually I agree with Andrew Hare above, just pass a list with a single element.

But if you really must accept a non-list, how about just turning it into a list in that case?

def select_rows(to_select):
    if type(to_select) is not list: to_select = [ to_select ]

    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in to_select:
            table.selectRow(row)

The performance penalty for doing 'in' on a single-item list isn't likely to be high :-) But that does point out one other thing you might want to consider doing if your 'to_select' list may be long: consider casting it to a set so that lookups are more efficient.

def select_rows(to_select):
    if type(to_select) is list: to_select = set( to_select )
    elif type(to_select) is not set: to_select = set( [to_select] )

    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in to_select:
            table.selectRow(row)

-----N

Sharkey
+1  A: 

I'd go along with Sharkey's version, but use a bit more duck typing:

def select_rows(to_select):
    try:
        len(to_select)
    except TypeError:
        to_select = [to_select]

    for row in range(0, table.numRows()):
        if _table.item(row, 1).text() in to_select:
            table.selectRow(row)

This has the benefit of working with any object that supports the in operator. Also, the previous version, if given a tuple or some other sequence, would just wrap it in a list. The downside is that there is some performance penalty for using exception handling.

DopplerShift
This one is problematic for unicodes and strings. cf: http://stackoverflow.com/questions/305359/correct-way-to-detect-sequence-parameter/425567#425567
Gregg Lind
Valid point, should at least have been "in (list, tuple)" ... or maybe "not in (string, unicode)".Preferably you'd want to look directly for "does this thingy support 'in'", I suppose.
Sharkey