Python: If More Than One Of Three Things Is True, Return False

I'm writing a django model that allows my site to have coupons. Coupons can have three types: lifetime account voucher, certain period of months voucher, certain number of dollar

Solution 1:

One thing I've done in similar situations is this:

coupon_types = (self.months, self.dollars, self.lifetime,)

true_count =  sum(1for ct in coupon_types if ct)
if true_count > 1:
    raise ValueError("Coupon can be valid for only one of: months, lifetime, or dollars")  

It's now much easier to add new coupon types to check for in the future!

Solution 2:

You could also use a list comp to filter false values:

iflen([x for x in [self.months, self.dollars, self.lifetime] if x]) > 1:
    raise ValueError()

Or building off MRAB's answer:

ifsum(map(bool, [self.months, self.dollars, self.lifetime])) > 1:
    raise ValueErrro()

Solution 3:

Keep the quantity in a single field, and have the type be a separate field that uses choices.

Solution 4:

Your code looks fine. Here's why:

1.) You wrote it, and you're the one describing the logic. You can play all sort of syntactical tricks to cut down the lines of code (true_count += 1 if self.months else 0, huge if statement, etc.), but I think the way you have it is perfect because it's what you first thought of when trying to describe the logic.

Leave the cute code for the programming challenges, this is the real world.

2.) If you ever decide that you need to add another type of coupon value type, you know exactly what you need to do: add another if statement. In one complex if statement, you'd end up with a harder task to do this.

Solution 5:

if (self.months && (self.dollars || self.lifetime))  || (self.dollars && (self.months || self.lifetime)) || (self.lifetime && (self.dollars || self.months))
    raise ValueError("Coupon can be valid for only one of: months, lifetime, or dollars") 

Edit:

I did a quick circuit mimization using a Karnaugh map (http://en.wikipedia.org/wiki/Karnaugh_map). It ends up this is the smallest possible function with boolean logic:

if((self.months && self.dollars) || (self.dollars && self.lifetime) || (self.lifetime && self.months))
    raise ValueError("Coupon can be valid for only one of: months, lifetime, or dollars") 

Logically both my statements are equivelant but the second one is technically faster / more efficient.

Edit #2: If anyone is interested here is the K-Map

A | B | C | f(A, B, C)
----------------------0 | 0 | 0 |     0
----------------------0 | 0 | 1 |     0
----------------------0 | 1 | 0 |     0
----------------------0 | 1 | 1 |     1
----------------------1 | 0 | 0 |     0
----------------------1 | 0 | 1 |     1
----------------------1 | 1 | 0 |     1
----------------------
1 | 1 | 1 |     1

Which Reduces to:

   C\AB
     -----------------
     | 0 | 0 | 1 | 0 |     
     -----------------      OR      AB + BC + AC
     | 0 | 1 | 1 | 1 |
     -----------------