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

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_list开发者_如何学编程ing
        #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

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.

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

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

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.

Remove some code duplication:

def site_exists(site):
    """
    returns the sitebean if it exists,
    else returns None
    """
    vpadmin_service = _get_vpadmin_service(site)
    all_sites = VpAdminServiceUtil.getSites(vpadmin_service)

    for site_listing in all_sites:
        if (site.getName() == site_listing.getName() and
            site.getCustomer().getId() == site_listing.getCustomer().getId()):
            if site.getId(): # if id is set; it should be the same
                if site.getId() != site_listing.getId(): continue
            else: # no id; consider it the same site
                site.setId(site_listing.getId()) #XXX side-effect
            return site_listing

Note: it is unexpected that site_exists() might modify its argument (via .setId()). Consider to refactor it:

def same_site(site, other):
    if site.getId() and site.getId() != other.getId(): 
       # if id is set; it should be the same
       return False 
    return (site.getName() == other.getName() and
            site.getCustomer().getId() == other.getCustomer().getId())

def get_site_listing(site):
    """
    returns the sitebean corresponding to `site`,
    returns None if there is none
    """
    vpadmin_service = _get_vpadmin_service(site)
    all_sites = VpAdminServiceUtil.getSites(vpadmin_service)

    return next((s for s in all_sites if same_site(site, s)), None)

Note: the code doesn't modify the site object. Use the return value from get_site_listing() instead.

If next() is unavailable then use:

for site_listing in all_sites:
    if same_site(site, site_listing):
       return site_listing
return None

btw, jython should provide property wrappers for you; so you could write:

def same_site(site, other):
    if site.id and site.id != other.id: 
       # if id is set; it should be the same
       return False 
    return site.name == other.name and site.customer.id == other.customer.id

and site.id = id instead of site.setId(id).

Using an exception handler as a code target is basically a "goto". I don't see anything wrong with it really (despite the ancient furor surrounding "goto considered harmful"), but knowing that it's a goto might give you a different perspective.

Using exception handling for control flow is pythonic in the sense that "it's easier to ask for forgiveness than for permission". However, that doesn't extend to making rules to break just so that you can ask for forgiveness.

Others have provided excellent alternatives. Just wanted to address the principle aspect.