The Pythonic way of validating a long chain of conditions in Python

Question

So I have a long chain of conditions that should be validated to be true. Instead of chaining a long if condition, I tried to be "innovative" and did it this way, which I reckon is more readable. But my question is, is this the optimal way of doing it?

Or is there a pythonic way of doing it? PS: Please respond with an alternative instead of answering "No", thanks!

Here's the code chunk:

def site_exists(site):
    """
    returns the sitebean if it exists,
    else returns false
    """
    vpadmin_service = _get_vpadmin_service(site)
    all_sites = VpAdminServiceUtil.getSites(vpadmin_service)
    for site_listing in all_sites:
        if site.getId():
            #condition check
            try:
                assert site.getId() == site_listing.getId()
                assert site.getName() == site_listing.getName()
                assert site.getCustomer().getId() == site_listing.getCustomer().getId()
            except AssertionError:
                continue
            #pass conditions
            return site_listing
        #no id, so just check for name and customer
        else:
            #condition check
            try:
                assert site.getName() == site_listing.getName()
                assert site.getCustomer().getId() == site_listing.getCustomer().getId()
            except AssertionError:
                continue
            #pass conditions
            site.setId(site_listing.getId())
            return site_listing
    return False

Answer 1

A simpler approach is to build a tuple of the conditions and compare the tuples:

def site_info(s):
    return s.getId(), s.getName(), s.getCustomer().getId()

if site_info(site) == site_info(site_listing):
    return site_listing
else:
    continue

If you have a lot of conditions, or the conditions are expensive, you can instead create a generator for the conditions, and compare with any or all:

import itertools
def iter_site_info(s):
    yield s.getId()
    yield s.getName()
    yield s.getCustomer().getId()

if all(x==y for (x, y) in itertools.izip(iter_site_info(site), iter_site_info(site_listing)):
    return site_listing
else:
    continue

I'm not sure whether Jython has any and all, but they're trivial functions to write.

EDIT - any and all appeared in Python 2.5, so Jython should have them.

Answer 2

Using exception for control flow is bad! And it will also kill your performance. Assuming the condition will be true in one out of 10 cases? So 9 exceptions to handle in the worst case - this comes with a huge performance cost.

If you want readability, try:

        if (
            condition1 and \
            condition2 and \
            condition3
            ):
            # do something

For the revised version, this should be equivalent:

for site_listing in all_sites:
    if site.getName() == site_listing.getName() and site.getCustomer().getId() == site_listing.getCustomer().getId():
        if not site.getId():
            site.setId(site_listing.getId())
        if site.getId() == site_listing.getId():
            return site_listing

Answer 3

I would implement an is_same method on site and call it in the for loops.

all_sites = VpAdminServiceUtil.getSites(vpadmin_service)
for site_listing in all_sites:
    if site_listing.is_same(site, set_id=True):
        return site_listing

As for its implementation (that is your real question), I suggest something like:

...
if self.getName()     != site.getName():     return False
if self.getCustomer() != site.getCustomer(): return False
...
return True

Answer 4

Use the __eq__ method! If you wrote the class of site yourself, no problem. If it is from a module outside of your control, see this solution, or create a somewhat less elegant wrapping class. For the wrap variant, this would be the class:

class WrappedSite(object):
    def __init__(self, site):
        self.site = site

    def __eq__(self, other):
        if site.getId():
            if self.site.getId() != other.site.getId():
                return False
        if self.site.getName() != other.site.getName():
            return False
        if self.site.getCustomer().getId() != other.site.getCustomer().getId():
            return False
        return True

Then your big ugly function is reduced to this:

def site_exists(site):
    """
    returns the sitebean if it exists,
    else returns false
    """
    vpadmin_service = _get_vpadmin_service(site)
    all_sites = VpAdminServiceUtil.getSites(vpadmin_service)
    wsite = WrappedSite(site)
    for site_listing in all_sites:
        if wsite == WrappedSite(site_listing):
            return site_listing
    return False

EDIT Fixed site_exists to return sitebean instead of True.