If the purpose of a single complicated LOC is clear, would you bother to split it on to multiple lines to make its method clear as well?

Question

Given some (relatively) complicated line of code, I would often be inclined to split it onto multiple lines to make its function/purpose clear to others.

However, if it is quite clear from context what a line does, would you still do the same thing merely to make clearer how it does it? Or would you take the attitude that future developers probably don't know/care how it works, only that it works?

For some context, the particular line that's prompted this question is Python code that takes a string like "1.title" and uses it to modify self.data[1].title:

#IRL, these 2 values are already set somehow
id = "1.title"
val = "foo"

setattr(self.data[int(id[:id.index(".")])], id[id.index(".")+1:], val)

That last line is not the nicest thing in the world to read, so I was thinking of splitting it into:

index = int(id[:id.index(".")])
attrib = id[id.index(".")+1:]
setattr(self.data[index], attrib, val)

If you wanted to go crazy, you could even do dotLocation = id.index(".") first, and use that in the setting of index and attrib.

This is probably a bit clearer to someone else reading it for the first time, but I prefer it nice and concise on one line, and it's highly unlikely that anyone is going to need to modify this one-line utility function. Is there really much point to splitting it up like that?

(Note: I'm fully aware that this is not exactly life-or-death stuff. I'm not going to lose any sleep over it, I'm just wondering what others would do in this kind of situation.)

(Last thing: someone might want to add better tags to this, it's my first Programmers question :) )

Answer 1

In this case, big fat YES.

Reasons:

• The original line actually violates DRY - on a micro scale, but still. You are performing the same calculation twice; factoring it out removes one potential source of bugs
• You have a bracket nesting level of 5, which is way way too much. 3 is about as deep as I'd be willing to read without line breaks and indentation to help me

I'd even take it a step further and separate concerns a bit more, factoring out the "split at first occurrence of specified character" into a function in its own right, so it would boil down to something like:

def split_at_char(s, c):
    charIndex = s.index(c)
    left = s[:charIndex]
    right = s[charIndex + 1:]
    return left, right

...and then, using it:

indexStr, attrName = split_at_char(id, '.')
index = int(indexStr)
setattr(self.data[attrName], index, val)

In fact, there's an existing python function that does exactly what split_at_char does: http://docs.python.org/library/stdtypes.html?highlight=split#str.split No need to reinvent any wheels here. So:

indexStr, attrName = id.split('.', 2)
index = int(indexStr)
setattr(self.data[index], attrName, val)

The ultimate test is: can someone who is not familiar with the codebase fluently read through the code without having to re-read anything? If not, the parts that make you stumble need more work.

Answer 2

Single line statements are fine, when they can be read linearly, e.g.:

[r.name for r in rows].sort.splitIntoColumns(2)

I find that yours:

setattr(self.data[int(id[:id.index(".")])], id[id.index(".")+1:], val)

is not as clear. The repetition of id.index(".") breaks the reading flow, and it's a bit too low-level for me. Why didn't you use split?

 index, name = id.split(".")
 setattr(self.data[int(index)], name, val)

That's only slightly longer, but seems quite clear to me.

Answer 3

It does look neat to have things concise. However, look at the following features of splitting especially when your 1 line is composed of separate logical functions.

1. Simple to read and maintain by others

2. Easier to catch errors on the individual logical parts rather than on the entire expression (in your example, index and attrib are logically separate parts) - You can test the result of each part separately

3. In a system with 10s of thousands of lines of code, what difference would it make if we add a few more?

A somewhat similar question was asked on: Formatting 'Complex' Math

Modification: I can't say that decomposition is always the right way. If the expression is obvious and you are not in identifying in which part it fails, make it all in one line.

Answer 4

Sometimes I base decisions like this on the ease of debugging the statements as separate lines. Every so often, while debugging, a single overloaded line of code becomes an impediment because it obscures the intermediate results.