Introduction
Clean code is a standard of code containing a set of rules and approaches in writing and forming code that make it easy to read, write, understand, and maintain.
In this series of articles, we will take a look at common mistakes and problems in writing code and the ways it could be improved.
All code problems I divided into several groups, and in this article, we will take a look at functions and methods.
Methods-initializers or top-level methods
a. Problems
b. Best practices
Methods-handlers
a. Problems
b. Best practices
Common problems
a. More than one task in one method
b. The name of the function doesn’t match what it does
c. More than 3 parameters
d. Boolean flags as parameters
e. Nested conditionals
a. Problems
In this type of method, we invoke a list of initializations: fields, factories, observers, child classes, handlers, listeners, and so on. As usual, it is needed in the beginning of the application start or module loading.
Mostly, each initialization has its own logic and particular order. To keep it clean, readable, and understandable, we should follow the next practices.
b. Best practices
init
prefixexport class App {
init(): void {
this.initMainPage();
this.initRouteChange();
this.initDefaultState();
}
private initMainPage(): void {
// logic
}
private initRouteChange(): void {
// logic
}
private initDefaultState(): void {
// logic
}
}
const app = new App();
In this example, we have a top-level method init
and three types of initialization in it: main page render, router activation, and setting the application default state. In this way, we make our code atomic and flexible and follows the rule one method - one task.
a. Problems
This type of method handles user events that occur in templates or just callbacks. In most cases, we need to have different scenarios depending on conditions at the moment of function invocation. The conditions could be very different: state of any variables, type of event or else.
b. Best practices
To make our handlers readable, we should follow these simple rules:
In this example, we moved functions for each condition to separate methods:
onClick(event: Event): void {
if (this.state.facetsLoaded) {
this.processMessage(event);
} else {
this.closeConnection(event);
}
}
In this example, function checkFacetsUpdate
checks if facets are updated, and if not, it causes side effects.
// Bad
checkFacetsUpdate(facets: Factes[]): boolean {
const facet = facets.find((facet) => facet.realiTime.length > 0);
if (facet.hasOwnProperty('real')) {
return true;
}
updateFacets();
return false;
}
Rule: one method - one task
To follow this rule we should strictly check only facets update; additional logic should be outside the function.
// Better
export function checkFacetsUpdate(facets: Factes[]): boolean {
const facet = facets.find(facet => !!facet.realiTime.length);
return facet.hasOwnProperty('real');
}
const isFacetsUpdated = checkFacetsUpdate(facets);
if (!isFacetsUpdated) {
updateFacets();
}
Here expected that updateUser
method should just update the user in all cases. But in reality, we have the condition for this update inside the function when the function logic will not be executed.
// Bad
export function updateUser(user: User): void {
if (user.role !== User.Admin) {
// logic for user update
}
}
Rule: The name of the function should strictly describe what it does.
There are some options to follow this rule:
updateUserIfRoleAdmin
// Better
export function updateUser(user: User): void {
// logic
}
}
if (user.role !== User.Admin) {
updateUser(user)
}
In this example, there are three parameters in the function signature.
// Bad
export function setChildren(
parents: TopicType[],
parentId: string,
children: TopicType[],
selectedIds: string[]): TopicType[] {
// logic
}
Rule: no more than 3 parameters
To follow this rule, we should pass one object-like parameter following a particular interface where all fields will be described.
// Better
interface SelectedTopic {
parents: TopicType[],
parentId: string,
children: TopicType[],
selectedIds: string[]
}
export function setChildren(selectedTopic: SelectedTopic): TopicType[] {
// logic
}
In this example, I see that when method searchTopic
is invoked, we just pass boolean parameters that are not clear at the moment of function invocation. Of course, we can go to the function signature and take a look at the realization, but it is bad practice to write a code that is not readable. Moreover, our function searchTopic
executes different scenarios depending on these flags, and we break the rule of one method - one task at the same time.
// Bad
class TopicService {
searchTopic(isParentTopic: boolean, isChildTopic: boolean): Topic {
if (isParentTopic) {
// logic
}
}
}
const topicService = new TopicService();
topicService.searchTopic(true, false);
Rule: Avoid flags in arguments and make separate methods for each flag
// Better
class TopicService {
private searchParentTopic(): Topic {
// logic
};
private searchChildTopic(): Topic {
// logic
};
searchTopic(): void {
if (isParentTopic) {
this.searchParentTopic();
}
if (isChildTopic) {
this.searchFacetsTopic();
}
}
}
const topicService = new TopicService();
Let’s take a look at very frequent situations with conditional assignment and nested if else
blocks.
// Bad
function makePrices(): Price[] {
let prices;
if (isFruitPrice) {
prices = makeFruitPraces();
} else {
if (isVegePrice) {
prices = makeVegePrice();
} else {
if (isCandyPrice) {
prices = makeCandyPrice();
} else {
prices = makePrice();
}
}
}
return prices;
}
Rule: replace nested conditionals with guard clauses
In case we have a conditional assignment with a return, it would be good practice just to make a return when the condition is met.
// Better
function makePrices(): Price[] {
if (isFruitPrice) return makeFruitPraces();
if (isVegePrice) return makeVegePrice();
if (isCandyPrice) return makeCandyPrice();
return makePrice();
}
In this article, we took a look at very frequent code smells in functions and methods and the ways they could be cleaned. Following these rules, we will write maintainable and readable code that will be easy to debug, read, and develop. Certainly, the category of problems is wider, and in the next articles, we will take a look at other categories.