DEV Community

Refactoring: My 6 favorite patterns

Bryce on March 04, 2020

Refactoring code has become one of my favorite things to do as a developer. It can have a major impact on code cleanliness, readability, and mainta...
Collapse
 
frankfont profile image
Frank Font

So cool that more than one of your favorite refactoring strategies specifically clarify intent of the code! This is so fundamental to good code. Having code communicate intent is so much more important than having code that is a few lines shorter.

Collapse
 
lexlohr profile image
Alex Lohr

Exactly. Code is meant to be read by developers. Computers don't need to understand the code to execute it, they only have to follow instructions.

Collapse
 
marianban profile image
Marian Ban • Edited

Hi thanks for the article.
Regarding 1. Replace Conditional with Polymorphism I would rather think carefully before introducing PizzaUser and IceCreamUser for this concrete example. I know that this is probably not a real world example but I would rather point out few things before people start to go wild with inheritance πŸ˜ƒ

So lets consider a new requirement: As an user I would like to receive emails for my favorite music and sms notifications for my favorite tv shows. So we have user.favorites.music, user.favorites.food, user.favorites.tvshow.

With our current implementation we are running into an obvious issue because we need something what is capable of sending pizza emails, favorite music emails and sms notifications. If we stick to our inheritance model then we will end up with many different classes like: (PizzaNirvanaGOTUser, IceCreamDoorsNarcosUser...). But we can do better with the command pattern πŸš€. Example:

class User {
  constructor(user) {
    this._user = user;
  }
  notify(notificationSender) {
    notificationSender.execute(this);
  }
}

class CompositeNotificationSender {
  execute(user) {
    this._notificationSenders.forEach(x => x.execute(user));
  }
}

// concrete notification senders
class FavoriteFoodMailSender { ... }
class FavoriteMusicMailSender { ... }
class FavoriteTvShowSmsSender { 
  execute(user) {
    sendTvShowSms(user);
  }
}

// actual usage
const user = getUser();
const notificationSender = new CompositeNotificationSender(
  new FavoriteFoodMailSender(),
  new FavoriteMusicMailSender(),
  new FavoriteTvShowSmsSender()
); 
user.notify(notificationSender);

Happy coding!

Collapse
 
richardcochrane profile image
Richard Cochrane

That's a good point Marian and the crux behind the composition vs inheritance thinking but I think there's a place for both. My main decider (and it's beside the point to the one the author is making) is asking if that method really belongs on the class. So classes inheriting from Animal (Dolphin, Cat, Elephant) would all "make sound" (their own sound) so that would be a great case for inheritance but if the functionality is not related, eg. "send mail" (sorry, it's contrived but let's say in a talking animals universe, like Zootropolis, where animals can send letters either by running, flying or the postal service), the sending of emails is not intrinsic to being an animal and the same functionality could be rightfully applied to companies, people and other types of class that don't inherit from Animal, then composition is a better way to go. In my opinion, of course :-)

Collapse
 
gkatsanos profile image
George Katsanos • Edited

reading both the original recommendation as well as the alternative, I really have a strong feeling to stick with the conditions. Also, why not move simply move/abstract the condition inside the sendmail function? The information we need is already inside the user object. I find creating the extra classes for each type of favorite food overengineered TBH. And adding one new favorite food type, means we need a whole new class for it?

sendEmail(user) { return `<div>Hey ${user.firstName}, we saw you like ${user.favorites.food}</div>` };
Collapse
 
oscherler profile image
Olivier β€œΓ–lbaum” Scherler

In example number five you use hasUserPaidThisWeek before you assign it. Does it still work because of hoisting or something like that? Isn’t it a bit counterintuitive for people who read the code later?

Collapse
 
brycedooley profile image
Bryce • Edited

Hey Olivier, good catch! Yes that would be an issue - it would need to be a function declaration which would make it work via hoisting - I'll update the example (I'll admit I didn't actually run any of this code - it was more to illustrate the concepts - so there may be other runtime errors 😬).

But, I do think that declaring a function right after it's used is a good pattern. It follows the "Stepdown Rule" described in the book Clean Code. The idea is code should read like a top-to-bottom narrative, which I do kind of like. More examples here: dzone.com/articles/the-stepdown-rule

Generally I think the most important thing is that related functions live close together in the code...whether they are before or after is less of a concern.

Collapse
 
oscherler profile image
Olivier β€œΓ–lbaum” Scherler

I agree with you: the top-to-bottom narrative works better. Declaring such functions before they are used makes you wonder what’s happening, before you see the usage, when you read the code.

And I think the way the code is set now is good. My brain is not bothered by the function being declared after it’s used, even if it’s inside the current scope, because it’s a function declaration.

P.S. no worries about not having run the examples. You’re right, they’re illustrations. I was just curious about that particular case as I found it surprising (then again, programming languages are full of surprises) and I’m not a JS expert.

Collapse
 
kant312 profile image
Quentin Delcourt

Thank you for the article!
I also like to store gigantic conditionals in a meaningful variable πŸ˜›

As for the example number 4, I suppose it's an implementation of the Value Object pattern, for elements that do not have a specific identity?
martinfowler.com/bliki/ValueObject...

Collapse
 
brycedooley profile image
Bryce

Yes that looks very similar!

Collapse
 
ineam profile image
Amine M

very nice patterns, but I think on the number 6 you wanted to put it this way:

const myObject = { toName, punctuation, fromName };

// Before

function sayHello(myObject) {
  return `Hello, ${myObject.toName}${myObject.punctuation} From, ${myObject.fromName}.`
} 

sayHello(customerName, end, myName);

// After

function sayHello({ toName, punctuation, fromName }) {
  return `Hello, ${toName}${punctuation} From, ${fromName}.`
} 

sayHello(myObject);
Collapse
 
fluffynuts profile image
Davyd McColl

A useful extension to (1) is the null pattern: providing an implementation which has the right methods but does nothing, instead of a conditional at the point of doing the required thing (eg a NoEmail user facade in the example above)

Collapse
 
bradtaniguchi profile image
Brad

I love #6 (single object param) hands down one of my favorite coding practices! Works great with TS too :D

Collapse
 
swarupkm profile image
Swarup Kumar Mahapatra
function sayHello({ toName, punctuation, fromName }) {
  return `Hello, ${toName}${punctuation} From, ${fromName}.`
} 

sayHello({ toName, punctuation, fromName });

I would prefer to modify it to below. In this way the signature of method is clear. It also tells what kind of objects are acceptable. It also tells what kind of properties should be bound together as an object. The grouping of properties is domain specific. It is not anonymous.
I am taking clues from Replace Anonymous Function with Expression.

function sayHello(personPair,punctuation) {
  const {toName, fromName} = personPair
  return `Hello, ${toName}${punctuation} From, ${fromName}.`
} 
const personPair = { toName, fromName };
sayHello(personPair, punctuation);

Note: I am not satisfied with personPair as variable name. Feel free to suggest better variable name.

Collapse
 
johannesjo profile image
Johannes Millan

Thank you for the article. Really interesting subject.

However, I would like to suggest to be very very (!!) cautious with the advises 4, 2 and 1, because of KISS and Yagni. Always think twice or better thrice before introducing a new abstraction level to your code.

Always ask yourself: Do we really need this now and what are possible costs of the abstraction?

Collapse
 
hoffmann profile image
Peter Hoffmann

While still refactoring I'd suggest to replace in the second example (example 5)

(user) => {
  if(user.lastPayment >= moment().startOf('week').toDate()) {
    return true;
  }
  return false;
}

with

function (user) {
    return user.lastPayment >= moment().startOf('week').toDate()
}

and don't forget, that arrow functions don't create their own this-scope.

Collapse
 
llcamara profile image
LLCamara

For #3, my usual suggestion for refactoring is to create a function instead of a const variable - in this case user.isSubscribed().

The nice thing I see in having a const variable is when there is a complex calculation used multiple times. Then sure, do the calculation once, store in a const and fire away.

Collapse
 
sebbdk profile image
Sebastian Vargr • Edited

This might be my functional programming bias talking.

But... in example, 4, 2 and 1 you managed to turn a few lines of code into many lines of code.
What is better in these examples?

6 tend be fragile in none-typed languages in my experience.

3, 5 are great tips, they increase verbosity tremendously. :)

Collapse
 
ronielramos profile image
Roniel

About #1, how do you implement "getUser(userData)"?

Collapse
 
brycedooley profile image
Bryce

I'd probably use a switch statement to instantiate the appropriate user type - so something like:

function getUser(userData) {
  const userType = userData.favorites.food;

  switch(userType) {
    case 'pizza':
      return new PizzaUser(userData);
    ... 
  }
}
Collapse
 
chakrihacker profile image
Subramanya Chakravarthy

You might have included this snippet in the article

Collapse
 
toledoroy profile image
Roy Toledo

This is like the #1 #1 part of the article... You might want to add it.

Collapse
 
awakeel profile image
Abdul wakeel

Switch statement itself sometime bound you, we can use factory pattern to return the desire class object ;)

Thread Thread
 
brycedooley profile image
Bryce

Thanks, Abdul. Could you provide an example of what the code might look like? Most factory functions I've seen still involve a switch statement or a series of if/else statements.

Collapse
 
chiubacaa profile image
Alex Chiu ☁

and all these techniques should make unit testing easier too πŸ™Œ

Collapse
 
bennykelly profile image
bennykelly

Great article but for number 6, did you actually introduce an object parameter or am I reading that wrong, looks the same?

Collapse
 
brycedooley profile image
Bryce

Thanks. I did, it's using object destructuring. They do look very similar but in the "after" version there are brackets around the params.

Collapse
 
bennykelly profile image
bennykelly

Ahhhh, thanks for the reply, learned 2 things from this article so! Many thanks again, great article

Collapse
 
tanguyandreani profile image
Tanguy Andreani

Nice article!

Collapse
 
irissiri7 profile image
irissiri7

Great post, very interestingπŸ™‚ I can't say I'm very good at refactoring yet (I'm a beginner still) but I'd love to improve because clean and readible code literally makes me happy!

Collapse
 
lexlohr profile image
Alex Lohr

For #4, TypeScript has enums, which are a lightweight and legible alternative to let a class do the heavy lifting.

Collapse
 
rcsm profile image
Rafael Cascalho • Edited

Just read the article and pretty sure I'm about to use some of these 15 mins from now :D

Collapse
 
gracesnow profile image
Grace Snow

Thanks for this, it's really helpful to read as a beginner.

I find as a good rule of thumb, if I'm having to write comments to explain what the code is doing, I probably need to refractor it!

Collapse
 
brycedooley profile image
Bryce

Love it!

Collapse
 
richardcochrane profile image
Richard Cochrane

Great article and well explained Bryce!