3 posts tagged with "quality"

Motivation​

I try to watch coding-related conference talks once in a while and thought that my recent pick of Design Strategies for JavaScript API by Ariya resonate with me. Here's a summary and discussion on the topic of code quality based on ideas from the talk.

Code Quality​

While the talk focuses on API design, it speaks to all programmers as writing functions that are used across classes, modules, and files is a common task. What's worse than inconveniencing others is the fact that some functions are misleading even to the author. When we do write functions, we should strive to achieve the following:

• Readable
• Consistent

Readability​

Read Out Loud​

If you can't pronounce or easily spell out the function name, it deserves a better name.

Avoid Boolean Traps​

Often the first toolkit that we get hold of when we start to modify a function to meet the new requirements is "Boolean parameter". We add a true/false value at the end of the existing parameter list. It won't be long before our list of parameters grows out of control and we can't pinpoint which parameter is responsible for what anymore.

One potential fix is to use an option object:

person.turn("left", true) // turn left and take one step forward
person.turn("left", false) // turn left and stay at the same place
// change to
person.turn("left", {"stepForward": true})
person.turn("left", {"stepForward": false})

Another refactoring idea is to abstract out the commonly used function into a separate function, so perhaps:

person.turn("left", true) // turn left and take one step forward
person.turn("left", false) // turn left and stay at the same place
// change to
person.turnAndStepForward("left") // if this combination is often used 

Do not jump into abstractions too quickly though.

Use a Positive Tone​

This might appear to be a glass-half-full or glass-half-empty subjectivity point of view. However, the talk gave by Ariya suggests that we should avoid double negatives such as x.setDisabled(true) and use x.setEnabled(true) instead. This is to help with understanding statements more intuitively. It is also important to use one over the other consistently.

Explicit Immutability​

I think this is one of the main takeaways I gathered from the talk. While I try my best to write immutable functions, some level of mutability is hard to avoid. When we do have functions that can either be mutable or immutable, it might be beneficial to indicate that in the function name. For example:

aString.trim() // modify the existing string
aString.trimmed() // only return a modified string

Consistency​

Naming​

To be consistent is to be predictable. This relies on making smart observations about the existing norm and agreed-upon conventions. With the knowledge of what we believe all programmers should know, which can be patterns and structures that are familiar, best-practices, or stood the test of time, we can write functions that will turn out to be unsurprising to potential readers.

On a smaller scale, if two functions do similar things, they ought to be named similarly. This is an extension of the idea of polymorphism. For example:

person.turn("left")
car.steer("left")

Perhaps a better way to name the functions will be to use turn for both.

person.turn("left")
car.turn("left")

Parameters​

In the same vein, having consistent parameters will help to reduce mistakes. For example:

person.rotate(1, 2) // first horizontally, second vertically
rectangle.rotate(1, 2) // first vertically, second horizontally

Suppose that both objects have a method called rotate but the parameters are two different ordered pairs of the same values. That is a disaster in the making.

Conclusion​

With the help of powerful IDEs, we now enjoy the convenience of having documentation of functions available as we write code. This may make recognizing what a function is doing or what each parameter means easier, but it should not be an encouragement to write bad functions. Also, if someone is already making a mess writing code, it may not be wise to trust his/her documentations, if there is any...

Motivation​

I am working on fixing some of the issues raised by flake8 (a Python Linter) in a Python-based backend repository and thought it would be nice to discuss some of the common issues and the solutions that I gathered from the web (well, mostly StackOverflow). The use of an auto formatter such as black will help resolve some of these common issues automatically. flake8rules is an excellent resource of a complete list of issues as well.

line too long (92 > 79 characters)flake8(E501)​

Line too long issues mainly happen for the following cases:

• string
• if-statement
• method chaining
• parameter list

... I was going to explain with examples how to use Python's implied line continuation inside parentheses, brackets and braces but decided not to. Nowadays I chose to leave it to my auto formatter to do the job.

For those who insist to write code without any helpful plugins or IDE support, I would like to share that practice does make perfect. I used Vim for a period of writing Java code without autocomplete or format-on-save. I ran style checks and manually fixed issues raised such as having a space between operators. After a month or two, these things became second nature and I was pretty happy with the ability to write well-formatted code without any help. I suppose that was an interesting experience so go ahead and try it yourself.

do not use bare 'except'flake8(E722)​

Example:

def create(item):
    try:
        item("creating")
    except:
        pass

Fix:

def create(item):
    try:
        item("creating")
    except Exception:
        pass

Explanation:

Bare except will catch exceptions you almost certainly don't want to catch, including KeyboardInterrupt (the user hitting Ctrl+C) and Python-raised errors like SystemExit

A better fix:

• Think about what exact exception to catch and specify that instead of just catching any exception.
• Think about whether this exception handling is necessary and are you unintentionally using it for control flow?
• When catching an exception, use either logging or other proper resolutions to handle the anticipated error.

'from some_package_name_here import *' used; unable to detect undefined names flake8(F403)​

I thought this is an interesting topic for discussion. Earlier in my coding journey, I was amused by the many lines of import statements found in some scripts. Sometimes the number of import statements outweigh the number of practical code within the same file.

Nowadays I know better than to fear abstractions (I still fear BAD abstractions and rightfully so). However, with the help of powerful IDEs, importing and tracing the variable/function to their imported package is easier than before. The problem with 'from xxx import *' is that it is unclear what has been imported. Following this, IDEs could not decide whether some of the undefined variables come from the package that you imported or they are errors.

Example

from package_a import *
from package_b import *
# suppose both packages included a function named pretty_print
# it is unclear which method is invoked below
pretty_print("abc")

Fix

from package_a import pretty_print
from package_b import other_function_required
pretty_print("abc")

Conclusion​

When browsing an unfamiliar code repository, we tend to have less sentimental feelings and that fresh perspective allows us to see the good, the bad, and the evil. Besides learning from the good practices, the hacky and the code standard violations (and things like commented out code) are excellent places to start reviewing concepts of coding styles and coding standards, to find out why code misbehaved.

That's all for now.

External resources:​

• Guide to setup black on VSCode
• Using black with flake8

“Smells,” you say, “and that is supposed to be better than vague aesthetics?” Well, yes. We have looked at lots of code, written for projects that span the gamut from wildly successful to nearly dead. In doing so, we have learned to look for certain structures in the code that suggest—sometimes, scream for—the possibility of refactoring.

• Kent Beck and Martin Fowler, Refactoring

Code smell is a term used to describe the tell-tail signs of bad software and therefore an indication that refactoring is overdue. Similar to design patterns that we learn to recognize and apply in our software, naming and identifying different code smells can help us triage the existing code base and proceed to apply fixes in the form of refactoring.

Below is a catalog of code smells identified by Martin Fowler

• Alternative Classes with Different Interfaces
• Comments
• Data Class
• Data Clumps
• Divergent Change
• Duplicated Code
• Feature Envy
• Global Data
• Insider Trading
• Large Class
• Lazy Element
• Long Function
• Long Parameter List
• Loops
• Message Chains
• Middle Man
• Mutable Data
• Mysterious Name
• Primitive Obsession
• Refused Bequest
• Repeated Switches
• Shotgun Surgery
• Speculative Generality
• Temporary Field

A Practical Example: Comments​

Comments are helpful when used to make clarifications and explain the purpose of the code. However, comments are also tricky to write because when done poorly, they can be a source of noise and potentially exposed the weakness in software. On top of the techniques of writing good comments, let's take a look at how comments can become code smells that prompt us to refactor.

Comments that describe what a function does​

// add expenditure to record
void processInput(int data) {
	//...
}

Course of action: Rename function and remove comment

void recordExpenditure(int amount) {
	//...
}

Comments that describe what a block of code does​

static Command parseInput(String input) throws InvalidInputException {
	String task = Parser.tokenize(input)[0];
	// check if the user input contains recognized keywords
	boolean hasKeyword = Arrays
			.stream(TaskType.values())
			.map(TaskType::getRep)
			.anyMatch(x -> x.equals(task));
	if (!hasKeyword) {
		return false;
	}
	return Parser.interpret(input);
}

Course of action: Extract function

private static boolean hasTaskKeyword(String input) {
	String task = Parser.tokenize(input)[0];
	return Arrays
			.stream(TaskType.values())
			.map(TaskType::getRep)
			.anyMatch(x -> x.equals(task));
}

static Command parseInput(String input) throws InvalidInputException {
	if (!hasTaskKeyword(input)) {
		throw new InvalidInputException(input);
	}
	return Parser.interpret(input);
}

Comments that describe assumptions​

public ShowCommand(Index index) {
	// note that index must not be null!
	// and index must be larger or equal to zero!
	this.index = index;
}

Course of action: Be defensive & use assertions

public ShowCommand(Index index) {
	requireNonNull(index);
	assert index.getZeroBased() >= 0;
	this.index = index;
}

Further reading​

• An short essay on code smell by Martin Fowler
• A catalog of code smells
• A comprehensive discussion on code smell is also part of the book: Refactoring: Improving the Design of Existing Code