Leveling up a function 🍄
Tags: javascript
Published: 2021-06-10 06:22:24

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.:

```js
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,
  }
];
```

The function looked like this.

```js
function getOneActiveAndOneInactiveUser() {
  const active = users.find((user) => user.isActive);
  const inactive = users.find((user) => !user.isActive);
  
  return [active, inactive].filter((user) => typeof user !== "undefined");
}
```

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.

```js
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);
}
```

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:

```js
+true; // 1
+false; // 0
```

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

```js
+!true; // 0
+!false; // 1
```

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

```js
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);
}
```

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.

```js
for (let i = 0; i < users.length; i++) {
    const index = +!users[i].isActive;
   
    temp = { ...temp, ...(!temp[index] && { [index]: users[i] }) };
}
``` 

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.

```js
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);
}
```

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.
 
```js
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);
}
```

Do you see what is the problem here? 

```js
+!array[i].isActive;
```

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.

```js
array[i]
```

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

```js
users.find((user) => user.isActive)
```

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.

```js
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);
}
```

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

```js
+!callbackFn(array[i], i, array)
```

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.:

```js
// 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);
```

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

```js
if (array.length === 0) return [];
if (typeof array === "undefined") return [];
if (typeof callbackFn === "undefined") return [];
```

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.