Code Smell 22 - Helpers

Do you need help? Who are you gonna call?

TL;DR: Helpers don't help. They are a non cohesive bunch of messy subrutines.

Problems

• Readability

• The Least surprise principle

• Bijection Fault

• Static methods

What exactly is a name - Part II Rehab

Maxi Contieri ・ May 23 '21

#tutorial #codenewbie #programming #webdev

Solutions

1. Find a suitable name

2. If the helper is a library, break all the services as different methods.

3. Methods should always be fulfilled by objects. Static methods are another code smell.

4. Avoid extracting the helpers to Anonymous Functions.

Sample Code

Wrong

export default class UserHelpers {
  static getFullName(user) {
    return `${user.firstName} ${user.lastName}`;
  }

  static getCategory(userPoints) {
    return userPoints > 70 ? 'A' : 'B';
  }
}

Notice static methods.

import UserHelpers from './UserHelpers';

const alice = {
  firstName: 'Alice',
  lastName: 'Gray',
  points: 78,
};

const fullName = UserHelpers.getFullName(alice);
const category = UserHelpers.getCategory(alice);

Right

class UserScore {  
  //This is anemic class and should have better protocol

  constructor(name, lastname, points) {
    this._name = name;
    this._lastname = lastname;
    this._points = points;
  }
  name(){
    return this._name;
  }
  lastname(){
    return this._lastname;
  }
  points(){
    return this._points;
  }
}

class FullNameFormatter {
  constructor(userscore) { 
    this._userscore = userscore;

 }
  fullname() {
    return `${this._userscore.name()} ${this._userscore.lastname()}`;
  }
}

class CategoryCalculator{
  constructor(userscore1){ 
     this._userscore = userscore1;
 }
  display() {
    return this._userscore.points() > 70 ? 'A' : 'B';
  }
}


let alice = new UserScore('Alice', 'Gray', 78);

const fullName = new FullNameFormatter(alice).fullname();
const category = new CategoryCalculator(alice).display();

or we can make the former Helper stateless for reuse...

class UserScore {  
  //This is anemic class and should have better protocol

  constructor(name, lastname, points) {
    this._name = name;
    this._lastname = lastname;
    this._points = points;
  }
  name(){
    return this._name;
  }
  lastname(){
    return this._lastname;
  }
  points(){
    return this._points;
  }
}

class FullNameFormatter {      
  fullname(userscore) {
    return `${userscore.name()} ${userscore.lastname()}`;
  }
}

class CategoryCalculator{
  display(userscore) {
    return userscore.points() > 70 ? 'A' : 'B';
  }
}  

let alice = new UserScore('Alice', 'Gray', 78);

const fullName = new FullNameFormatter().fullname(alice);
const category = new CategoryCalculator().display(alice);

Detection

• Code naming standards should forbid classes with this name on them.

Tags

• Namings

Conclusion

This is a well established cultural name and a legacy habit from structured programming.

Most developers are reluctant to let old habits go.

We must be aware of the damage this kind of names are bringing us.

Relations

Code Smell 18 — Static Functions

Maxi Contieri ・ Nov 6 '20

#codenewbie #tutorial #development #coding

Code Smell 21 - Anonymous Functions Abusers

Maxi Contieri ・ Nov 10 '20

#codenewbie #oop #developing #programming

Also known as

• Utils

More Info

How to Decouple a Legacy System

Maxi Contieri ・ Mar 1 '21

#codenewbie #programming #poo #tutorial

---

This article is part of the CodeSmell Series.

How to Find the Stinky parts of your Code

Maxi Contieri ・ May 21 '21

#codenewbie #tutorial #codequality #beginners

Last update: 2021/07/04

Comments

[Alain Van Hout]

You're showing us 'good' code and 'bad' code, but you're not actually explaining why the one is better than the other or what advantages or disadvantages the two alternatives have. In all, it seems that you've simply rewritten two short functions as multiple longer classes.

[Maxi Contieri]

hi Alain.

Besides rewrite them (most refactorings are about rewrites) solution is less coupled, has a real world name, is more readable and has no static methods.
Length is not a big issue when declarativeness is a trade off.
For me, its is more intention revealing a formatter that formats than a helper that.... hmm helps?

[Alain Van Hout]

That does add more detail, thanks. I personally don't see it as less coupled (use a class vs use a function). As to better names, that's a fact but it would work as well if the functions were renames to e.g. formatToFullName and determineCategory (or something better based on the domain). Static or not seems mostly irrelevant to me, since it doesn't really affect transparency (while I would say that the class boilerplate does reduce transparency).

But a class version of the methods of course also works, so to a degree it's a matter of taste I suppose.

[Maxi Contieri]

indeed. it has some taste on it.

According to SOLID principles Class single responsibility is to create instances. Not to be overloaded with static functions. But it is just an opinion.
As long as we don't call them Helpers I think we are in a better position.