Proper design for function doing one job but involving many little tasks

Question

I have a class in Python that its only responsibility is receive text, a font path, max font size and min font size and print the text on an image if it fits.

I have a function named word_wrap() and it does the following:

1. declare variables (text, font)
2. split text into lines if it finds any \n characters
3. split each line into words and get font size (width, height) for every word and calculates if the total_width_of_words + width_of_current_word > width_image ... I bet you understand what it does if this returns False or True. :-)
4. We now have a list of lists of the form: [['hello', 'world'], ['I', 'am', 'posting', 'on', 'programmers.se']] and each inner list is a sentence that fits in the width of the Image we're going to print the text into. :-)
5. Multiply the number_of_sentences x average_height of the sentences to see if all the sentences fits into the height of the Image. If it does not the function return False... if it does fit then the function returns a font object.

word_wrap() is called from a function called bisect() and if the text do not fit in the image the first time bisect() call word_wrap() one more time with a smaller font size, etc etc.

Should word_wrap() be re-factored in smaller functions? If this is the case, do you have any suggestion on what the structure would look like?

I'm really open to feedback, any help would be so much appreciated.

Answer 1

I'm just going to assume that your bisect() function is a method of the class you mentioned and that it looks similar to this:

def bisect()
    while self.word_wrap() == False:
        pass

As far as the word_wrap() method is concered, I would propose to break it up into smaller methods because small methods are:

1. Easier to Test

Imagine writing a unit test for your word_wrap() method. You'll need to test whether:

• the text is split into lines correctly
• the lines are split into words correctly
• the line length check does indeed work for every kind of input (say, a line with no words on it)
• the height calculation works as expected
• the returned font object has the correct properties and values

Writing a unit test for the latter bullet point certainly isn't an issue. However, the rest of the method is opaque to any caller (including unit tests), meaning it can not be (unit) tested thoroughly. Breaking all these bullet points up into their own distinct methods will allow you to test them.

2. Easier to Read

Some or even all of the tasks performed need their own variables, their own logic and their own comments, all of which have to be scanned and "organized" by a reader.

2.a. Easier to Understand

From my experience, more than 3 or 4 variables in one function will lead to the reader having to look them up again when reading the rest of the method, regardless of how simple the variables were in the first place. I'm not saying you should never use more than 3 or 4 variables in one method, but I would advise to try and avoid it. Having multiple methods allows the reader to ignore all the variables that are required to get a smaller task done and instead see the smaller tasks as a method that returns a result, not more, not less. This 'frees' up brainpower that would otherwise be used to remember what a certain variable did and whether it will still be used somewhere else in a function.

... the span of absolute judgment and the span of immediate memory impose severe limitations on the amount of information that we are able to receive, process, and remember. By organizing the stimulus input simultaneously into several dimensions and successively into a sequence of chunks, we manage to break (or at least stretch) this informational bottleneck. http://psychclassics.yorku.ca/Miller/

2.b. Easier to Debug

A bug that can be easily understood is also much easier to debug, simply because it is easy to define what exactly isn't working. It's the same thing that distinguishes a good bug report ("Firefox freezes when opening SVG file on Win10; logs attached") from a bad one ("Firefox crashes when opening file") or a good SO question ("JS: Uncaught Exception: can not call method foo of bar") from a bad one ("my web site dont works, please fix").

2.c. Easier to Re-Use

Sometime in the future you might have to solve a problem similar to the one you're solving now, for example if you need to fit text into an arbitrary geometric shape instead of just an image (or rectangle, really). A lot of the smaller tasks, such as splitting the text into lines and words, measuring word size and fitting words into a shape will have to be used again to implement the new feature. Instead of having to refactor these smaller tasks out of one method then, why not split them up into their own methods now?

Finally, imagine how much of this answer you would have already forgotten if it was written without any formatting or bullet points.

Answer 2

TL;DR: It depends. Is the function maintainable as is? Do you have other use cases for the component functions?

There are 2 things that typically drive decomposing a function into component functions.

1. Having a specific need for the component function alone.
2. Making code more maintainable.

Reason 1. is pretty straight forward. Either you have this need and you know our you don't. So we'll focus on 2.

There are reasons and code can be more or less maintainable. This mostly has to do with how long it will take a developer to understand the code enough to safely extend the function or fix a problem.

The tricky part is that, while there are some examples that are clearly costly (a single function that is 5 pages long) they are the easy ones to recognize. The hard ones are the ones that look a little too large or a little too complicated, but it's not clear cut. The decisions about whether to decompose these are often personal preference.

However, in a team there is an easy way to check. Have one of your teammates read the code. If he understands what's going on quickly and easily then your probably fine. If he struggles to make out what is intended, or what's going on then decompose or add comments.