Recently I wrote about a particular code smell: Side Effects. Several people called me out for not providing a suggested fix to the identified problem. If you didn't read the original article, you can find it here. You will want to be familiar with what I wrote to continue on here with the solution.
In that article, we saw how we have a side effect in our loadUser function. Let's go through options for addressing this code smell.
Our first task is to look at the calling code. Ultimately this is where the code smell is coming from because it's in the calling code that we SHOULD be handling the responsibility of emptying the cart, but instead, it got put into the loadUser function.
Let's look at some example calling code.
Here our handleUserLogin method first creates a new empty user, sets it into the session object's user variable, and finally asks the session to load the User. It's inside this call that the cart is emptied. This is the crux of our issue. That operation of loading the user has some specific tasks. But it's ALSO emptying out the cart. That is perfectly appropriate in the situation of handling the user login. But if we ever needed to load user data and didn't want the side effect of emptying out the cart to happen, (maybe while refreshing data from the server) we don't have that option. A function should only do one thing. The loadUser function does two.
So let's fix that by extracting the cart.empty call out of the loadUser function. The simplest answer is just to move it up one level into our calling code.
And our loadUser is now clean of side effects:
Now, when someone else comes along and calls loadUser, they will get the result they want, and no unnecessary side effects.