[Tutor] How (not!) lengthy should functions be?

[Peter Otten]

boB Stepp wrote:

> As I go through my current coding project(s) I find myself breaking
> functions down into other functions, especially when I see see
> (unnecessarily) duplicated code fragments. I understand this to be
> regarded as good practice. However, I am wondering if I am carrying
> things too far? For instance I have a collection of functions that do
> simple units conversions such as:
> 
> def percent2Gy(dose_percent, target_dose_cGy):
>    """
>    Convert a dose given as a percent of target dose into Gy (Gray).
>    """
>    dose_Gy = cGy2Gy((dose_percent / 100.0) * target_dose_cGy)
>    return dose_Gy
> 
> This function calls another units conversion function, cGy2Gy(), in
> doing its work. Generally speaking, I have units conversions functions
> for every conversion I currently need to do plus some that I am not
> using yet because I can easily see the need for them in the future.
> 
> My current understanding of function length best practice is that: 1)
> Each function should have preferably ONE clearly defined purpose. 2) I
> have seen varying recommendations as to number of lines of code per
> function, but I have seem multiple recommendations that a function
> generally should fit into one screen on one's monitor. Of course, some
> people have HUGE monitors! And I assume that any guidance applies
> equally well to methods.
> 
> Am I on-track or am I getting carried away?

Normally I just stress that functions cannot get too short as both beginners 
and smart people tend to make them too long. As far as I'm concerned a 
function with a single line is fine while a function with more than 10 lines 
needs justification.

However, there's more to it. You are working in an environment where people 
may be harmed if you get your numbers wrong, and nothing hinders you to swap 
the arguments in your percent2Gy() function or even pass it a length and an 
amount of dollars. You need measures to make this unlikely.

Unambiguous function names and keyword-only parameters (not available in 
Python 2) and of course tests help, but you might also consider custom types 
that are restricted to a well-defined set of operations. A sketch:

# For illustration purpose only!
class Percent:
    def __init__(self, value):
        if value < 1 or value > 100: # XXX allow 0%?
            raise ValueError
        self.value = value
    def factor(self):
        return self.value / 100.0
    def __repr__(self):
        return "{} %".format(self.value)

class DoseInGray:
    def __init__(self, value, **comment):
        self.value = value
        self.comment = comment.pop("comment", "<no comment>")
        if comment:
            raise TypeEror
    def __repr__(self):
        return "{} Gy # {}".format(self.value, self.comment)
    def __str__(self):
        return  "{} Gy".format(self.value)

class TargetDoseInGray:
    def __init__(self, value):
        self.value = value
    def partial_dose(self, percent):
        if not isinstance(percent, Percent):
            raise TypeError
        return DoseInGray(
            self.value * percent.factor(),
            comment="{} of the target dose".format(percent))
    def __str__(self):
        return "{} Gy".format(self.value)


if __name__ == "__main__":
    target_dose = TargetDoseInGray(20)
    print target_dose
    
    partial_dose = target_dose.partial_dose(Percent(10))
    print partial_dose
    print repr(partial_dose)

    target_dose.partial_dose(0.1) # raises TypeError

Output:

$ python gray.py 
20 Gy
2.0 Gy
2.0 Gy # 10 % of the target dose
Traceback (most recent call last):
  File "gray.py", line 43, in <module>
    target_dose.partial_dose(0.1) # raises TypeError
  File "gray.py", line 27, in partial_dose
    raise TypeError
TypeError

OK, I got carried away a bit with my example, but you might get an idea 
where I'm aiming at. The way I wrote it it is pretty clear that Percent(10) 
denote 10 rather than 1000 %.

I can't spare you a last remark: Python may not be the right language to 
make this bulletproof.