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
1 Answer 1
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)
Explore related questions
See similar questions with these tags.
ui_util.word_wrap("13_chars_word new line", 12)
yields.13_chars_wor
d new line
\$\endgroup\$textwrap.wrap
? \$\endgroup\$