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, 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;
are = newArea
return newArea
}
As you can see the function name calculateAndUpdateArea easily tells us by the word and that is doing two things, 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 that are not clear what they represent, making 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 for example what the number 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, etc.
Let's start by fixing the most obvious errors which are 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 is using let while the other is using const, also there is 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 will 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 code, 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() {
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 allows you to traverse deeper in the code.
Let's apply a few concepts to improve this code.
Inversion
This concept is about inverting conditionals, applying this concept will allow us to remove n levels from the nested structure.
function main() {
if (is_maintenance_period) {
console.log("Feature unavailable");
}
if (!is_authenticated_user) {
console.log("Invalid user credentials");
}
if (!is_authorized_user) {
console.log("User not authorized");
}
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 we have two conditions that are related which are the condition that checks is_authenticated_user and the one that checks is_authorized_user so we can merge them, it is important to know that this method will make your error messages less specific.
function main() {
if (is_maintenance_period) {
console.log("Feature unavailable");
}
if (!is_authenticated_user || !is_authorized_user) {
console.log("User not authorized");
}
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 that we are going to search complex logic and as the technique's name indicates we are going to extract it and place them in their own function.
function main() { if (is_maintenance_period) { console.log("Feature unavailable"); } if (!is_authenticated_user || !is_authorized_user) { console.log("User not authorized"); }
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 with the previous example we have two cases of complex logic that we can extract, the first one is the if statement that verifies if the user is authorized, so lets extract it in its own function.
function isValidUser() {
return isAuthenticatedUser && isAuthorizedUser;
}
function main() {
if (isMaintenancePeriod) {
console.log("Feature unavailable");
}
if (!isValidUser()) {
console.log("User not authorized");
}
for (const product of cart) {
switch (product.type) {
case "alcohol":
total += alcoholTax;
break;
case "electronics":
total += electronicsTax;
break;
default:
total += generalTax;
break;
}
}
}
Now we can apply extraction to the taxes calculation.
def is valid_user():
return is_authenticated_user and is_authorized_user:
function calculate_taxes(product)
switch (product.type) {
case "alcohol":
total += alcoholTax;
break;
case "electronics":
total += electronicsTax;
break;
default:
total += generalTax;
break;
}
function isValidUser() {
return isAuthenticatedUser && isAuthorizedUser;
}
function main() {
if (isMaintenancePeriod) {
console.log("Feature unavailable");
}
if (!isValidUser()) {
console.log("User not authorized");
}
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[length("/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 getUser(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 very straightforward: one gets only data from one user and the other from multiple users.
Imagine that you are tasked to remove the caching behavior, the caching behavior is found in multiple locations, you will be required to search everywhere to make the changes, this can make you miss spots where changes are required.
To solve this we can start by applying extraction with the duplicated code, in this case the code that gets user data from the database.
The new function should look like:
def 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[length("/user/")]
user := getSingleUser(userID)
w.Header().Set("Content-Type", "application/json'")
JSON.NewEncoder(w).Encode(user)
}
func getUser(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 some duplicate code in both functions which is the code in charge of writing the response to the client, the extracted code should look like:
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[length("/user/")]
user := getSingleUser(userID)
writeResponse(w, users)
}
func getUser(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 is almost impossible to determine what this code does, because of how things are named, even if you wrote this code, in the future you might not be able to understand it, that is why 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 but less than 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 is easy to tell that the code above has an excessive amount of comments, let's fix it, by removing the unnecessary comments.
- 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// Check if number is divisible by i// 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 but less than 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 but less than square root, so we can stop there is considered a good comment in this case since the logic for (let i = 2; i <= Math.sqrt(number); i++) it might be difficult to quickly understand it.
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, , 3, 3, 4, 8]
Sometimes we can optimize our code using tools that already exist, for example for this case the function countingSort is not necessary we can use the method .sort() directly in the array to achieve the same output.
const amy_array = [4, 2, 2, 8, 3 , 3 , 1]
console.log(my_array.sort())); // Output [1, 2, , 3, 3, 4, 8]