CODEWITHSUNDEEP
python
Tomasz Zieliński
Tomasz Zieliński

Reputation: 16356

What is The Pythonic Way for writing matching algorithm

I have this piece of code (should be self-explanatory; if not, just ask):

for tr in completed_taskrevs:
    found = False
    for nr in completion_noterevs:
        if tr.description in nr.body:
            completion_noterevs.remove(nr)
            found = True
            break
    assert found

How can I make it more pythonic?

Upvotes: 2

Views: 336

Answers (3)

gruszczy
gruszczy

Reputation: 42188

Try this:

for tr in compleded_taskrevs:
    try:
        nrs = (nr for nr in completion_noterevs if tr.description in nr.body)
        completion_noterevs.remove(nrs.next())
    except StopIteration:
        raise ValueError('Some error')

EDIT: Devin is right. Assertion is not the way to go, better to use a standard exception.

Upvotes: 8

Devin Jeanpierre
Devin Jeanpierre

Reputation: 95536

Using assert/AssertionError is probably wrong here, I'd say. It's useful for "debugging assertions", which is to say, to make sure your code is sane. It's not useful as a way to raise an error when you get invalid data, for many reasons, the most interesting of which is probably that assert isn't even guaranteed to be executed-- if your code is compiled with any optimization settings, it won't be. And heck, even if this is a debugging thing, I'd still use raise-- it's more readable, and it'll always happen, no matter when or why the data is wrong.

So to make it more "pythonic", I would remove the assert and replace it with something nicer. As it happens, such a nicer thing exists, and it's the raise statement. Further, I would replace the clumsy value set/check with the else clause of loops, which is executed when the loop is exhausted (or, for while loops, when the condition becomes false). So if you break, the else clause is not executed.

for tr in completed_taskrevs:
    for nr in completion_noterevs:
        if tr.description in nr.body:
            completion_noterevs.remove(nr)
            break
    else:
        raise ValueError("description not found"); # or whatever exception would be appropriate

Other than this, I probably wouldn't change anything.

Upvotes: 6

rui
rui

Reputation: 11284

The list generator below could return you all items nr that are valid tr.description in nr.body however it's hard to simplify your algorithm as it has quite a few branches. I would just say go with the algorithm you have as long as it does what you want.

[nr for tr in completed_taskrevs for nr in completion_noterevs if tr.description in nr.body]

Upvotes: 3

Related Questions