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 dollars voucher.

To keep things simple, I'm only allowing coupons to have one of the three possible values (i.e. a voucher can't be for $10 and 5 months). But I want to check when a coupon is being saved to ensure this rule is true.

Currently I have:

true_count = 0
if self.months:
    true_count += 1
if self.dollars:
    true_count += 1
if self.lifetime:
    true_count += 1    

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

I know there's a better way to do this, but I'm not seeing it (call it coder's block).

Help is much appreciated.

In case it maters, the three types are int, int, and bool

months = models.IntegerField(default=0)
cents = models.IntegerField(default=0)
#dollars = models.FloatField(default=0.00)
#dollars replaced with integer cents per advice of group
lifetime = models.BooleanField(def开发者_运维技巧ault=False)

---

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

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

true_count =  sum(1 for 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!

---

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

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

Or building off MRAB's answer:

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

---

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

---

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.

---

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 |
     -----------------

---

I think spreading this over a few lines is fine - this makes it easier to maintain if there were more attributes to test in the future. Using len or sum feels a bit too obfuscated

# Ensure that only one of these values is set
true_count = 0
true_count += bool(self.months)
true_count += bool(self.dollars)
true_count += bool(self.lifetime)

---

I don't know if this is better for you, but doing it this way would work:

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

---

If you have Python2.7 or newer

from collections import Counter
items_to_test = (self.months, self.dollars, self.lifetime)
true_count = Counter(map(bool, items_to_test))[True]

---

Even better solution than before, with combinations, any, and all. Assuming you have all the attributes you want to test in a sequence called attributes:

from itertools import combinations
any(map(all, combinations(attributes, 2)))

In english, it reads

Are any length-2 combinations of the attributes all true?

This solution works for an arbitrary number of attributes, and can be modified to test for an arbitrary number of them being true.

Although admittedly it's very inefficient, I'd say it's pretty cute and readable.

---

How about

if len(filter([self.months, self.dollars, self.lifetime])) > 1:
...

I find it just as readable as a list comprehension with an if clause, and more concise.

---

bool is a subclass of int because Python originally lacked bool, using int for Boolean, so:

if self.months + self.dollars + self.lifetime > 1:
    ...

This works because False == 0 and True == 1 are both true.