Keep your functions small
Keeping your functions small is a great way to improve the readability and maintainability of your code. In this tutorial, we will refactor one large function to multiple smaller ones.
The problem with long functions
Maybe you recognize this situation?
boss - "Hey could you take a look at this code, it does not seem to work. It should be a quick fix and it will only take 5 min.
You - Give a smile back to your boss and says, "Sure. no problem I got this."
You find the function and realize it's over 1000 rows long, 5 levels of nested if statements, variables with nondescriptive names and on top of that it seems to do 10 different things.
You finally found the problem after 3 hours, was it a hard problem? Probably not. However, because of the state of the code, you had to spend 2 hours 55 min just to understand how it works.
One giant function
Have a look at the code below, can you figure out what it's doing?
const EMPLOYEE = "employee";
const FREELANCE = "freelance";
const employees = [
{
name: "Sara",
id: 1,
type: EMPLOYEE,
sales: 30,
amountPerSoldItem: 16,
active: true
},
{
name: "Tom",
id: 2,
type: EMPLOYEE,
sales: 25,
amountPerSoldItem: 17,
active: true
},
{
name: "Anna",
id: 3,
type: EMPLOYEE,
sales: 12,
amountPerSoldItem: 12,
active: true
},
{
name: "Peter",
id: 4,
type: EMPLOYEE,
sales: 23,
amountPerSoldItem: 13,
active: false
}
];
const freelancers = [
{
name: "Aaron",
type: FREELANCE,
rate: 14,
workedHours: [2, 2, 6, 5, 4]
},
{
name: "Martin",
type: FREELANCE,
rate: 20,
workedHours: [8, 8, 3]
},
{
name: "Janet",
type: FREELANCE,
rate: 14,
workedHours: [7]
},
{
name: "Nicole",
type: FREELANCE,
rate: 25,
workedHours: [6, 5, 8, 2, 4, 6]
}
];
const generateSalaryReport = id => {
let person;
let salary = 0;
const minimumFreelanceHours = 10;
for (let i = 0; i < employees.length; i++) {
if (employees[i].id === id && employees[i].active) {
person = employees[i];
}
}
if (!person) {
for (let i = 0; i < freelancers.length; i++) {
if (freelancers[i].id === id) {
let totalHours = 0;
for (let x = 0; x < freelancers[i].workedHours; x++) {
totalHours += freelancers[i].workedHours[x];
}
if (totalHours > minimumFreelanceHours) {
person = freelancers[i];
}
}
}
}
if (!person) {
return;
}
if (person.type === EMPLOYEE) {
const bonusEveryN = 10;
const bonusAmount = 30;
const totalBonus = Math.floor(person.sales / bonusEveryN) * bonusAmount;
salary = person.sales * person.amountPerSoldItem + totalBonus;
}
if (person.type === FREELANCE) {
salary = person.workedHours.reduce((a, b) => person.rate * a + b, 0);
}
let rating = 1;
if(person.sales <= 1) {
return 1;
}
const calculateRating = Math.floor(person.sales / 5)
rating = calculateRating > 5 ? 5 : calculateRating;
const symbol = "⭐️";
const symbols = symbol.repeat(rating);
const report = {
name: person.name,
id: person.id,
amountToPay: salary,
rating,
date: new Date()
};
const message = `
👍👍👍👍👍👍👍👍👍
name: ${report.name}
amountToPay: ${report.amountToPay}
rating: ${symbols},
date: ${report.date.getDate()}-${report.date.getMonth() +
1}-${report.date.getFullYear()}
👍👍👍👍👍👍👍👍👍
`;
console.log(message);
const parsedData = JSON.parse(localStorage.getItem("reports")) || {};
const userReport = parsedData[person.id] || [];
userReport.push(report);
parsedData[person.id] = userReport;
localStorage.setItem("reports", JSON.stringify(parsedData));
return report;
};
generateSalaryReport(1);
Here I have created a little program that calculates the salary for employees and freelancers. This is just a made-up example but you could use the same technique for all types of code.
The problem with this code is in generateSalaryReport
it's doing to many stuff. Code should be read as English. Here is a list of all the stuff generateSalaryReport
is doing.
- Finding employee or freelancer
- calculate bonus for an employee
- calculate salary
- give a rating
- generate report
- log message
- save report
Let's now try to refactor this into code that is easier to read.
Simplify by replacing for lops with higher-order functions like map, reduce, find and so on.
We want to find an employee that matches the id and is active. This code works, however, it's quite verbose and we have to manually iterate over the array.
for (let i = 0; i < employees.length; i++) {
if (employees[i].id === id && employees[i].active) {
person = employees[i];
}
}
We can hide away some of the complexity by using a higher-order function.
In case you don't know what a higher-order function is, here is a quick description. A higher-order function is a function that takes a function as one of its parameters or returns another function.
let person = employees.find(employee => employee.id === id && employee.active);
From 5 rows down to 1 great start! let's move on to the next piece of code.
If we were not able to find an employee, we have to see if the person exists in our freelancers array.
if (!person) {
for (let i = 0; i < freelancers.length; i++) {
if (freelancers[i].id === id) {
let totalHours = 0;
for (let x = 0; x < freelancers[i].workedHours; x++) {
totalHours += freelancers[i].workedHours[x];
}
if (totalHours > minimumFreelanceHours) {
person = freelancers[i];
}
}
}
}
That's a lot of code to find a freelancer that has a matching id and has worked a minimum of 8 hours.
if (!person) {
const freelancer = freelancers.find(freelancer => freenlancer.id === id);
const workedHours = freelancer ? freelancer.workedHours : [];
const workedMoreThenMinHours = workedHours.reduce((a, b) => a + b, 0) >= minimumFreelanceHours;
if(workedMoreThenMinHours) {
person = freelancer;
}
}
Isn't this much better? we managed to reduce the length of our code and made it much easier to read.
Current code
...
const generateSalaryReport = id => {
let salary = 0;
let person = employees.find(employee => employee.id === id && employee.active);
const minimumFreelanceHours = 10;
if (!person) {
const freelancer = freelancers.find(freelancer => freenlancer.id === id);
const workedHours = freelancer ? freelancer.workedHours : [];
const workedMoreThenMinHours = workedHours.reduce((a, b) => a + b, 0) >= minimumFreelanceHours;
if (workedMoreThenMinHours) {
person = freelancer;
}
}
if (!person) {
return;
}
if (person.type === EMPLOYEE) {
const bonusEveryN = 10;
const bonusAmount = 30;
const totalBonus = Math.floor(person.sales / bonusEveryN) * bonusAmount;
salary = person.sales * person.amountPerSoldItem + totalBonus;
}
if (person.type === FREELANCE) {
salary = person.workedHours.reduce((a, b) => person.rate * a + b, 0);
}
let rating = 1;
if(person.sales <= 1) {
return 1;
}
const calculateRating = Math.floor(person.sales / 5)
rating = calculateRating > 5 ? 5 : calculateRating;
const symbol = "⭐️";
const symbols = symbol.repeat(rating);
const report = {
name: person.name,
id: person.id,
amountToPay: salary,
rating,
date: new Date()
};
const message = `
👍👍👍👍👍👍👍👍👍
name: ${report.name}
amountToPay: ${report.amountToPay}
rating: ${symbols},
date: ${report.date.getDate()}-${report.date.getMonth() +
1}-${report.date.getFullYear()}
👍👍👍👍👍👍👍👍👍
`;
console.log(message);
const parsedData = JSON.parse(localStorage.getItem("reports")) || {};
const userReport = parsedData[person.id] || [];
userReport.push(report);
parsedData[person.id] = userReport;
localStorage.setItem("reports", JSON.stringify(parsedData));
return report;
};
generateSalaryReport(1);
While we have improved our code a little bit the big problem still remains.
Refactor into smaller functions
Currently, our generateSalaryReport
is aware of how to find the person, let's create a separate function for that, we might want to use it in other places.
const findPayablePerson = (id) => {
let person = employees.find(employee => employee.id === id && employee.active);
if (!person) {
const freelancer = freelancers.find(freelancer => freenlancer.id === id);
const workedHours = freelancer ? freelancer.workedHours : [];
const workedMoreThenMinHours = workedHours.reduce((a, b) => a + b, 0) >= minimumFreelanceHours;
if (workedMoreThenMinHours) {
person = freelancer;
}
}
return person;
};
We put the logic on how to find a person in a separate function, you could take this further and separate how to find employees and freelancers in a separate function but I will stop here.
Replace the old code and call findPayablePerson
.
const generateSalaryReport = id => {
let salary = 0;
const person = findPayablePerson(id);
if (!person) {
return;
}
...
}
Let's create a method for how to calculate the salary.
const calculateSalary = person => {
let salary = 0;
if (person.type === EMPLOYEE) {
const bonusEveryN = 10;
const bonusAmount = 30;
const totalBonus = Math.floor(person.sales / bonusEveryN) * bonusAmount;
salary = person.sales * person.amountPerSoldItem + totalBonus;
}
if (person.type === FREELANCE) {
salary = person.workedHours.reduce((a, b) => person.rate * a + b, 0);
}
return salary;
}
const generateSalaryReport = id => {
const person = findPayablePerson(id);
if (!person) {
return;
}
const salary = calculateSalary(person);
...
}
Pretty neat right? we can see generateSalaryReport
is shrinking more and more.
Let's create a method for generating rating.
const generateRating = person => {
let rating = 1;
if(person.sales <= 1) {
return 1;
}
const calculateRating = Math.floor(person.sales / 5)
rating = calculateRating > 5 ? 5 : calculateRating;
return rating;
};
const generateSalaryReport = id => {
const person = findPayablePerson(id);
if (!person) {
return;
}
const salary = calculateSalary(person);
const rating = generateRating(person);
...
}
Let's move on and create a function to print the report.
const printReport = (report, rating) => {
const symbol = "⭐️";
const message = `
👍👍👍👍👍👍👍👍👍
name: ${report.name}
amountToPay: ${report.amountToPay}
rating: ${symbol.repeat(rating)},
date: ${report.date.getDate()}-${report.date.getMonth() +
1}-${report.date.getFullYear()}
👍👍👍👍👍👍👍👍👍
`;
console.log(message);
}
We will wait with adding in this method, the reason for that is printReport
does not belong to the function generateSalaryReport
, they are two separate things.
Next, we will create a method for saving the report.
const saveReport = (report) => {
const parsedData = JSON.parse(localStorage.getItem("reports")) || {};
const userReport = parsedData[report.id] || [];
userReport.push(report);
parsedData[report.id] = userReport;
localStorage.setItem("reports", JSON.stringify(parsedData));
}
Same as with printReport
we will not call this function inside generateSalaryReport
.
Inside generateSalaryReport
remove const person = findPayablePerson(id);
and rename id
parameter to person
. Remove the if statement about person. The last step is to create a starting point for our little program. Create a function called init and add the following code.
const init = id => {
const person = findPayablePerson(id);
const report = generateSalaryReport(person);
if (report) {
saveReport(report);
printReport(report);
}
};
Replace generateSalaryReport(1)
with init(1)
the final code should look like this.
const EMPLOYEE = "employee";
const FREELANCE = "freelance";
const employees = [...];
const freelancers = [...];
const findPayablePerson = id => {
let person = employees.find(employee => employee.id === id && employee.active);
if (!person) {
const minimumFreelanceHours = 10;
const freelancer = freelancers.find(freelancer => freenlancer.id === id);
const workedHours = freelancer ? freelancer.workedHours : [];
const workedMoreThenMinHours = workedHours.reduce((a, b) => a + b, 0) >= minimumFreelanceHours;
if (workedMoreThenMinHours) {
person = freelancer;
}
}
return person;
};
const calculateSalary = person => {
let salary = 0;
if (person.type === EMPLOYEE) {
const bonusEveryN = 10;
const bonusAmount = 30;
const totalBonus = Math.floor(person.sales / bonusEveryN) * bonusAmount;
salary = person.sales * person.amountPerSoldItem + totalBonus;
}
if (person.type === FREELANCE) {
salary = person.workedHours.reduce((a, b) => person.rate * a + b, 0);
}
return salary;
};
const generateRating = person => {
let rating = 1;
if(person.sales <= 1) {
return 1;
}
const calculateRating = Math.floor(person.sales / 5)
rating = calculateRating > 5 ? 5 : calculateRating;
return rating;
};
const printReport = (report, rating) => {
const symbol = "⭐️";
const message = `
👍👍👍👍👍👍👍👍👍
name: ${report.name}
amountToPay: ${report.amountToPay}
rating: ${symbol.repeat(rating)},
date: ${report.date.getDate()}-${report.date.getMonth() +
1}-${report.date.getFullYear()}
👍👍👍👍👍👍👍👍👍
`;
console.log(message);
};
const saveReport = report => {
const parsedData = JSON.parse(localStorage.getItem("reports")) || {};
const userReport = parsedData[report.id] || [];
userReport.push(report);
parsedData[report.id] = userReport;
localStorage.setItem("reports", JSON.stringify(parsedData));
};
const generateSalaryReport = person => {
const salary = calculateSalary(person);
const rating = generateRating(person);
const report = {
name: person.name,
id: person.id,
amountToPay: salary,
rating,
date: new Date()
};
return report;
};
const init = id => {
const person = findPayablePerson(id);
if (!person) {
return;
}
const report = generateSalaryReport(person);
if (!report) {
return;
}
saveReport(report);
printReport(report);
return report;
};
init(1);
Conclusion
Look how much more organized and maintainable the code is. Next time your boss says can you do this 5 min fix it might actually be a 5 min fix thanks to smaller functions.