Boxes v0.14

In the previous lesson we mentioned code smells, the existence of hardcoded "magic numbers" and global variables in our code, and we cleaned it up. There is one other code smell that makes our code pretty funky.

The layout function is long. It's 101 lines long. That is nuts. Functions over 50 lines are usually too complicated. It's also one of the undesirable side effects of the process by which we wrote it. When you write your code organically from the core of an idea, it tends to produce long functions, because it's easier to just tweak things in place instead of breaking things into separate functions.

So, in the future you may decide to do that along the way, or just do what we will now do and fix it afterwards.

This functions does all the following things:

  • Breaks our list of boxes into rows
  • Insert hyphens if needed
  • Justifies lines if needed
  • Breaks the rows into pages
  • Deletes leftover pages

That is a lot of responsibility for a single function. So, we need to break it up. The most complicated part of doing this, and the part you will get better at overtime, is figuring out where the "natural" places to break are.

The easy one is justifying rows. It's here:

110            # We adjust the row
111            # Remove all right-margin spaces
112            while row[-1].letter == " ":
113                row.pop()
114            slack = (pages[page].x + pages[page].w) - (
115                row[-1].x + row[-1].w
116            )
117            # Get a list of all the ones that are stretchy
118            stretchies = [b for b in row if b.stretchy]
119            if not stretchies:  # Nothing stretches do as before.
120                bump = slack / len(row)
121                # The 1st box gets 0 bumps, the 2nd gets 1 and so on
122                for i, b in enumerate(row):
123                    b.x += bump * i
124            else:
125                bump = slack / len(stretchies)
126                # Each stretchy gets wider
127                for b in stretchies:
128                    b.w += bump
129                # And we put each thing next to the previous one
130                for j, b in enumerate(row[1:], 1):
131                    b.x = row[j - 1].x + row[j - 1].w + separation

The only inputs to that code are row, pages[page] and separation so extracting it and making it a function is easy:

59def justify_row(row, page, separation):
60    """Given a row and a page, adjust position of elements in the row
61    so it fits the page width.
62
63    It modifies the contents of the row in place, so returns nothing.
64    """
65
66    # Remove all right-margin spaces
67    while row[-1].letter == " ":
68        row.pop()
69    slack = (page.x + page.w) - (row[-1].x + row[-1].w)
70    # Get a list of all the ones that are stretchy
71    stretchies = [b for b in row if b.stretchy]
72    if not stretchies:  # Nothing stretches do as before.
73        bump = slack / len(row)
74        # The 1st box gets 0 bumps, the 2nd gets 1 and so on
75        for i, b in enumerate(row):
76            b.x += bump * i
77    else:
78        bump = slack / len(stretchies)
79        # Each stretchy gets wider
80        for b in stretchies:
81            b.w += bump
82        # And we put each thing next to the previous one
83        for j, b in enumerate(row[1:], 1):
84            b.x = row[j - 1].x + row[j - 1].w + separation

We can also do a simple function to add hyphens to a row if needed:

87def add_hyphen(row, separation):
88    """If the row requires a hyphen at the end, add it, respecting separation.
89
90    Returns the added hyphen or None."""
91    h_b = None
92    if row[-1].letter == "\xad":
93        # Add a visible hyphen in the row
94        h_b = hyphenbox()
95        h_b.x = row[-2].x + row[-2].w + separation
96        h_b.y = row[-2].y
97        row.append(h_b)  # So it's justified
98    return h_b

And change the code in layout so it uses those functions:

144            h_b = add_hyphen(row, separation)
145            if h_b:
146                _boxes.append(h_b)  # So it's drawn
147            break_line = True
148            justify_row(row, pages[page], separation)

And does it work? The same as before.

$ python boxes.py pride-and-prejudice.txt lesson3.svg

lesson3.svg

That will be enough for now. Our layout function is down to 76 lines, which is in the upper limit of acceptable (aim for fewer than 50 lines if you can).

It may seem like we are just spinning our wheels here and not doing anything useful. After all, our output is the same for several lessons! Don't worry, in the next lesson, you will see why we did this.

What we have been doing is called refactoring and is important. It could be defined as doing changes in the code that don't affect its behavior. That the output has not changed is the whole point. We are doing these changes not because they fix bugs but because they improve our code so finding and fixing those bugs is easier from now on.

Before, if we wanted to change the way we justified lines, we would do it inside layout and any silly mistake, (for example, indenting something wrong) could break everything. Now it's isolated in a nice little corner of our code.

And that little function is going to be worked on.


Further references:

results matching ""

    No results matching ""