6
\$\begingroup\$

I'm attempting to make some ui tools for my python ncurses application, and I needed a word wrap function. It works pretty well, but trying to read it is frustrating. I'm specifically looking for readability improvements, but if there are some other tips you can give, including edge cases or PEP violations, they would also be appreciated.

class ui_util:
    def chunk_word(word, length):
        return [ word[i:i+length] for i in range(0, len(word), length) ]

    def word_wrap(str, width):
        def add_first_word():
            nonlocal word
            nonlocal wrapped_line
            nonlocal lines_final

            if (len(word) <= width):
                wrapped_line = word
            else:
                chunks = ui_util.chunk_word(word, width)
                for i in range(len(chunks) - 1):
                    lines_final.append(chunks[i])
                wrapped_line = chunks[-1]

        lines = str.splitlines()
        lines_final = []
        for line in lines:
            if (len(line) <= width):
                lines_final.append(line)
            else:
                words = line.split()
                wrapped_line = ""
                for word in words:
                    if (len(wrapped_line) == 0):
                        add_first_word()
                    elif (len(wrapped_line) + len(word) + 1 <= width):
                        wrapped_line += ' ' + word
                    else:
                        lines_final.append(wrapped_line)
                        add_first_word()
                lines_final.append(wrapped_line)
        return lines_final

Called like:

lines = ui_util.word_wrap("some test words 13_chars_word\r\nnew line\r\n12_char_word 13_chars_word", 12)
for i in range(len(lines)):
    print(lines[i])

Output:

some test
words
13_chars_wor
d
new line
12_char_word
13_chars_wor
d
\$\endgroup\$
  • 1
    \$\begingroup\$ Keep in mind, that if you don't use a monotype font, then a string with a length of 12 may be shorter, than one with a length of only 10 ("iiiiiiiiii", "MMMMMMM") \$\endgroup\$ – Bálint Jul 27 '17 at 23:04
  • \$\begingroup\$ @Bálint You're absolutely right. But this is specifically for an ncurses application. A non-monotype font would ruin the entire program, let alone the word wrap function. \$\endgroup\$ – DeathTails Jul 27 '17 at 23:10
  • \$\begingroup\$ @JerryCoffin The "new line" has a line break before it or else it would exhibit that behavior. ui_util.word_wrap("13_chars_word new line", 12) yields. 13_chars_wor d new line \$\endgroup\$ – DeathTails Jul 27 '17 at 23:17
  • \$\begingroup\$ Yes. Each line is split and wrapped separately. \$\endgroup\$ – DeathTails Jul 27 '17 at 23:19
  • 3
    \$\begingroup\$ What was wrong with textwrap.wrap? \$\endgroup\$ – Gareth Rees Jul 28 '17 at 8:32
5
\$\begingroup\$

Code organization

The add_first_word() closure passes information via three nonlocal variables, which makes the code hard to understand.

I find it dissatisfying that the first word needs to be handled as a special case. Instead, I would recommend using ' '.join(…) to help you add spaces conditionally. That seems especially appropriate, considering that you also used .split().

It seems that you are using ui_util as a namespace rather than as a real class. I would recommend just using the module for namespacing. Furthermore, consider hiding or getting rid of the chunk_word() helper, in which case you would just have a word_wrap() function.

Naming

str is the name of a built-in type. Avoid re-appropriating this conventional usage as a variable name. I suggest renaming the parameter as text.

lines_final is an awkward variable name. It implies that there is something wrong with lines. I would say that if you split the text on explicit line breaks, then what you get is "paragraphs".

Style

In Python, parentheses around if conditions are extraneous, and therefore unconventional.

PEP 8, the official style guide, says:

Pet Peeves

Avoid extraneous whitespace… immediately inside parentheses, brackets or braces.

You wrote:

return [ word[i:i+length] for i in range(0, len(word), length) ]

Suggested solution

I think that it would make sense to convert this function into a generator. Instead of accumulating all of the output in a lines_final list, you can just yield each line as it fills up. (If the caller really needs a list, then use list(word_wrap(…)).)

Once you eliminate the special case as mentioned above, the code becomes much simpler.

def word_wrap(text, width):
    for paragraph in text.splitlines():
        line_len, line_buf = 0, []
        for word in paragraph.split():
            # If appending the word would cause overflow, flush the line.
            if line_len + len(line_buf) + len(word) > width:
                yield ' '.join(line_buf)
                line_len, line_buf = 0, []
            # If just this word alone would overflow, break it by force.
            while len(word) > width:
                yield word[:width]
                word = word[width:]
            line_buf.append(word)
            line_len += len(word)
        if line_buf:
            yield ' '.join(line_buf)

Example usage:

for line in word_wrap("…", 12):
    print(line)
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.