Good practices for coding
Author:
Avoid ambiguous names
Take a look at the following example:
const MIN_PASSWORD = 6;
function checkPasswordLength(password) {
return password.length >= MIN_PASSWORD
}
At first glance the name MIN_PASSWORD doesn't tell us much about it, so let's fix it by renaming it to a more specific name.
const MIN_PASSWORD_LENGTH = 6;
function checkPasswordLength(password) {
return password.length >= MIN_PASSWORD_LENGTH
}
We still have some ambiguity in the function name. The current name does not tell us what it checks, so let's fix it.
const MIN_PASSWORD_LENGTH = 6;
function isPasswordLongEnough(password) {
return password.length >= MIN_PASSWORD_LENGTH
}
With the new name isPasswordLongEnough the prefix is tells us that the function will return a boolean.
These changes make the code readable and maintainable.
Single responsibility principle
Functions must do only one task. Check the code below.
let area = 0
function calculateAndUpdateArea(radius) {
const newArea = Math.PI * radius * radius;
area = newArea
return newArea
}
As you can see, the function name calculateAndUpdateArea tells us, by the word and, that it's doing two things: it calculates the area and also updates a global variable.
Let's fix it.
let area = 0
function calculateArea(radius) {
return Math.PI * radius * radius;
}
area = calculateArea(radius);
Now our function does one thing when called and the global variable is not updated within the function.
Don't use "Magic" values
"Magic" values are those values in our code whose meaning isn't clear, which makes the code harder to read. Take a look at the example below.
let price = 10
if (transactionType === 1) {
price += 1.1;
}
You might be guessing what the numbers 1 and 1.1 represent in the code. Let's fix that by giving those numbers a name.
const TAXABLE_TRANSACTION_TYPE = 1
const TAX_MULTIPLE = 1.1;
let price = 10
if (transactionType === TAXABLE_TRANSACTION_TYPE) {
price += TAX_MULTIPLE;
}
Now the code is easier to understand.
Formatting
Try to keep the same format for everything. Take a look at the example below.
const name = "Conner";
let age=25;
function getUserInfo(){
console.log("User info:");
console.log("Name: " + name);
console.log(`Age: ${age}`);
}
As you can see, the example lacks consistency: the code has different levels of indentation, different ways to print messages, and more.
Let's start by fixing the most obvious error: the indentation levels.
const name = "Conner";
let age=25;
function getUserInfo(){
console.log("User info:");
console.log("Name: " + name);
console.log(`Age: ${age}`);
}
Now let's fix the variable declarations: one uses let while the other uses const. There's also a lack of spaces in the declaration of age.
const name = "Conner";
const age = 25;
function getUserInfo(){
console.log("User info:");
console.log("Name: " + name);
console.log(`Age: ${age}`);
}
Lastly, let's fix quotes and methods to print messages. For this case, we'll keep using the template literal method.
const name = "Conner";
const age = 25;
function getUserInfo(){
console.log("User info:");
console.log(`Name: ${name}`);
console.log(`Age: ${age}`);
}
Failing fast
This principle means that if you are handling possible errors within your code, you should do it as early as possible to avoid doing unnecessary work. Check the function below.
function getUppercaseInput(input){
const result = input?.toUpperCase?.();
if (typeof input !== 'string' || input.trim() === '') {
throw new Error('Invalid input');
}
return result;
}
To fix the code above, we can move the if statement that throws the error to the top of the function so there is no need to assign a value to result.
function getUppercaseInput(input){
if (typeof input !== 'string' || input.trim() === '') {
throw new Error('Invalid input');
}
return input.toUpperCase();
}
This allows us to even simplify the return statement, since we don't have to do extra checks.
Avoid deeply nested code
Take a look at this code
function main() {
let total = 0;
if (!is_maintenance_period) {
if (is_authenticated_user) {
if (is_authorized_user) {
for (let product of cart) {
switch (product.type) {
case "alcohol":
total += alcohol_tax;
break;
case "electronics":
total += electronics_tax;
break;
default:
total += general_tax;
}
}
} else {
console.log("User not authorized");
}
} else {
console.log("Invalid user credentials");
}
} else {
console.log("Feature unavailable");
}
}
As you read the code, you need to remember each condition that lets you traverse deeper into it.
Let's apply a few concepts to improve this code.
Inversion
This concept is about inverting conditionals. Applying it lets us remove n levels from the nested structure.
function main() {
let total = 0;
if (is_maintenance_period) {
console.log("Feature unavailable");
return;
}
if (!is_authenticated_user) {
console.log("Invalid user credentials");
return;
}
if (!is_authorized_user) {
console.log("User not authorized");
return;
}
for (const product of cart) {
switch (product.type) {
case "alcohol":
total += alcohol_tax;
break;
case "electronics":
total += electronics_tax;
break;
default:
total += general_tax;
break;
}
}
}
Now there is no need to remember each condition to traverse the code.
Merge related if statements
In our example, two conditions are related: the one that checks is_authenticated_user and the one that checks is_authorized_user. We can merge them, but it's important to know that doing so makes your error messages less specific.
function main() {
let total = 0;
if (is_maintenance_period) {
console.log("Feature unavailable");
return;
}
if (!is_authenticated_user || !is_authorized_user) {
console.log("User not authorized");
return;
}
for (const product of cart) {
switch (product.type) {
case "alcohol":
total += alcohol_tax;
break;
case "electronics":
total += electronics_tax;
break;
default:
total += general_tax;
break;
}
}
}
Extraction
This technique means we search for complex logic and, as the name indicates, extract it into its own function.
function main() {
let total = 0;
if (is_maintenance_period) {
console.log("Feature unavailable");
return;
}
if (!is_authenticated_user || !is_authorized_user) {
console.log("User not authorized");
return;
}
for (let product of cart) {
switch (product.type) {
case "alcohol":
total += alcohol_tax;
break;
case "electronics":
total += electronics_tax;
break;
default:
total += general_tax;
break;
}
}
}
Following the previous example, we have two cases of complex logic we can extract. The first is the if statement that verifies whether the user is authorized, so let's extract it into its own function.
function is_valid_user() {
return is_authenticated_user && is_authorized_user;
}
function main() {
let total = 0;
if (is_maintenance_period) {
console.log("Feature unavailable");
return;
}
if (!is_valid_user()) {
console.log("User not authorized");
return;
}
for (const product of cart) {
switch (product.type) {
case "alcohol":
total += alcohol_tax;
break;
case "electronics":
total += electronics_tax;
break;
default:
total += general_tax;
break;
}
}
}
Now we can apply extraction to the taxes calculation.
function calculate_taxes(product) {
switch (product.type) {
case "alcohol":
return alcohol_tax;
case "electronics":
return electronics_tax;
default:
return general_tax;
}
}
function is_valid_user() {
return is_authenticated_user && is_authorized_user;
}
function main() {
let total = 0;
if (is_maintenance_period) {
console.log("Feature unavailable");
return;
}
if (!is_valid_user()) {
console.log("User not authorized");
return;
}
for (const product of cart) {
total += calculate_taxes(product);
}
}
Now whoever reads the main function can easily know what the code does without reading all the other functions.
Avoid code duplication
Take a look at the following functions
func getUser(w http.ResponseWriter, r *http.Request){
userID := r.URL.Path[len("/user/"):]
cachesMux.Lock()
user, found := cache[userID]
cachesMux.Unlock()
if !found{
query := "SELECT user_name, email FROM users WHERE user_id = ?"
stmt, _ := db.Prepare(query)
var u User
_ = stmt.QueryRow(userID).Scan(&u.Username, &u.Email)
cachesMux.Lock()
cache[userID] = u
cachesMux.Unlock()
user = u
}
w.Header().Set("Content-Type", "application/json")
JSON.NewEncoder(w).Encode(user)
}
func getUsers(w http.ResponseWriter, r *http.Request){
var requestBody RequestBody
JSON.NewDecoder(r.Body).Decode(&requestBody)
userIDs := requestBody.userIDs
var users []User
for _, userID := range userIDs{
cachesMux.Lock()
user, found := cache[userID]
cachesMux.Unlock()
if !found{
query := "SELECT user_name, email FROM users WHERE user_id = ?"
stmt, _ := db.Prepare(query)
var u User
_ = stmt.QueryRow(userID).Scan(&u.Username, &u.Email)
cachesMux.Lock()
cache[userID] = u
cachesMux.Unlock()
user = u
}
users = append(users, user)
}
w.Header().Set("Content-Type", "application/json")
JSON.NewEncoder(w).Encode(users)
}
This code seems straightforward: one function gets data for a single user, the other for multiple users.
Imagine you're tasked with removing the caching behavior. Since it's found in multiple locations, you'll need to search everywhere to make the changes, which can make you miss spots that need updating.
To solve this, we can start by applying extraction to the duplicated code, in this case, the code that gets user data from the database.
The new function should look like:
func getSingleUser(userID string) User {
cachesMux.Lock()
user, found := cache[userID]
cachesMux.Unlock()
if !found{
query := "SELECT user_name, email FROM users WHERE user_id = ?"
stmt, _ := db.Prepare(query)
var u User
_ = stmt.QueryRow(userID).Scan(&u.Username, &u.Email)
cachesMux.Lock()
cache[userID] = u
cachesMux.Unlock()
user = u
}
return user
}
Now the function can be called by both original functions
func getUser(w http.ResponseWriter, r *http.Request){
userID := r.URL.Path[len("/user/"):]
user := getSingleUser(userID)
w.Header().Set("Content-Type", "application/json")
JSON.NewEncoder(w).Encode(user)
}
func getUsers(w http.ResponseWriter, r *http.Request){
var requestBody RequestBody
JSON.NewDecoder(r.Body).Decode(&requestBody)
userIDs := requestBody.userIDs
var users []User
for _, userID := range userIDs{
user := getSingleUser(userID)
users = append(users, user)
}
w.Header().Set("Content-Type", "application/json")
JSON.NewEncoder(w).Encode(users)
}
We still have duplicate code in both functions: the code that writes the response to the client. The extracted code should look like this:
func writeResponse(w http.ResponseWriter, response interface{}) {
w.Header().Set("Content-Type", "application/json")
JSON.NewEncoder(w).Encode(response)
}
Replacing the duplicated code in the original functions
func getUser(w http.ResponseWriter, r *http.Request){
userID := r.URL.Path[len("/user/"):]
user := getSingleUser(userID)
writeResponse(w, user)
}
func getUsers(w http.ResponseWriter, r *http.Request){
var requestBody RequestBody
JSON.NewDecoder(r.Body).Decode(&requestBody)
userIDs := requestBody.userIDs
var users []User
for _, userID := range userIDs{
user := getSingleUser(userID)
users = append(users, user)
}
writeResponse(w, users)
}
Now that we extracted the duplicated code, the code is more readable and the changes to the caching behavior have to be done only once in the new function.
Don't use names that only you understand
Take a look at the code below
type P struct {
N string
P float64
}
func main(){
ps := []P{
{"Apple", 0.50}
{"Banana", 0.25}
{"Orange", 0.75}
{"Pear", 0.30}
}
t := 0.08
var tc float64 = 0
for _, p := range ps {
tc += p.P + (p.P * t)
}
ac := tc / float64(len(ps))
fmt.Println(tc)
fmt.Println(ac)
}
It's almost impossible to tell what this code does because of how things are named. Even if you wrote it yourself, you might not understand it in the future. That's why you should always use meaningful names.
type Product struct {
Name string
Price float64
}
func main(){
products := []Product{
{"Apple", 0.50}
{"Banana", 0.25}
{"Orange", 0.75}
{"Pear", 0.30}
}
tax := 0.08
var totalCost float64 = 0
for _, product := range products {
totalCost += product.Price + (product.Price * tax)
}
averageCost := totalCost / float64(len(products))
fmt.Println(totalCost)
fmt.Println(averageCost)
}
Now that we are using meaningful names for all variables we can easily understand what is happening.
Avoid excessive comments
Check the code below
// Function to check if a number is prime
function isPrime(number) {
// Check if number is less than 2
if (number < 2) {
// If less than 2, not a prime number
return false;
}
// At least 1 divisor must be less than or equal to the square root, so we can stop there
for (let i = 2; i <= Math.sqrt(number); i++) {
// Check if number is divisible by i
if (number % i == 0) {
// if divisible, number is not prime
return false;
}
}
// After all checks, if not divisible by any i, number is prime
return true;
}
It's easy to tell that the code above has an excessive number of comments. Let's fix it by removing the unnecessary ones.
- The comment
// Function to check if a number is primeis not necessary since the function nameisPrimealready tells us what the function does. - The following comments share the same problem, which is that they are trying to explain very simple code, so they are not necessary.
// Check if number is less than 2// If less than 2, not a prime number// Check if number is divisible by i// if divisible, number is not prime// After all checks, if not divisible by any i, number is prime
So now our code looks like this:
function isPrime(number) {
if (number < 2) {
return false;
}
// At least 1 divisor must be less than or equal to the square root, so we can stop there
for (let i = 2; i <= Math.sqrt(number); i++) {
if (number % i == 0) {
return false;
}
}
return true;
}
Only the comment // At least 1 divisor must be less than or equal to the square root, so we can stop there is considered a good comment in this case, since the logic in for (let i = 2; i <= Math.sqrt(number); i++) might be difficult to quickly understand.
Optimize your code
Take a look at this example:
function countingSort(my_array, min, max) {
let count = new Array(max - min + 1).fill(0);
my_array.forEach((element) => {
count[element - min]++;
})
let index = 0;
for (let i = min; i <= max; i++) {
while (count[i - min] > 0) {
my_array[index++] = i;
count[i - min]--;
}
}
return my_array;
}
const my_array = [4, 2, 2, 8, 3 , 3 , 1]
console.log(countingSort(my_array, 1, 8)); // Output [1, 2, 2, 3, 3, 4, 8]
Sometimes we can optimize our code using tools that already exist. For example, in this case the function countingSort isn't necessary; we can use the .sort() method directly on the array to achieve the same output.
const my_array = [4, 2, 2, 8, 3 , 3 , 1]
console.log(my_array.sort()); // Output [1, 2, 2, 3, 3, 4, 8]