First example

See the code below, and imagine yourself reviewing a PR that contains it:

function async formatHoursOfOperation(venue) {
    let openingHours;
    if (venue.openingHoursId) {
      openingHours = await getOpeningHours(venue.openingHoursId);
    } else {
      const park = await getParkInfo();
      openingHours = await getParkOpeningHours(park.openingHoursId);
    }
    /// ...function continues
}

Simple isn't it? But here is what can be improved:

• 🥸 In my experience it has always been advantageous to avoid mutations (https://blog.sapegin.me/all/avoid-mutation/): they make the code complex and unpredictable.

• 🥲 The function is doing more than it's supposed to, and we're using let as a workaround.

Let's refactor it:

function async getVenueOpeningHours(venue) {
    if (venue.openingHoursId) return getOpeningHours(venue.openingHoursId);

    const park = await getParkInfo();
    return getParkOpeningHours(park.openingHoursId);
}

function formatHoursOfOperation(venue) {
    const openingHours = await getVenueOpeningHours(venue.openingHoursId)
    /// ...function continues
}

• ✅ First, the method was refactored, using the "extract method" technique (https://refactoring.guru/pt-br/extract-method). Many times when we have this composition of let, if and else, it means that our method is doing more than it should. In this case, it was fetching and formatting the hours, which was separated into two methods, which can be easily unit tested.

• ✅ Now in a separate method, we can return the values depending on the conditions, instead of reassigning the variable. With that, it was possible to get rid of the else, which is also very controversial (I haven't used this keyword for a long time either): https://williamdurand.fr/2013/06/03/object-calisthenics/#2-dont- use-the-else-keyword. It is also important to highlight the use of early return, making the code much more readable.

Second example

Recently I was building a minesweeper engine, and by searching for "generating no duplicated random numbers in javascript", I saw something like this:

const MAX = 100;

function getUniqueRandomNumbers(length) {
    const numbers = [];
    for (let i = 0; i < length; i++) {
        const randonNumber = Math.floor(Math.random() * MAX);
        if (numbers.includes(randonNumber)) {
            i--;
            continue;
        } else {
            numbers.push(randonNumber);
        }
    }

    return numbers;
}

Messy right? Let's talk about what's wrong with this code:

• 🤮 i--, but inside an incremental for loop...It hurts my eyes.

• 🥲 What's inside of numbers? I know that numbers is a const, but it remains difficult to track the variable value. It happens that const just prevents variable reassignment, not mutations ⚠️.

So let's fix it:

const MAX = 100;

function getRandomNumber(numbers) {
    const randonNumber = Math.floor(Math.random() * MAX);
    if (numbers.includes(randonNumber)) return getRandomNumber(numbers);

    return randonNumber;
}

function getUniqueRandomNumbers(length) {
    return [...Array(length).keys()]
        .reduce((numbers) => [...numbers, getRandomNumber(numbers)], [])
}

• ✅ We got rid of numbers.push (mutation) using reduce and using the accumulator parameter.

• Another thing I learned from functional programming is to prefer expressions rather than statements (https://www.joshwcomeau.com/javascript/statements-vs-expressions/), that's why [...Array(length).keys()] was the choice here (it creates an array from 0 to length). In general, array.map, array.reduce..., is more readable than for, forof....

Conclusion

It's the basic concepts that make the code readable and scalable. Time teaches us better ways to program and makes refactoring essential.