Tag Archives: Code Health

What’s in a Name?

This is another post in our Code Health series. A version of this post originally appeared in Google bathrooms worldwide as a Google Testing on the Toilet episode. You can download a printer-friendly version to display in your office.

by Adam Raider

“There are only two hard things in computer science: cache invalidation and naming things.” —Phil Karlton

Have you ever read an identifier only to realize later it doesn’t do what you expected? Or had to read the implementation in order to understand an interface? These indirections eat up our cognitive bandwidth and make our work more difficult. We spend far more time reading code than we do writing it; thoughtful names can save the reader (and writer) a lot of time and frustration. Here are some naming tips:

Spend time considering names—it’s worth it. Don’t default to the first name that comes to mind. The more public the name, the more expensive it is to change. Past a certain scale, names become infeasible to change, especially for APIs. Pay attention to a name in proportion to the cost of renaming it later. If you’re feeling stuck, consider running a new name by a teammate.
Describe behavior. Encourage naming based on what functions do rather than when the functions are called. Avoid prefixes like “handle” or “on” as they describe when and provide no added meaning:

button.listen('click', handleClick)

button.listen('click', addItemToCart)

Reveal intent with a contextually appropriate level of abstraction:

High-abstraction functions describe the what and operate on high-level types.
Lower-abstraction functions describe the how and operate on lower-level types.

For example, logout might call into clearUserToken, and recordWithCamera might call into parseStreamBytes.

Prefer unique, precise names. Are you frequently asking for the UserManager? Manager, Util, and similar suffixes are a common but imprecise naming convention. What does it do? It manages! If you’re struggling to come up with a more precise name, consider splitting the class into smaller ones.
Balance clarity and conciseness—use abbreviations with care. Commonly used abbreviations, such as HTML, i18n, and RPC, can aid communication but less-known ones can confuse your average readers. Ask yourself, “Will my readers immediately understand this label? Will a reader five years from now understand it?”
Avoid repetition and filler words. Or in other words, don’t say the same thing twice. It adds unnecessary visual noise:

userData.userBirthdayDate

user.birthDate

Software changes—names should, too. If you see an identifier that doesn’t aptly describe itself—fix it!

Learn more about identifier naming in this post: IdentifierNamingPostForWorldWideWebBlog.

Source: Google Testing Blog

Let Code Speak for Itself

by Shiva Garg and Francois Aube

Comments can be invaluable for understanding and maintaining a code base. But excessive comments in code can become unhelpful clutter full of extraneous and/or outdated detail.

Comments that offer useless (or worse, obsolete) information hurt readability. Here are some tips to let your code speak for itself:

Write comments to explain the “why” behind a certain approach in code. The comment below has two good reasons to exist: documenting non-obvious behavior and answering a question that a reader is likely to have (i.e. why doesn’t this code render directly on the screen?):

// Eliminate flickering by rendering the next frame off-screen and swapping into the

// visible buffer.

RenderOffScreen();

SwapBuffers();

Use well-named identifiers to guide the reader and reduce the need for comments:

// Payout should not happen if the user is

// in an ineligible country.

std::unordered_set<std::string> ineligible =

{"Atlantis", "Utopia"};

if (!ineligible.contains(country)) {

Payout(user.user_id);

}

if (IsCountryEligibleForPayout(country)) { Payout(user.user_id); }

Write function comments (a.k.a. API documentation) that describe intended meaning and purpose, not implementation details. Choose unambiguous function signatures that callers can use without reading any documentation. Don’t explain inner details that could change without affecting the contract with the caller:

// Reads an input string containing either a

// number of milliseconds since epoch or an

// ISO 8601 date and time. Invokes the

// Sole, Laces, and ToeCap APIs, then

// returns an object representing the Shoe

// available then or nullptr if none were.

Shoe* ModelAvailableAt(char* time);

// Returns the Shoe that was available for

// purchase at `time`. If no model was

// available, throws a runtime_error.

Shoe ModelAvailableAt(time_t time);

Omit comments that state the obvious. Superfluous comments increase code maintenance when code gets refactored and don’t add value, only overhead to keep these comments current:

// Increment counter by 1.

counter++;

Learn more about writing good comments: To Comment or Not to Comment?, Best practices for writing code comments

Source: Google Testing Blog

Exceptional Exception Handling

by Yiming Sun

Have you ever seen huge exception-handling blocks? Here is an example. Let's assume we are calling bakePizza() to bake a pizza, and it can be overbaked, throwing a PizzaOverbakedException.

class PizzaOverbakedException extends Exception {};

void bakePizza () throws PizzaOverbakedException {};

try {

// 100+ lines of code to prepare pizza ingredients.

...

bakePizza();

// Another 100+ lines of code to deliver pizza to a customer.

...

} catch (Exception e) {

throw new IllegalStateException(); // Root cause ignored while throwing new exception.

}

Here are the problems with the above code:

Obscuring the logic. The method bakePizza(), is obscured by the additional lines of code of preparation and delivery, so unintended exceptions from preparation and delivery may be caught.
Catching the general exception. catch (Exception e) will catch everything, despite that we might only want to handle PizzaOverbakedException here.
Rethrowing a general exception, with the original exception ignored. This means that the root cause is lost - we don't know what exactly goes wrong with pizza baking while debugging.

Here is a better alternative, rewritten to avoid the problems above.

class PizzaOverbakedException extends Exception {};

void bakePizza () throws PizzaOverbakedException {};

// 100+ lines of code to prepare pizza ingredients.

...

try {

bakePizza();

} catch (PizzaOverbakedException e) { // Other exceptions won’t be caught.

// Rethrow a more meaningful exception; so that we know pizza is overbaked.

throw new IllegalStateException(“You burned the pizza!”, e);

}

// Another 100+ lines of code to deliver pizza to a customer.

...

Source: Google Testing Blog

Clean Up Code Cruft

By Per Jacobsson

The book Clean Code discusses a camping rule that is good to keep in the back of your mind when writing code:

Leave the campground cleaner than you found it

So how does that fit into software development? The thinking is this: When you make changes to code that can potentially be improved, try to make it just a little bit better.

This doesn't necessarily mean you have to go out of your way to do huge refactorings. Changing something small can go a long way:

Rename a variable to something more descriptive.
Break apart a huge function into a few logical pieces.
Fix a lint warning.
Bring an outdated comment up to date.
Extract duplicated lines to a function.
Write a unit test for an untested function.
Whatever other itch you feel like scratching.

Cleaning up the small things often makes it easier to see and fix the bigger issues.

But what about "If it's not broken, don't fix it"? Changing code can be risky, right? There's no obvious rule, but if you're always afraid to change your code, you have bigger problems. Cruft in code that is actively being changed is like credit card debt. Either you pay it off, or you eventually go bankrupt.

Unit tests help mitigate the risk of changing code. When you're doing cleanup work, be sure there are unit tests for the things you're about to change. This may mean writing a few new ones yourself.

If you’re working on a change and end up doing some minor cleanup, you can often include these cleanups in the same change. Be careful to not distract your code reviewer by adding too many unrelated cleanups. An option that works well is to send the cleanup fixes in multiple tiny changes that are small enough to just take a few seconds to review.

As mentioned in the book: "Can you imagine working on a project where the code simply got better as time passed?"

“Clean Code: A Handbook of Agile Software Craftsmanship” by Robert C. Martin was published in 2008.

Source: Google Testing Blog

Write Clean Code to Reduce Cognitive Load

By Andrew Trenk

Do you ever read code and find it hard to understand? You may be experiencing cognitive load!

Cognitive load refers to the amount of mental effort required to complete a task. When reading code, you have to keep in mind information such as values of variables, conditional logic, loop indices, data structure state, and interface contracts. Cognitive load increases as code becomes more complex. People can typically hold up to 5–7 separate pieces of information in their short-term memory (source); code that involves more information than that can be difficult to understand.

Two brains displayed side-by-side.

The left brain is red with a sad face. The text below it says 'Complex code: Too much cognitive load'.;

The left brain is green with a happy face. The text below it says 'Simple code: Minimal cognitive load'.

Cognitive load is often higher for other people reading code you wrote than it is for yourself, since readers need to understand your intentions. Think of the times you read someone else’s code and struggled to understand its behavior. One of the reasons for code reviews is to allow reviewers to check if the changes to the code cause too much cognitive load. Be kind to your co-workers: reduce their cognitive load by writing clean code.

The key to reducing cognitive load is to make code simpler so it can be understood more easily by readers. This is the principle behind many code health practices. Here are some examples:

Limit the amount of code in a function or file. Aim to keep the code concise enough that you can keep the whole thing in your head at once. Prefer to keep functions small, and try to limit each class to a single responsibility.
Create abstractions to hide implementation details. Abstractions such as functions and interfaces allow you to deal with simpler concepts and hide complex details. However, remember that over-engineering your code with too many abstractions also causes cognitive load.
Simplify control flow. Functions with too many if statements or loops can be hard to understand since it is difficult to keep the entire control flow in your head. Hide complex logic in helper functions, and reduce nesting by using early returns to handle special cases.
Minimize mutable state. Stateless code is simpler to understand. For example, avoid mutable class fields when possible, and make types immutable.
Include only relevant details in tests. A test can be hard to follow if it includes boilerplate test data that is irrelevant to the test case, or relevant test data is hidden in helper functions.
Don’t overuse mocks in tests. Improper use of mocks can lead to tests that are cluttered with calls that expose implementation details of the system under test.

Learn more about cognitive load in the book The Programmer’s Brain, by Felienne Hermans.

Source: Google Testing Blog

Simplify Your Control Flows

By Jeff Hoy

When adding loops and conditionals, even simple code can become difficult to understand.
Consider this change:

if (commode.HasPreferredCustomer()) {	if (commode.HasPreferredCustomer()) {
commode.WarmSeat();	commode.WarmSeat();
	} else if (commode.CustomerOnPhone()) {
	commode.ChillSeat();
}	}

While the above change may seem simple, even adding a single else statement can make the code harder to follow since the complexity of code grows quickly with its size. Below we see the code surrounding the above snippet; the control flow on the right illustrates how much a reader needs to retain:

while (commode.StillOccupied()) {

if (commode.HasPreferredCustomer()) {

commode.WarmSeat();

} else if (commode.CustomerOnPhone()) {

commode.ChillSeat();

}

if (commode.ContainsKale()) {

commode.PrintHealthCertificate();

break;

}

Code Control Flow with 5 structures and 9 edges:

challenging for a reader to retain in memory.

In order to fully understand the code, the reader needs to keep the entire control flow in their head. However, the retention capacity of working memory is limited (source) Code path complexity will also challenge the reader, and can be measured using cyclomatic complexity.

To reduce cognitive overhead of complex code, push implementation logic down into functions and methods. For example, if the if/else structure in the above code is moved into an AdjustSeatTemp() method, the reviewer can review the two blocks independently, each having a much simpler control graph:

while (commode.StillOccupied()) {

commode.AdjustSeatTemp();

if (commode.ContainsKale()) {

commode.PrintHealthCertificate();

break;

}

3 control structures and 5 edges: easier to remember

Commode::AdjustSeatTemp()

with 2 structures and 4 edges

Avoiding complexity makes code easier to follow. In addition, code reviewers are more likely to identify logic errors, and maintainers are less likely to introduce complex code.

Source: Google Testing Blog

Improve Readability With Positive Booleans

By Max Kanat-Alexander

Reading healthy code should be as easy as reading a book in your native language. You shouldn’t have to stop and puzzle over what a line of code is doing. One small trick that can assist with this is to make boolean checks about something positive rather than about something negative.

Here’s an extreme example:

if not nodisable_kryponite_shield:

devise_clever_escape_plan()

else:

engage_in_epic_battle()

What does that code do? Sure, you can figure it out, but healthy code is not a puzzle, it’s a simple communication. Let’s look at two principles we can use to simplify this code.

1. Name your flags and variables in such a way that they represent the positive check you wish to make (the presence of something, something being enabled, something being true) rather than the negative check you wish to make (the absence of something, something being disabled, something being false).

if not enable_kryponite_shield:

devise_clever_escape_plan()

else:

engage_in_epic_battle()

That is already easier to read and understand than the first example.

2. If your conditional looks like “if not … else …” then reverse it to put the positive case first.

if enable_kryponite_shield:

engage_in_epic_battle()

else:

devise_clever_escape_plan()

Now the intention of the code is immediately obvious.

There are many other contexts in which this gives improvements to readability. For example, the command foo --disable_feature=False is harder to read and think about than
foo --enable_feature=True, particularly when you change the default to enable the feature.

There are some exceptions (for example, in Python, if foo is not None could be considered a “positive check” even though it has a “not” in it), but in general checking the presence or absence of a positive is simpler for readers to understand than checking the presence or absence of a negative.

Source: Google Testing Blog

The Secret to Great Code Reviews: Respect Reviewers’ Comments

By Marius Latinis

You prepared a code change and asked for a review. A reviewer left a comment you disagree with. Are you going to reply that you will not address the comment?

When addressing comments for your code reviewed by colleagues, find a solution that makes both you and the reviewer happy. The fact that a reviewer left a comment suggests you may be able to improve the code further. Here are two effective ways to respond:

When it’s easy for you to make an improvement, update the code. Improved code benefits future readers and maintainers. You will also avoid a potentially long and emotional debate with a reviewer.

If the comment is unclear, ask the reviewer to explain. To facilitate the process, talk directly with the reviewer through chat, or in person.

Let’s demonstrate with an example code review scenario:

You prepare a code change that modifies the following function:

3 // Return the post with the most upvotes.	3 // Return the post with the most upvotes.
	4 // Restrict to English if englishOnly = true.
4 Post findMostUpvotedPost(	5 Post findMostUpvotedPost(
5 List<Post> posts) {	6 List<Post> posts,
	7 boolean englishOnly) {
6 ...	8 ... // Old and new logic mixed together.
7 }	9 }

The code reviewer leaves the following comment:
alertreviewer
11:51 AM
The new function signature is too complex. Can we keep the signature unchanged?

Reply

You disagree with the comment that one additional parameter makes the signature too complex. Nevertheless, do not reject the suggestion outright.

There is another issue that might have prompted the comment: it is not the responsibility of this function to check the post’s language (https://en.wikipedia.org/wiki/Single-responsibility_principle).

You rewrite your code to address the reviewer’s comment:
ImmutableList<Post> englishPosts = selectEnglishPosts(posts); // Your new logic.
Post mostUpvotedEnglishPost = findMostUpvotedPost(englishPosts); // No change needed.

Now the code is improved, and both you and the reviewer are happy.

Source: Google Testing Blog

Else Nuances

By Sam Lee and Stan Chan

If your function exits early in an if statement, using or not using an else clause is equivalent in terms of behavior. However, the proper use of else clauses and guard clauses (lack of else) can help emphasize the intent of the code to the reader.

Consider the following guidelines to help you structure your functions:

Use a guard clause to handle special cases upfront, so that the rest of the code can focus on the core logic. A guard clause checks a criterion and fails fast or returns early if it is not met, which reduces nesting (see the Reduce Nesting article).

def parse_path(path: str) -> Path:

if not path:

raise ValueError(“path is empty.”)

else:

# Nested logic here.

...

def parse_path(path: str) -> Path:

if not path:

raise ValueError(“path is empty.”)

# No nesting needed for the valid case.

...

Use else if it is part of the core responsibility. Prefer to keep related conditional logic syntactically grouped together in the same if...else structure if each branch is relevant to the core responsibility of the function. Grouping logic in this way emphasizes the complementary nature of each condition. The complementary nature is emphasized explicitly by the else statement, instead of being inferred and relying on the resulting behavior of the prior return statement.

def get_favicon(self) -> Icon:

if self.user.id is None:

return Icon.SIGNED_OUT

if self.browser.incognito: return Icon.INCOGNITO

if not self.new_inbox_items:

return Icon.EMPTY;

return Icon.HAS_ITEMS

def get_favicon(self) -> Icon:

if self.user.id is None:

return Icon.SIGNED_OUT

elif self.browser.incognito:

return Icon.INCOGNITO

elif not self.new_inbox_items:

return Icon.EMPTY

else:

return Icon.HAS_ITEMS

# No trailing return is needed or allowed.

When it’s idiomatic, use a switch (or similar) statement instead of if...else statements. (switch/when in Go/Kotlin can accept boolean conditions like if...else.)

Not all scenarios will be clear-cut for which pattern to use; use your best judgment to choose between these two styles. A good rule of thumb is use a guard if it's a special case, use else if its core logic. Following these guidelines can improve code understandability by emphasizing the connections between different logical branches.

Source: Google Testing Blog

Use Abstraction to Improve Function Readability

By Palak Bansal and Mark Manley

Which version of the createPizza function below is easier to understand?

func createPizza(order *Order) *Pizza {

pizza := &Pizza{Base: order.Size,

Sauce: order.Sauce,

Cheese: “Mozzarella”}

if order.kind == “Veg” {

pizza.Toppings = vegToppings

} else if order.kind == “Meat” {

pizza.Toppings = meatToppings

}

oven := oven.New()

if oven.Temp != cookingTemp {

for (oven.Temp < cookingTemp) {

time.Sleep(checkOvenInterval)

oven.Temp = getOvenTemp(oven)

}

if !pizza.Baked {

oven.Insert(pizza)

time.Sleep(cookTime)

oven.Remove(pizza)

pizza.Baked = true

}

box := box.New()

pizza.Boxed = box.PutIn(pizza)

pizza.Sliced = box.SlicePizza(order.Size)

pizza.Ready = box.Close()

return pizza

}

func createPizza(order *Order) *Pizza {

pizza := prepare(order)

bake(pizza)

box(pizza)

return pizza

}

func prepare(order *Order) *Pizza {

pizza := &Pizza{Base: order.Size,

Sauce: order.Sauce,

Cheese: “Mozzarella”}

addToppings(pizza, order.kind)

return pizza

}

func addToppings(pizza *Pizza, kind string) {

if kind == “Veg” {

pizza.Toppings = vegToppings

} else if kind == “Meat” {

pizza.Toppings = meatToppings

}

func bake(pizza *Pizza) {

oven := oven.New()

heatOven(oven)

bakePizza(pizza, oven)

}

func heatOven(oven *Oven) { … }

func bakePizza(pizza *Pizza, oven *Oven) { … }

func box(pizza *Pizza) { … }

You probably said the right-hand side is easier, but why? The left side mixes together several levels of abstraction: low-level implementation details (e.g., how to heat the oven), intermediate-level functions (e.g., how to bake pizza), and high-level abstractions (e.g., preparing, baking, and boxing the pizza).

The right side is easier to follow because the functions have a consistent level of abstraction, providing a top-down narrative of the code’s logic. createPizza is a high-level function that delegates the preparing, baking, and boxing steps to lower-level specialized functions with intuitive names. Those functions, in turn, delegate to their own lower-level specialized functions (e.g., heatOven) until they reach a function that handles implementation details without needing to call other functions.

Avoid mixing different abstraction layers into a single function. Nest functions at equal abstraction levels to provide a narrative. This self-documenting style is simpler to follow, debug, and reuse.

You can learn more about this topic in the book Clean Code by Robert C. Martin.

googblogs.com

All Google blogs and Press in one site

Tag Archives: Code Health

What’s in a Name?

Source: Google Testing Blog

Let Code Speak for Itself

Source: Google Testing Blog

Exceptional Exception Handling

Source: Google Testing Blog

Clean Up Code Cruft

Source: Google Testing Blog

Write Clean Code to Reduce Cognitive Load

Source: Google Testing Blog

Simplify Your Control Flows

Source: Google Testing Blog

Improve Readability With Positive Booleans

Source: Google Testing Blog

The Secret to Great Code Reviews: Respect Reviewers’ Comments

Source: Google Testing Blog

Else Nuances

Source: Google Testing Blog

Use Abstraction to Improve Function Readability

Source: Google Testing Blog