DEV Community

loading...

Leveling up a function 🍄

potouridisio profile image Ioannis Potouridis ・4 min read

Hello friends,

After almost a year of absence, I decided to start sharing again.

This is the story of a function that returned the first active and the first inactive user from a set of users e.g.:

const users = [
  {
    firstName: "Amy",
    lastName: "Effertz",
    id: "5778c2ac-b82f-45e6-8aa3-0b6d83e9a6bb",
    isActive: false,
  },
  {
    firstName: "Chaim",
    lastName: "Halvorson",
    id: "248a9de0-d8e8-4f8e-ac64-311185b47168",
    isActive: true,
  },
  {
    firstName: "Elyssa",
    lastName: "Konopelski",
    id: "f0917603-06fb-45d8-befc-e716319122b3",
    isActive: true,
  },
  {
    firstName: "Kendall",
    lastName: "Glover",
    id: "aae249e6-e36b-4889-a982-6babe17dd696",
    isActive: false,
  },
  {
    firstName: "Sigmund",
    lastName: "Eichmann",
    id: "f3505b2a-7e85-4994-b3c6-3b62a4b7f77c",
    isActive: false,
  }
];
Enter fullscreen mode Exit fullscreen mode

The function looked like this.

function getOneActiveAndOneInactiveUser() {
  const active = users.find((user) => user.isActive);
  const inactive = users.find((user) => !user.isActive);

  return [active, inactive].filter((user) => typeof user !== "undefined");
}
Enter fullscreen mode Exit fullscreen mode

It did exactly what it said.

It was also efficient because of how find works.

This is how most of the developers would have written it. 👌

Actually it is what most humans understand.

-I need to find one active and one inactive user.

But as you grow as a developer you start thinking in different terms.

-I need to find the first two opposite occurences in an array.

Notice that the first sentence is specific to users and especially their active status where the second one doesn't care that much of the source and condition which means that you can say it more times than the first.

So I started refactoring the function and the first iteration looked like this.

function getFirstOppositeOccurences() {
  let temp = {};

  for (let i = 0; i < users.length; i++) {
    if (users[i].isActive) {
      if (typeof temp[0] === "undefined") {
        temp[0] = users[i];
      } else {
        continue;
      }
    } else {
      if (typeof temp[1] === "undefined") {
        temp[1] = users[i];
      } else {
        continue;
      }
    }
  }

  return Object.values(temp);
}
Enter fullscreen mode Exit fullscreen mode

Pretty uggly, right?

It has more lines of code than the first one, it is less readable, it uses a for like it's... 1999, it has a variable named temp like we are writting code in some kind of university, it has a lot of "if elses" and it still depends on the users and their isActive property. The only "cool" thing was the use of Object.values to create that array.

Another thing you do as you grow as a developer is finding patterns in code. So I looked my code again and especially my if blocks. They looked similar except from those 0 and 1 indices.

That 0 and 1 depended on that isActive condition. I needed to add my active user first and inactive second. A condition always returns a boolean so I immediately thought of casting that boolean to a number:

+true; // 1
+false; // 0
Enter fullscreen mode Exit fullscreen mode

But I needed the truthy to be in 0 index so I stuck in the logical NOT operator.

+!true; // 0
+!false; // 1
Enter fullscreen mode Exit fullscreen mode

The "outter" if was gone and the second iteration of the function looked like this.

function getFirstOppositeOccurences2() {
  let temp = {};

  for (let i = 0; i < users.length; i++) {
    const index = +!users[i].isActive;

    if (typeof temp[index] === "undefined") {
      temp[index] = users[i];
    } else {
      continue;
    }
  }

  return Object.values(temp);
}
Enter fullscreen mode Exit fullscreen mode

The second if was simply checking to add a user entry only if it was not already merged to the temp object.

I used the word merged here instead of added as a hint. You can totally get rid of that if by turning your if to something like this.

for (let i = 0; i < users.length; i++) {
    const index = +!users[i].isActive;

    temp = { ...temp, ...(!temp[index] && { [index]: users[i] }) };
}
Enter fullscreen mode Exit fullscreen mode

It is a one liner but is it readable?

I am not a huge fan of ternaries and there's a better way of clearing out "if else" conditions. You will still keep the if but it will work as a guard clause.

So in the third iteration, the function looked like this.

function getFirstOppositeOccurences() {
  let temp = {};

  for (let i = 0; i < users.length; i++) {
    const index = +!users[i].isActive;

    if (typeof temp[index] !== "undefined") continue;

    temp[index] = users[i];
  }

  return Object.values(temp);
}
Enter fullscreen mode Exit fullscreen mode

We're getting somewhere with the readability and that nested 💩, but our function is still user dependent.

In order to make the function independent, instead of using the users array as a closure, I tried passing it as an argument.

function getFirstOppositeOccurences(array) {
  let temp = {};

  for (let i = 0; i < array.length; i++) {
    const index = +!array[i].isActive;

    if (typeof temp[index] !== "undefined") continue;

    temp[index] = array[i];
  }

  return Object.values(temp);
}
Enter fullscreen mode Exit fullscreen mode

Do you see what is the problem here?

+!array[i].isActive;
Enter fullscreen mode Exit fullscreen mode

That array may include anything now and the items probably won't have an isActive property.

So I needed to isolate that condition from the function. But how am I going to do that? If you notice that line there's an item that I need to access.

array[i]
Enter fullscreen mode Exit fullscreen mode

If you go to that first function you'll notice that this is already implemented. Everyone is using array functions like find, map etc.

users.find((user) => user.isActive)
Enter fullscreen mode Exit fullscreen mode

They all accept a callback function that provides us with each item per iteration, so this is answer for isolating the condition from my function.

My next iteration looked like this.

function getFirstOppositeOccurences(array, callbackFn) {
  let temp = {};

  for (let i = 0; i < array.length; i++) {
    const index = +!callbackFn(array[i], i, array);

    if (typeof temp[index] !== "undefined") continue;

    temp[index] = array[i];
  }

  return Object.values(temp);
}
Enter fullscreen mode Exit fullscreen mode

The only thing changed is of course passing the callback as an argument and this line:

+!callbackFn(array[i], i, array)
Enter fullscreen mode Exit fullscreen mode

I call it with the current item, the index and the original array. This gives a little bit of flexibility for the conditions you want to pass e.g.:

// get two users, one active and one inactive
getFirstOppositeOccurences(users, (user) => user.isActive);
// get two users, the third one (index === 2) and one that is not the third one
getFirstOppositeOccurences(users, (user, index) => index === 2);
Enter fullscreen mode Exit fullscreen mode

Finally, I had to add some early escapes in the top of my function in order to avoid some bad paths.

if (array.length === 0) return [];
if (typeof array === "undefined") return [];
if (typeof callbackFn === "undefined") return [];
Enter fullscreen mode Exit fullscreen mode

What do you think?

Do you think it leveled up? 🍄

Do you have any suggestions to improve this further?

I prefer to keep functions like the first but I enjoy practicing.

I hope you enjoyed reading this article too, and more happy if you learned something new or remembered something old.

Discussion (0)

Forem Open with the Forem app