Single Responsibility Lines‽

From Wikipedia:

The Single Responsibility Principle (SRP) is a computer programming idea: every module or class should have responsibility for a single functionality. All its services should be narrowly aligned with that responsibility. I leave the discussion about SRP in modules and classes to the plethora of sources addressing them

Yeah, But Every Line? #

Yes, every line should perform a single1 task. Even if you are using Perl, every line should have one specific goal. Complex expressions are ¡very impressive¡, but such preening makes the code hard to understand2. Breaking complex expressions and commands into informational variables and steps makes them easier to read3, easier to debug, and easier to refactor

There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies. — Charles Hoare

Good Names Help #

Proper naming, as well as intermediate and informational variables, reduce the need for comments4 by making the code communicate well. They make debugging and logging easy and consistent. Intermediate variables do not make code slower (or faster). Interpreters and compilers break the source down to the operator level; a little whitespace and a local token are not going to phase them5

Do not confuse a compound expression with an overly complex one, although length is a consideration. (x > 2) && (x < 14) is a single test, and if it fits neatly on one line, is a perfectly nice expression

This monstrosity

state_of_being.find("walk to school", False) ||
           state_of_being.find("take your lunch", False) ||
                           are_tautologies_welcome_here

even with the descriptive names, is not readable

Getting Data Is Not the Same as Using Data # 

my_data_bag[thingie1][wet_sproket == 4 ? "subkey1" : "subkey2"] =
   calculate_important_data(wet_sproket != 4 ? source2 : source1.child)

As with complex mathematical expressions6, the inner-most expression evaluates first. Unlike mathematics, we don’t have to do it like this. Figure out the input parameters, then use them to get data, then apply the data

if wet_sproket == 4
    param = source1.child
else
    param = source2

important_data = calculate_important_data(param)

if thingie not in my_data_bag
    my_data_bag[thingie] = {}

if wet_sproket == 4
    my_data_bag[thingie]["subkey1"] = important_data
else
    my_data_bag[thingie]["subkey2"] = important_data

I’ve “wasted” ten lines of code (not counting whitespace) making the assignment readable, understandable, testable, debuggable, and robust

Returning Is a Thing #

Even short examples can have embedded complexity

func handle(queue chan *Request) {
    for req := range queue {
        req.resultChan <- req.f(req.args)
    }
}

should be more like

func handle(queue chan *Request) {
    for req := range queue {
        result = req.f(req.args)
        req.resultChan <- result
    }
}

Again, clever expressions holding lots of business rules and operations are bad; combining them with a control structure is worse

return this_too() ? "shall" : "pass".strip()

should be more like

result = this_too() ? "shall" : "pass".strip()
return result

or

if this_too():
    result = "shall"
else:
    result = "pass".strip()

return result

Don’t Confuse Clarity with … #

Brevity #

A common mantra7 in business and marketing speak (although that is rarely succinct). We do not pay by the character anymore: we can afford good naming, vowels, and vertical whitespace

Loquation #

Artificially long names don’t help any more than commenting every line. If good code reads like prose, “Never use a long word where a short one will do” (George Orwell). See also “Homeopathic naming” from Kevlin Henney

Efficiency #

A seemingly straight-forward line like this

for i, value in enumerate(data[::2]):
     value2 = data [i + 1]

tries to do two things at once. Slicing data is a operation which returns a new slice of alternate elements. The author intended to iterate the original slice, skipping alternate members, but the code iterates through every item in an array of the odd-indexed elements of the data[]
This code implements the desired behaviour clearly

oddElements = data[::2]
evenElements = data[1::2]
for I, value1 in enumerate(oddElements):
    value2 = evenElements[i]

Iterating over an array is a separate responsibility from modifying the array

Look at how seemingly straight-forward code can hide intent
for i, value in range(value * 4):
Even this is not too simple to expand to separate lines. This code does not tell us why it is multiplying by four (e.g. quarterly to annual). Using a constant might tell us more, and looking at it as two different operations, iteration and modification, will tell us everything

Breaking the alternating array into two parallel arrays (as above) makes using them easy. Clever (worse) code might look like

 # this is clever code. don't do this

oddElements = data[::2]
for i, value1 in enumerate(oddElements):
     value2 = data[i * 2 - 1]

This code has a bug in it8, but it’s not obvious. Breaking the second index into its own line might make the bug more obvious

Examples #

Do one conceptual thing #

The compound operations are both easy to understand and fit the operation

log.info("user name: ${user.name.lower.trim}")

Does one thing: operates a loop #

for (i = array_length - 1; i >= 0; i--)

Does one thing: records and operates on the result of a call #

if (value, err := DoThatThingYouDo(47); err == nil)

Does several things #

Forcing code onto one line does no one any favours
Many, many comments might make this understandable (if they stayed up-to-date)

return transylvania_6_5000(total_value < MEDIAN_LIMIT ? total_value + extras(total_value, previous_value) : total_value * 1.2)`

Rather, break the expression down into separate lines. These arithmetic additions are easy to see and understand because the names are clear if they are on separate lines

if total_value < MEDIAN_LIMIT
    total_extras = extras(total_value, previous_value)
    result = calculate_total_extras(total_value + total_extras)
else
    result = calculate_total_extras(total_value * ABOVE_MEDIAN_MARKUP)

return result

or even

if total_value < MEDIAN_LIMIT
    total_extras = extras(total_value, previous_value)
    extras_and_values = total_value + total_extras
    result = calculate_total_extras(extras_and_values)
else
    marked_up_value = total_value * ABOVE_MEDIAN_MARKUP
    result = calculate_total_extras(marked_u-_value)

return result

Our goal is clarity; Do What’s Indicated9

Does two things #

This operates a loop by making an evaluation unrelated to the loop itself

// DOES TOO MUCH     
for (i = array_length;
      i >= 0 && array[i] > 14 && array[i] < max_val * 1.12; 
      i--)

Managing the loop is one thing; testing the data is something else. Testing for a null-terminator might be simple enough to include in the loop-control expression

lowest_allowed_value = 14
highest_allowed_valule = max_val * 1.12
for i = 0; i < array_length; i++)
    if (array[i] <= lowest_allowed_value ) || (array[i] >= highest_allowd_value)
        break

or break the calculation into a function with readable code. If the function name is good, readers will never have to look at that code

for i = 0; i < array_length && value_within_range(array[i]); i++)

Smells and Tells #

Ternaries #

Ternary expressions are a common violation of doing one thing. They are best used to fill in default values, make plurals, select the correct indefinite article, and include or omit small bits of text. Apart from trivial formatting operations, don’t call functions in ternaries. Nesting ternaries is Right Out

Line Length #

Line length is a broad metric, and often a good one. If a non-string line is wrapping, breaking it into separate lines is usually better than wrapping it. Compilers and interpreters are very good at optimizing code; the number of lines source code spans, even with intermediate variables, is never going to affect performance or object size

Indentation #

I like four-space indentation because it’s easier to read and makes it obvious when one has nested structures too deeply. Find a block of code starting at a shallow depth and move it to its own function where reasonable

Comments #

Comments cannot compensate for obtuse code. They decay and, in the words of Kevlin Henney, someone who writes obtuse code rarely has the ability to write clear comments

Code Reviews #

If any code reviewer does not understand what a line does, refactor the code instead of just telling them or, at worst, adding a comment. They represent the code-readers of the future (maybe even Future You). Their confusion indicates that the code does not express its purpose. Don’t argue that it is obvious or what a “reasonable” coder would think; take it as evidence that your code failed to communicate with another person10

See Also #

  1. for large values of one ↩

  2. and leads to the pseudo-hazing ritual of “Just Read the Code” because “everyone has to go through it”  ↩

  3. Programs must be written for people to read, and only incidentally for machines to execute — Harold Abelson ↩

  4. Comment Only What the Code Cannot Say, Kevlin Henney ↩

  5. Trying to outsmart a compiler defeats much of the purpose of using one. — Kernighan & Plauger  ↩

  6. nested and embedded ternaries are a code smell ↩

  7. which is usually followed by buzzwords, sadly enough. See Leverging Our Synergies ↩

  8. the first iteration will try and access data[-1] ↩

  9. Elihu King, 1992 ↩

  10. To self-examine, consider an exercise: Explain to Me Like I’m Five. Not only will it help focus on the most important parts, it helps with descriptive naming ↩

 
0
Kudos
 
0
Kudos

Now read this

A Simple Caching Pattern in Go

One might use it like this: var myCache = NewOurCache(0, calculateTimestamp) // ... var myValue time.Time = myCache.Open(dateText, timeText, Location) OurCacheType is an alias for the actual data cached. It can be any type including a... Continue →