Your way to better abstractions

When done right, abstractions are powerful. Though they are difficult, tricky and need maintenance they help to reuse the same code in different places. Fixing a bug in one place would mean fixing in all other places where they are used. But when they go wrong (as they often do), they don’t age well. They resist change and become very difficult to manage.

How does this happen? It happens due to principles like DRY and Write Everything Twice which inherently focus on duplication or redundancy in code. I am not saying that these principles are bad by design, but that following them religiously could lead to bad abstractions. Abstractions are difficult and a good abstraction will change over time.

“Duplication is far cheaper than the wrong abstraction”

said Sandi Metz in The Wrong Abstraction and personally, I agree with her.

Initially an abstraction is added to handle a particular use case. Over time when more and more similar use cases are added, it would look like a good abstraction in the beginning, and later end up being very different from what you had intended, as it had to handle too many use cases. An example for this is Chinese whispers.

A Clear Concept: An Example

Let me try to explain this with an example. Consider having a streaming application with Free and Premium content. Paid customers get Premium + Free content and free users get only Free content.

You start by checking if the user is premium or not and return the content on this basis.

1// content.js
2function getContent(user) {
3  const isPremium = user.premiumEndDate >= today
4  return isPremium ? freeContent.concat(premiumContent) : freeContent
5}

This seems fine, nothing fancy.

Later, you decided to add advertisements to your product. If the user is not a premium member they are shown some ads.

1// home-banner.js
2function getSellingScreen(user) {
3  const isPremium = user.premiumEndDate >= today
4  return isPremium ? null : sellingScreen
5}

As soon as you give this for review, your colleague says “Hey, I have seen isPremium check before. Can you abstract that out?”

So you create a new abstraction in your shared folder with user-membership.js and introduce a method to check membership.

1// user-membership.js
2function isPremiumUser(user) {
3  return user.premiumEndDate >= today
4}
5
6// content.js
7function getContent(user) {
8  const isPremium = isPremiumUser(user)
9  return isPremium ? freeContent.concat(premiumContent) : freeContent
10}
11
12// home-banner.js
13function getSellingAds() {
14  const isPremium = isPremiumUser(user)
15  return isPremium ? null : sellingScreen
16}

Later, the feature for a user to pause their premium membership is also introduced.

This means, these users shouldn’t see Premium Content They also shouldn’t see the selling ads.

1// content.js
2function getContent(user) {
3  const isPremium = user.premiumEndDate >= today
4  return isPremium ? freeContent.concat(premiumContent) : freeContent
5}

Since your isPremiumUser doesn’t consider the state of the membership, you introduce a check onlyActive. So you wouldn’t break other places where this abstraction is used and hence would default to false. So you decide to add this condition in the abstraction itself. Thus you introduce a new field: membershipStatus in your User entity.

You update both getContent and getSellingAds. You also rename the variable to isActivePremium to have better understanding.

1// user-membership.js
2function isPremiumUser(user, { onlyActive = false } = {}) {
3  const isPremium = user.premiumEndDate >= today
4  if (!onlyActive) {
5    return isPremium
6  }
7  return isPremium && user.membershipStatus === "active"
8}
9
10// content.js
11function getContent(user) {
12  const isActivePremium = isPremiumUser(user, { onlyActive: true })
13  return isActivePremium ? freeContent.concat(premiumContent) : freeContent
14}
15
16//  home-banner.js
17function getSellingAds() {
18  const isPremium = isPremiumUser(user)
19  return isPremium ? null : sellingScreen
20}

This looks good. Not too shabby for a Friday night push.

After a few weeks, you realise that premium users often forget to resume their membership. So you decide to show a reminder in the banner space, where we used to show ads, when a premium user is watching it.

Thus you update your getSellingAds method to handle the new cases. Let’s also update the name of the method to getBannerContent

1// user-membership.js
2function isPremiumUser(user, { onlyActive = false } = {}) {
3  const isPremium = user.premiumEndDate >= today
4  if (!onlyActive) {
5    return isPremium
6  }
7  return isPremium && user.membershipStatus === "active"
8}
9
10// content.js
11function getContent(user) {
12  const isActivePremium = isPremiumUser(user, { onlyActive: true })
13  return isActivePremium ? freeContent.concat(premiumContent) : freeContent
14}
15
16//  home-banner.js
17function getBannerContent() {
18  const isPremium = isPremiumUser(user)
19  const isPremiumActive = isPremiumUser(user, { onlyActive: true })
20  if (!isPremium) {
21    return sellingScreen
22  }
23  if (!isPremiumActive) {
24    return ResumeReminder
25  }
26  return null
27}

Great work! You have successfully reused your abstraction. Now you give it for review. But the Reviewer isn’t very happy. The meanings of the checks seem to be very vague. For example,what is the difference between isPremium and isPremiumActive? You convince your reviewer with well thought out comments. Since your getSellingAds is not only showing ads but also reminder, you renamed the function to getBannerContent.

A few weeks later, your premium content is given for a free trial. This means, Users who are not premium members have to see the premium content. At the same time,they should also see the ads. You decide to handle this use case in your isPremiumUser by introducing another check includeFreeTrial, which again defaults to false.

Now the code looks like this:

1// user-membership.js
2function isPremiumUser(
3  user,
4  { onlyActive = false, includeFreeTrial = false } = {}
5) {
6  const isPremium = user.planType === "premium"
7  if (!onlyActive || !includeFreeTrial) {
8    return isPremium
9  }
10  const isActivePremium = isPremium && user.isMembershipStatus === "active"
11  const isUserFreeTrial = user.planType === "freeTrail"
12  if (onlyActive && includeFreeTrial) {
13    return isUserFreeTrial || isActivePremium
14  }
15  if (onlyActive) {
16    return isActivePremium
17  }
18  return user.isUserFreeTrial
19}
20
21// content.js
22function getContent(user) {
23  const isActivePremium = isPremiumUser(user, {
24    onlyActive: true,
25    includeFreeTrial: true,
26  })
27  return isActivePremium ? freeContent.concat(premiumContent) : freeContent
28}
29
30// home-banner.js
31function getBannerContent() {
32  const isPremium = isPremiumUser(user)
33  const isPremiumActive = isPremiumUser(user, { onlyActive: true })
34  if (!isPremium) {
35    return sellingScreen
36  }
37  if (!isPremiumActive) {
38    return ResumeReminder
39  }
40  return null
41}

At this point, isPremiumUser is handling too many cases. In the beginning we thought only premium users could watch premium content. But that is not the case anymore. So now you have too many boolean checks in the “abstraction” and you are handling some unused cases like, onlyActive = false and includeFreeTrial = false.

Not happy about this? No choice but to push it ahead as it is an urgent requirement.

This is the first step where you should understand that your abstraction is doing far more than it was intended to do. This happened because our initial assumption that only premium users can watch premium content was wrong. And this happens often and the right thing to do at this point is to take back a step. Re consider: what exactly is your abstraction doing now? Do you still need this abstraction? Your abstractions need to change with your business requirements.

So now you try to inline your abstractions, copy and paste the abstraction where it is used. You can only do this if you have integration tests. If you don’t have enough integration tests add them before inlining the abstractions.Unit tests won’t help you here as you had mocked these abstractions.

1// content.js
2function getContent(user) {
3+  const isFreeTrial = user.planType === "freeTrail"
4-  const isActivePremium = isPremiumUser(user, {
5-    onlyActive: true,
6-    includeFreeTrial: true,
7-  })
8+  const isActivePremium =
9+    user.planType === "premium" && user.membershipStatus === "active"
10-  return isActivePremium
11+  return isFreeTrial || isActivePremium
12    ? freeContent.concat(premiumContent)
13    : freeContent
14}
15
16// home-banner.js
17function getBannerContent() {
18-  const isPremium = isPremiumUser(user)
19-  const isPremiumActive = isPremiumUser(user, { onlyActive: true })
20-  if (!isPremium) {
21+  if (user.planType !== "premium") {
22    return sellingScreen
23  }
24- if (!isPremiumActive) {
25+  if (user.membershipStatus !== "active") {
26    return resumeReminder
27  }
28  return null
29}

Now the code looks much cleaner despite no abstraction being used. Hold on! But you still need abstractions! You can’t copy the same code over and over without abstracting. Now it is clear that we have introduced planType and membershipStatus which weren’t there when we started, at which point we couldn’t have predicted this.

1// user-membership.js
2function getUserPlanType(user) {
3  return user.planType;
4}
5
6// content.js
7function getContent(user) {
8  const userPlanType = getUserPlanType(user);
9  const isFreeTrial = userPlanType === 'freeTrail';
10  const isPremium = userPlanType === ‘premium’;
11  const isActivePremium = isPremium && user.membershipStatus === 'active';
12  return isFreeTrial || isActivePremium
13    ? freeContent.concat(premiumContent)
14    : freeContent;
15}
16
17// home-banner.js
18function getBannerContent() {
19  const userPlanType = getUserPlanType(user);
20  if (userPlanType !== 'premium') {
21    return sellingScreen;
22  }
23  if (user.membershipStatus !== 'active') {
24    return resumeReminder;
25  }
26  return null;
27}

This is your new abstraction and here you have a simpler way of getting the plan type: getUserPlanType. You could add another abstraction to membership status, but you have already seen that it is not used much and decide to add it later. This is perfectly okay: to have a duplicate code until you figure out a better abstraction. A few weeks later, you might come across a new use case which could give you a better idea on what to abstract and what not to.

Some might say that the abstraction is being used in too many places and that we can’t just inline every usage. Again, this is okay and you can deprecate the use of the old abstraction. You can then migrate on a case to case basis whenever you are working on the module. After enough tries, you have come up with better abstractions. You realised the abstraction you were looking for was isEligibleForPremiumContent and not isPremiumUser.

1// user-membership.js
2function isPremium(user) {
3    return getUserPlanType(user) === ‘premium’;
4}
5
6function isFreeTrial(user) {
7    return getUserPlanType(user) === ‘freeTrial’;
8}
9
10function isActivePremium(user) {
11    return isPremium(user) && user.membershipStatus === 'active'
12}
13
14function isEligibleForPremiumContent(user) {
15    return isFreeTrial(user) || isActivePremium(user);
16}
17
18function isPausedPremium(user) {
19    return isPremium(user) && user.membershipStatus === ‘paused’;
20}
21
22// content.js
23function getContent(user) {
24  return isEligibleforPremiumContent(user)
25    ? freeContent.concat(premiumContent)
26    : freeContent;
27}
28
29
30// home-banner.js
31function getSellingAds() {
32  if (!isPremium(user)) {
33    return sellingScreen;
34  }
35  if (isPausedPremium(user)) {
36    return resumeReminder;
37  }
38  return null;
39}

These are the few possible abstractions you could come up with after a few more iterations. And when compared with the old abstraction, this looks much better.

Always remember:

• Similar looking code shouldn’t be the only criterion for abstraction, include your business requirement too.
• You will eventually end up in bad abstraction if you don’t pause to re-think the abstraction with changes in business requirements.
• It is okay to have duplicate code in a few places.
• Your integration tests need to be closer to your business logic rather than your abstractions.
• Keep your abstractions lean.

There is no hard and fast rule for good abstraction. It is okay to duplicate the code until a good abstraction is found and does not mean that the same code will be duplicated all over the codebase. Nobody wants to fix the same bug at 10 different places.

Duplicate until you find your abstraction.