Clean Up Code Smells with Clean Code: TypeScript Edition
source link: https://blog.bitsrc.io/what-are-code-smells-and-how-clean-code-can-help-typescript-version-990697a87f46
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Clean Up Code Smells with Clean Code: TypeScript Edition
Common Code Smells in JavaScript and TypeScript and How To Fix Them
What is a Code Smell?
According to Wikipedia, in computer programming, a code smell is any characteristic in the source code of a program that possibly indicates a deeper problem.
To simplify the definition, code smells are a result of unclean or misguided programming.
Code smells have many severities, from minor code smells that maybe acceptable, to code smells that make program development becomes much more complicated and expensive.
Code Smell in JavaScript and TypeScript
Although some of the code smells are universal like:
- Too much comments
- Too much of “TODO”
- Too many parameters on function
In this article I will only focus on Code Smells that are common in JavaScript and TypeScript.
Redundant casts and non-null assertions should be avoided
The TypeScript compiler automatically casts a variable to the relevant type inside conditionals where it is possible to infer the type (because typeof
, instanceof
, etc was used). This compiler feature makes casts and not-null
assertions unnecessary.
Should avoid:
function getName(x?: string | UserName) {
if (x) {
console.log("Getting name for " + x!); // Noncompliant
if (typeof x === "string")
return (x as string); // Noncompliant
else
return (x as UserName).name; // Noncompliant
}
return "NoName";
}
Do this instead:
function getName(x?: string | UserName) {
if (x) {
console.log("Getting name for " + x);
if (typeof x === "string")
return x;
else
return x.name;
}
return "NoName";
}
Ternary operators should not be nested
Ternary operators are widely used in JavaScript because of its simplicity and easy to read. But sometimes we abused it too much. Instead, we should use another line to express the nested operation as a separate statement.
Should avoid:
function getReadableStatus(job) {
return job.isRunning() ? "Running" : job.hasErrors() ? "Failed" : "Succeeded "; // Noncompliant
}
Do this instead:
function getReadableStatus(job) {
if (job.isRunning()) {
return "Running";
}
return job.hasErrors() ? "Failed" : "Succeeded";
}
Array-mutating methods should not be used misleadingly
Many of JavaScript’s Array
methods return an altered version of the array while leaving the source array intact. reverse
and sort
do not fall into this category. Instead, they alter the source array in addition to returning the altered version, which is likely not what was intended.
This rule raises an issue when the return values of these methods are assigned, which could lead maintainers to overlook the fact that the original value is altered.
Should avoid:
var b = a.reverse(); // Noncompliant
var d = c.sort(); // Noncompliant
Do this instead:
var b = [...a].reverse();
a.reverse();
c.sort(); // this sorts array in place
Assignments should not be redundant
The transitive property says that if a == b
and b == c
, then a == c
. In such cases, there’s no point in assigning a
to c
or vice versa because they’re already equivalent.
This rule raises an issue when an assignment is useless because the assigned-to variable already holds the value on all execution paths.
Should avoid:
a = b;
c = a;
b = c; // Noncompliant: c and b are already the same
Do this instead:
a = b;
c = a;
Do not abuse “Any” in TypeScript
Nothing much to say about this, you should only use “any” on legacy libraries that aren’t type safe, in 99% of cases we can avoid “any” to have type safe
Type aliases should be used
Union and intersection types are convenient but can make code harder to read and maintain. So if a particular union or intersection is used in multiple places, the use of a type alias is recommended.
Should avoid:
function foo(x:string|null|number) { // Noncompliant
// ...
}
function bar(x:string|null|number) {
// ...
}
function zoo(): string|null|number {
return null;
}
Do this instead:
type MyType = string | null | number;
function foo(x: MyType) {
// ...
}
function bar(x: MyType) {
// ...
}
function zoo(): MyType {
return null;
}
“undefined” should not be passed as the value of optional parameters
Unlike in JavaScript, where every parameter can be omitted, in TypeScript you need to explicitly declare this in the function signature. Either you add ?
in the parameter declaration and undefined
will be automatically applied to this parameter. Or you add an initializer with a default value in the parameter declaration. In the latter case, when passing undefined
for such parameter, default value will be applied as well. So it’s better to avoid passing undefined
value to an optional or default parameter because it creates more confusion than it brings clarity. Note, that this rule is only applied to the last arguments in function call.
Should avoid:
function foo(x: number, y: string = "default", z?: number) {
// ...
}
foo(42, undefined); // Noncompliant
foo(42, undefined, undefined); // Noncompliant
foo(42, undefined, 5); // OK, there is no other way to force default value for second parameter
Do this instead:
function foo(x: number, y: string = "default", z?: number) {
// ...
}
foo(42);
Regular expressions should not contain multiple spaces
Multiple spaces in a regular expression can make it hard to tell how many spaces should be matched. It’s more readable to use only one space and then indicate with a quantifier how many spaces are expected.
Should avoid:
const pattern = /Hello, world!/;
Do this instead:
const pattern = /Hello, {3}world!/;
“delete” should not be used on arrays
The delete
operator can be used to remove a property from any object. Arrays are objects, so the delete
operator can be used here too, but if it is, a hole will be left in the array because the indexes/keys won’t be shifted to reflect the deletion.
The proper method for removing an element at a certain index would be:
Array.prototype.splice
- add/remove elements from the arrayArray.prototype.pop
- add/remove elements from the end of the arrayArray.prototype.shift
- add/remove elements from the beginning of the array
Should avoid:
var myArray = ['a', 'b', 'c', 'd'];
delete myArray[2]; // Noncompliant. myArray => ['a', 'b', undefined, 'd']
console.log(myArray[2]); // expected value was 'd' but output is undefined
Do this instead:
var myArray = ['a', 'b', 'c', 'd'];
// removes 1 element from index 2
removed = myArray.splice(2, 1); // myArray => ['a', 'b', 'd']
console.log(myArray[2]); // outputs 'd'
Imports from the same modules should be merged
Multiple imports from the same module should be merged together to improve readability.
Should avoid:
import { B1 } from 'b';
import { B2 } from 'b'; // Noncompliant
Do this instead:
import { B1, B2 } from 'b';
Conclusion
There are a lot of minor code smells in JavaScript or TypeScript that I couldn’t list them all, just selected major code smells.
If you want to know more about code smells in TypeScript, check out this page
Build apps with reusable components like Lego
Bit’s open-source tool help 250,000+ devs to build apps with components.
Turn any UI, feature, or page into a reusable component — and share it across your applications. It’s easier to collaborate and build faster.
Split apps into components to make app development easier, and enjoy the best experience for the workflows you want:
→ Micro-Frontends
→ Design System
→ Code-Sharing and reuse
→ Monorepo
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK