Testing lower- and upper bound values in Python

Question

Code functionality

The following code tests whether the values that a user specifies for a lower bound and upper bound on two properties:

1. The lower bound is smaller than the upper bound.
2. The values are larger than 0 (positive).

Since it first checks whether the lower bound is smaller than the upper bound, it implicitly verifies the upper bound is larger than 0, if it tests whether the lower bound is larger than 0. Therefore, my consideration is: I can make the code more compact by omitting the check for "is the upper bound larger than 0".

Code

# Object getting labels class Get_labels: def __init__(self,lower_bound,upper_bound,configuration_name): self.lower_bound = lower_bound self.upper_bound = upper_bound self.configuration_name = configuration_name self.check_threshold_validity() # Verifies if the chosen thresholds are valid values. def check_threshold_validity(self): if self.lower_bound>=self.upper_bound: raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be smaller than the upper bound={self.upper_bound} for configuration={self.configuration_name}') # checks if lower bound (and implicitly upper bound) are above zero if self.lower_bound<=0: raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be larger than 0 for configuration={self.configuration_name}') if __name__ == '__main__': get_labels = Get_labels(-1,25,"first") 

Design choice

However, if the code is modified it might not be obvious the upper bound also needs to be checked because that is done implicitly. That might result in the edge case with upper bound below zero is not being caught after modifications. Hence to prevent this scenario, I can implement two unit test that check if an error is raised for:

1. The lower bound negative, upper bound negative
2. The lower bound zero, upper bound negative
3. The lower bound positive, upper bound negative

Question

Is it recommended to include the explicit check in the main code anyways, even though it is tested in the unit tests?

Answer 1

Except under unusual circumstances, classes are things or entities -- hence the term objects -- while functions or methods are actions or operations. You want to name them accordingly. For that reason, Get_labels strikes me as an oddly named class. Based on what you've shown us, I might suggest the name Bounds as an alternative. A side benefit of that name is that it allows you to shorten up the attribute names without loss of meaning.

A separate method for checking the basic validity of the bounds seems like over-engineering to me -- unless the checking logic becomes much more complex or unless it will be used elsewhere in the code. So I would just do simple validation in __init__() in this case.

Avoid the temptation to use chatty or verbose messages in your code. It won't help you or your users over the long run -- at least that's my experience. Keep things direct and rigorously concise. In fact, it's often beneficial to keep the messages very technical rather than natural in their stylistic orientation. What I mean by that is rather than describing the problem the way one might verbally to a human ("The lower bound, which was 1000, must be less than the upper bound, which as 125"), you are often better off to describe the problem in a formulaic, schematic, computer-like way. Among other things, that approach allows you to adopt a conventional format for all error messages in an application. The error message format shown in the rewrite below could be described generically as PROBLEM: SELF. A consistent approach makes it easier to write validation code in the first place and to maintain it over time. Consistency also conveys professionalism to users.

Along those lines, you can often simplify the creation of such validation messages by first defining __repr__() for your class, as illustrated below.

For the validations you have so far, a ValueError is a closer fit than raising a general Exception. Also, you might consider checking for other kinds of errors: for example, are bounds restricted to integers only?

If you do check for that, raise a TypeError.

Finally, a stylistic and admittedly subjective fine point. Below is a stack trace from your code as written. We see the verbose message twice, first as an f-string, and then with parameters filled in. What's wrong with that? Nothing serious, but it's heavy, tedious, even lacking a certain elegance. At a minimum, one could say that the repetition of the verbose message is mildly distracting to the user, putting an extra increment of a visual or cognitive burden on the user to figure out what's going on. Compare that to the stack trace from the revised code.

# ORIGINAL. Traceback (most recent call last): File "bounds.py", line 19, in <module> get_labels = Get_labels(-1,25,"first") File "bounds.py", line 7, in __init__ self.check_threshold_validity() File "bounds.py", line 17, in check_threshold_validity raise Exception(f'Sorry, the lower threshold={self.lower_bound} should be larger than 0 for configuration={self.configuration_name}') Exception: Sorry, the lower threshold=-1 should be larger than 0 for configuration=first # REVISED. Traceback (most recent call last): File "bounds.py", line 72, in <module> b1 = Bounds(1000, 125, 'first') File "bounds.py", line 67, in __init__ raise Exception(msg) Exception: Upper bound must be greater than lower: Bounds(1000, 125, first) 

Code with some possible edits for you to consider:

class Bounds: def __init__(self, lower, upper, name): self.lower = lower self.upper = upper self.name = name if lower <= 0 or upper <= 0: msg = f'Bounds must be positive: {self}' raise ValueError(msg) if upper <= lower: msg = f'Upper bound must be greater than lower: {self}' raise ValueError(msg) def __repr__(self): return f'Bounds({self.lower}, {self.upper}, {self.name!r})'