As an (aspiring) Junior React developer it’s hard to understand if you’re code is any good and how you can improve it. It would be great to get a review from an experienced dev. But sharing your code takes guts. And the reviews you get in online communities are often shallow.
Recently, a developer was brave enough and requested a code review. Kudos. The feature was interesting. The code wasn’t bad. But there were a few mistakes that are very common among Junior React developers.
So here’s your opportunity to learn from these mistakes. On this page, you can find a code review and a step-by-step refactoring journey. By
- using a better data structure
- removing unnecessary state and
- a few other refactorings and cleanup
we increase the performance of the code, make it easier to read and maintain, and shorten the component from initially 177 to 102 lines (not that this is a good metric).
If you’re rather a video person you can watch the complete review and refactoring session here.
Note: the code on this page is slightly improved compared to the code in the video using a Set instead of a Map.
Table Of Contents
The Component To Review
The Functionality
The component is a table with a few somewhat complex features. Here’s a quick video:
The table renders a list of issues. An issue can have a status “open” or “resolved” that is reflected in the last column. Issues with the status “resolved” can’t be selected.
The checkbox at the top is partially checked when some but not all rows are selected. Clicking on that checkbox allows the user to select or deselect all issues with the status “open”.
The Original Code
The original code isn’t bad. It works and does its job. For the most part, it’s quite readable. Especially on a high level. And it’s similar to what many Junior React devs would produce. At the same time, there are a few problems that are again common among Junior devs. And we’ll discuss them in a bit.
But first things first. Here’s the component’s code. You can also find a functioning version on CodeSandbox.
import { useState } from "react";
import classes from "./Table.module.css";
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
})
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
const handleOnChange = (position) => {
const updatedCheckedState = checkedState.map((element, index) => {
if (position === index) {
return {
...element,
checked: !element.checked,
backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
};
}
return element;
});
setCheckedState(updatedCheckedState);
const totalSelected = updatedCheckedState
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState) {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(false);
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
setSelectDeselectAllIsChecked(false);
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(true);
}
};
const handleSelectDeselectAll = (event) => {
let { checked } = event.target;
const allTrueArray = [];
issues.forEach((element) => {
if (element.status === "open") {
allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
} else {
allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
}
});
const allFalseArray = new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
});
checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);
const totalSelected = (checked ? allTrueArray : allFalseArray)
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState && issues[index].status === "open") {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
setSelectDeselectAllIsChecked((prevState) => !prevState);
};
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
className={classes.checkbox}
type={"checkbox"}
id={"custom-checkbox-selectDeselectAll"}
name={"custom-checkbox-selectDeselectAll"}
value={"custom-checkbox-selectDeselectAll"}
checked={selectDeselectAllIsChecked}
onChange={handleSelectDeselectAll}
/>
</th>
<th className={classes.numChecked}>
{numCheckboxesSelected
? `Selected ${numCheckboxesSelected}`
: "None selected"}
</th>
</tr>
<tr>
<th />
<th>Name</th>
<th>Message</th>
<th>Status</th>
</tr>
</thead>
<tbody>
{issues.map(({ name, message, status }, index) => {
let issueIsOpen = status === "open";
let onClick = issueIsOpen ? () => handleOnChange(index) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
className={stylesTr}
style={checkedState[index]}
key={index}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
checked={checkedState[index].checked}
onChange={() => handleOnChange(index)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
<td>{name}</td>
<td>{message}</td>
<td>
{issueIsOpen ? (
<span className={classes.greenCircle} />
) : (
<span className={classes.redCircle} />
)}
</td>
</tr>
);
})}
</tbody>
</table>
);
}
export default Table;
The issues array that is rendered in the table looks like this.
[
{
"id": "c9613c41-32f0-435e-aef2-b17ce758431b",
"name": "TypeError",
"message": "Cannot read properties of undefined (reading 'length')",
"status": "open",
"numEvents": 105,
"numUsers": 56,
"value": 1
},
{
"id": "1f62d084-cc32-4c7b-943d-417c5dac896e",
"name": "TypeError",
"message": "U is not a function",
"status": "resolved",
"numEvents": 45,
"numUsers": 34,
"value": 1
},
...
]
The component is more on the complex side with 177 lines of code (LOC). But as mentioned, on the high level it’s quite readable. This is what you see when the functions are collapsed.
function Table({ issues }) {
// decides which checkboxes is selected
const [checkedState, setCheckedState] = useState(...);
// decides if the top checkbox is checked or not
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
// represents the number of selected checkboxes
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
// handles the click on a row / checkbox
const handleOnChange = (position) => { ... };
// responsible for setting the partially checked state of the top checkbox
const handleIndeterminateCheckbox = (total) => { ... };
// handles the click on the top checkbox
const handleSelectDeselectAll = (event) => { ... };
return (
<table>
...
</table>
);
}
Not too bad. But there’s room for improvement.
Problems With The Original Code
Problem 1: Sub-Optimal Data Structure
Let’s have a look at the top of the component.
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
...
- The
checkedState
variable is an array. - It’s initialized to have the same length as the issues array.
- Each entry is an object that represents the checked state of an issue.
The first problem is that an array of objects is always hard to mutate. We have to access the correct object via its index inside the array. So we need to find and pass indices around.
The second problem is that this approach doesn’t scale very well. For example, if we had 10k issues we’d have a 10k long checkedState
array filled with mostly false
boolean values.
The alternative: We could use a different data structure like an object or a Map that maps an issue id to its checked value.
// alternative data structure (example when two checkboxes are checked)
{
"issue-id-1": true,
"issue-id-4": true,
}
Even better, JavaScript has a native Set that is similar to an array but can only hold unique values and is optimized for accessing them in a performant way.
Problem 2: Not Deriving Variables From State
Again at the top of the component, we can see another very common problem in the code of Junior developers: Unnecessary state variables that could be derived from props or another state.
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...
In this code, we have three unnecessary pieces of state:
-
backgroundColor
is a style that can be derived from the value ofchecked
(it’s either#eee
if checked is true otherwise#fff
) -
selectDeselectAllIsChecked
is used to set thechecked
prop for the checkbox at the top (can be derived fromcheckedState
andissues
) -
numCheckboxesSelected
is the number next to that checkbox (can be derived fromcheckedState
)
Problem 3: Directly Accessing The DOM
A bit further down the code, we can find the function that handles the indeterminate
state of the top checkbox:
function Table({ issues }) {
...
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(false);
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
setSelectDeselectAllIsChecked(false);
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(true);
}
};
...
We can see that the top checkbox is accessed via document.getElementById(...)
. This is relatively unusual in React apps and mostly not necessary.
Note that setting indeterminateCheckbox.indeterminate
directly on the DOM element is necessary. We can’t set indeterminate
as a prop on the checkbox.
Problem 4: Variable Naming
The author of this code put some effort into selecting good names for the variables and functions. Kudos to that.
Unfortunately, some names are a bit confusing. We can find some examples at the top of the component:
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...
For example, checkedState
is redundant and misleading. I’d expect a boolean here but this is an array of objects. A better name might be checkedItems
.
selectDeselectAllIsChecked
kind of is understandable but a bit hard to read. It tells us whether all checkboxes are checked or not but I had to read it like 5 times. A better name could be isEveryItemChecked
.
The Refactoring
Now that we’ve analyzed some of the problems with the original code let’s start to refactor it step by step.
Step 1: Refactor Data Structure From Array To Set
The most impactful step is to use a Set instead of the array as the checkedState
. We could have used an object as well but the Set has slight advantages like direct access to its size
which we’ll use quite a bit. Compared to a Map, the Set is a slightly better fit here as we only need to know the ids of all selected items instead of a key-value pair.
As we’re already touching this code we also remove the backgroundColor
from the state and rename the state variable.
//before
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
// after
const [checkedById, setCheckedById] = useState(new Set());
As a reminder, the shape of the checkedById
state (represented as an array) looks now like this:
// shape of checkedById (when two checkboxes are checked)
["issue-id-1", "issue-id-4"]
This change isn’t trivial. It has implications for the rest of our code. We’ll see one by one how each part of the code is affected.
Let’s start with the handler function that’s connected to the checkboxes in the table.
Here’s the before and after of the code:
// before
const handleOnChange = (position) => {
const updatedCheckedState = checkedState.map((element, index) => {
if (position === index) {
return {
...element,
checked: !element.checked,
backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
};
}
return element;
});
setCheckedState(updatedCheckedState);
const totalSelected = updatedCheckedState
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState) {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};
// after
const handleOnChange = (id) => {
const updatedCheckedById = new Set(checkedById);
if (updatedCheckedById.has(id)) {
updatedCheckedById.delete(id);
} else {
updatedCheckedById.add(id);
}
setCheckedById(updatedCheckedById);
const totalSelected = updatedCheckedById.size;
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};
We got rid of quite some clutter. Some quick notes:
- The line
return sum + issues[index].value
in the original code is interesting. It uses a value in the data to increment the sum and calculate the number of selected elements. This seems over-complex. - In the refactored code we can now simply use
updatedCheckedById.size
to get the number of selected elements. - Since we don’t want any
false
values in our state we have to delete the corresponding value when a checkbox is deselected viaupdatedCheckedById.delete(id)
.
Let’s continue with the handler function that is connected to the top checkbox.
Here’s the before and after.
// before
const handleSelectDeselectAll = (event) => {
let { checked } = event.target;
const allTrueArray = [];
issues.forEach((element) => {
if (element.status === "open") {
allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
} else {
allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
}
});
const allFalseArray = new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
});
checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);
const totalSelected = (checked ? allTrueArray : allFalseArray)
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState && issues[index].status === "open") {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
setSelectDeselectAllIsChecked((prevState) => !prevState);
};
// after
const handleSelectDeselectAll = (event) => {
if (event.target.checked) {
const openIssues = issues.filter(({ status }) => status === "open");
const allChecked = new Set(openIssues.map(({ id }) => id));
setCheckedById(allChecked);
setNumCheckboxesSelected(allChecked.size);
} else {
setCheckedById(new Set());
setNumCheckboxesSelected(0);
}
setSelectDeselectAllIsChecked((prevState) => !prevState);
};
Again a few quick notes:
- The original implementation is a bit cumbersome with the
allTrueArray
andallFalseArray
. We could refactor this as well but since we use an object now we can simply remove this code. - Because we use a Set in the new implementation creating the new
allChecked
state is very straightforward. We filter all open issues and then construct a new Set from an array of ids.
The last piece of code that we have to adjust is the JSX returned from the component. In particular the code inside the <tbody>
element.
// before
<tbody>
{issues.map(({ name, message, status }, index) => {
let issueIsOpen = status === "open";
// we have to use the id of the issue instead of the index here
let onClick = issueIsOpen ? () => handleOnChange(index) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
className={stylesTr}
// the backgroundColor isn't part of checkedState anymore
// so we have to adjust this
style={checkedState[index]}
key={index}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
// we have to use the id instead of the index here
checked={checkedState[index].checked}
onChange={() => handleOnChange(index)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
...
</tr>
);
})}
</tbody>
// after
<tbody>
{issues.map(({ id, name, message, status }, index) => {
let issueIsOpen = status === "open";
let onClick = issueIsOpen ? () => handleOnChange(id) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
key={id}
className={stylesTr}
style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
checked={checkedById.has(id)}
onChange={() => handleOnChange(id)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
...
</tr>
);
})}
</tbody>
Again a few quick notes:
- We have to change most of the occurrences of
index
in this code. - Instead of writing
key={index}
on the<tr>
element we use theid
as key now. This doesn’t change anything in the current version as the order of issues doesn’t change. But it’s best practice and it’s likely that some kind of sorting will be introduced to our table in the future. - Previously the background color of the row was set via this line:
style={checkedState[index]}
. This takes a moment to understand. For any given row this translates to e.g.style={{ checked: true, backgroundColor: "#fff" }}
. - The new version is much more explicit in this regard:
style={{ backgroundColor: checkedById[id] ? "#eee" : "#fff" }}
You can find all code changes here on GitHub and a function version of this step here on CodeSandbox (note that you need to change App.jsx
to see this version of the component rendered in the browser).
Step 2: Using Derived State
The second problem we discussed above is the unnecessary state variables selectDeselectAllIsChecked
and numCheckboxesSelected
. As mentioned, these can easily be derived from the checkedById
state and the issues
data array.
// before
function Table({ issues }) {
const [checkedById, setCheckedById] = useState(new Set());
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...
// after
function Table({ issues }) {
const [checkedById, setCheckedById] = useState(new Set());
const openIssues = issues.filter(({ status }) => status === "open");
const numOpenIssues = openIssues.length;
const numCheckedIssues = checkedById.size;
...
We don’t necessarily need the openIssues
variable here. But this way we can reuse it inside the refactored handleSelectDeselectAll
function (which is not shown here).
On top of these changes, we also have to remove the calls to the state setters from the rest of the code. These changes have two advantages:
- We get rid of a few lines of code.
- More importantly, we don’t need to manage the additional state anymore. This is great because we can easily forget to update the state and thus introduce bugs.
As the last step, we can memoize the calculation of numOpenIssues
. This is not required for the current version but might be necessary once we work with larger data sets.
const openIssues = useMemo(
() => issues.filter(({ status }) => status === "open"),
[issues]
);
You can see the complete list of code changes here on GitHub and a functioning version of this step here on CodeSandbox.
Step 3: Remove document.getElementById()
The final important step in our refactoring journey is to access the top checkbox the React way instead of using document.getElementById()
.
What’s the React way? We use a ref
.
// before
function Table({ issues }) {
...
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
}
};
...
// after
function Table({ issues }) {
...
const topCheckbox = useRef();
const handleIndeterminateCheckbox = (checkedById) => {
const numSelected = checkedById.size;
if (numSelected === 0) {
topCheckbox.current.indeterminate = false;
} else if (numSelected === numOpenIssues) {
topCheckbox.current.indeterminate = false;
} else {
topCheckbox.current.indeterminate = true;
}
};
...
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
ref={topCheckbox}
...
A few quick notes:
- The most important thing is that we replaced
document.getElementById()
byuseRef()
. This also allows us to remove theid
from the checkbox. - The line
issues.forEach((element) => {
in the original code is responsible for calculating the number of open issues. This is a bit cumbersome approach and could have easily been replaced withissues.filter(...).length
. But we already have defined thenumOpenIssues
variable at the top of our component. So no need for this code anymore. - We used
if ... else if ... else
instead of threeif
statements. We also switched the conditions around to have the onlytrue
value at the bottom. This makes it easier to process for our brains in my opinion.
Actually, we don’t even need the if ... else if ... else
statement. We can simply replace it with a single line of code:
topCheckbox.current.indeterminate =
numSelected > 0 && numSelected < numOpenIssues;
Using this line it also doesn’t make much sense to keep the handleIndeterminateCheckbox
function anymore.
You can find all the code changes here on GitHub and a functioning version of this step here on CodeSandbox.
Final Code
After a bit of additional cleanup (here are the changes) we’re able to reduce the code from initially 177 LOC to 102 LOC.
import { useMemo, useState, useRef } from "react";
import classes from "./Table.module.css";
function Table({ issues }) {
const topCheckbox = useRef();
const [checkedById, setCheckedById] = useState(new Set());
const openIssues = useMemo(
() => issues.filter(({ status }) => status === "open"),
[issues]
);
const numOpenIssues = openIssues.length;
const numCheckedIssues = checkedById.size;
const handleOnChange = (id) => {
const updatedCheckedById = new Set(checkedById);
if (updatedCheckedById.has(id)) {
updatedCheckedById.delete(id);
} else {
updatedCheckedById.add(id);
}
setCheckedById(updatedCheckedById);
const updatedNumChecked = updatedCheckedById.size;
topCheckbox.current.indeterminate =
updatedNumChecked > 0 && updatedNumChecked < numOpenIssues;
};
const handleSelectDeselectAll = (event) => {
if (event.target.checked) {
const allChecked = new Set(openIssues.map(({ id }) => id));
setCheckedById(allChecked);
} else {
setCheckedById(new Set());
}
};
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
type="checkbox"
ref={topCheckbox}
className={classes.checkbox}
checked={numOpenIssues === numCheckedIssues}
onChange={handleSelectDeselectAll}
/>
</th>
<th className={classes.numChecked}>
{numCheckedIssues
? `Selected ${numCheckedIssues}`
: "None selected"}
</th>
</tr>
<tr>
<th />
<th>Name</th>
<th>Message</th>
<th>Status</th>
</tr>
</thead>
<tbody>
{issues.map(({ id, name, message, status }) => {
const isIssueOpen = status === "open";
return (
<tr
key={id}
className={
isIssueOpen ? classes.openIssue : classes.resolvedIssue
}
style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
>
<td>
<input
type="checkbox"
className={classes.checkbox}
checked={checkedById.has(id)}
disabled={!isIssueOpen}
onChange={() => handleOnChange(id)}
/>
</td>
<td>{name}</td>
<td>{message}</td>
<td>
<span
className={
isIssueOpen ? classes.greenCircle : classes.redCircle
}
/>
</td>
</tr>
);
})}
</tbody>
</table>
);
}
export default Table;
You can find all code changes here on GitHub and a version of the final component here on CodeSandbox.
From my perspective, this is not only more concise but also much more readable.
Takeaways
- Watch out for unnecessary state that can be derived from other state or data.
- Using an object, a map, or a set can often be easier and more efficient than an array.
- The React way of accessing DOM elements is
useRef()
and notdocument.getElementById()
.
Top comments (11)
Thanks for React information. I use Perl in my web development. I can combinate with Perl and React.
Nice video. Great to see your thought process
Thank you 🖐
Really interesting. Thanks for this informative article!
Thanks, I'm glad you liked it
Thanks for this. I learnt a lot following your explanations. But I have a point to make. The reason he had the onClick function on the row is to be able to click the row to check and uncheck the box, removing it won't give that opportunity anymore. You can now only click the box to check and uncheck after removing it.
Yeah that's true. I mention in the video that removing the click handler from the
tr
is a change in behavior. But in the original code there's a click handler on thetr
and on theinput
which doesn't make sense imo. And if we wanted to make thetr
clickable we would have to adjust more things like setting a tab index to make it accessible. So I just opted for using the inputThis is really cool to see how in depth you went with this. As a junior react developer myself I’ve never seen any utilize the document object within a react application. Is this as common as it seems?
Yeah, it's not very common. There are scenarios where it's required. For example, when you need to attach a scroll or click listener to the HTML body. Or maybe when you work with third-party tools where you can't use a ref. But in most cases there's a more React-like alternative.
Great read! Thanks for sharing this with us Johannes! 👌
Thanks for sharing this refactoring code it was really interesting. Mainly in the use of Set.