Stop stuffing helpers into Angular services

Stop stuffing helpers into Angular services
Play this article

Does your angular code look like this? Then please think about it twice.

export interface UserDto {
  roles: string[];
  username: string;
  themeIdentifier: string;
}

export class UserService {
  public fetchUser(): Promise<User> {
    return this._api.get('/api/currentUser') as Promise<UserDto>;
  }
  public isAdminUser(user: UserDto): boolean {
    return user.roles.map(c => c.toLowerCase()).includes('admin');
  }
  public themeFor(user: UserDto): any {
    // some logic to fetch theme based on users theme identifier
  }
}

This is a really bad practice for Angular projects and in this issue, I want to talk about the 'why'.

Buzz teaching you the Angular way

What do we look at?

I stumbled over code like that many times. It's an Angular service that fetches data and provides helper functions to operate on that data.

While this might not seem like a problem at first... Did you notice the many UserDto references in the signature of each method?

There's a term for it: Anemic Domain Model

Code like this is described as an anti-pattern by Martin Fowler: The Anemic Domain Model. The logic and the behavior are scattered in multiple different places.

The correct object-oriented way of the previously shown code snippet would look something like this.

export interface UserDto {
  roles: string[];
  username: string;
  themeIdentifier: string;
}

export class User {
  private _theme: Theme;

  public get theme() {return this._theme;}

  constructor(private _roles: string[], private _themeIdentifier: string, public username: string) {
    this._initializeTheme();
  }

  private _initializeTheme() {
    this._theme = new Theme(this._themeIdentifier);
  }

  public isAdminUser(): boolean {
    return this._roles.map(c => c.toLowerCase()).includes('admin');
  }
}

export class UserService {
  public async fetchUser(): Promise<User> {
    const userDto = await this._api.get('/api/currentUser') as Promise<User>;
    return User.fromDto(userDto);
  }
}

// Usage example below
_userService.fetchUser().then((user: User) => {
  user.isAdminUser(); // returns true or false
  user.theme // direct read only access
})

This way is much cleaner and more intuitive to use. Furthermore, with one look you now know exactly what the user can and can't do.

But is it that bad and should we care?

To be honest, I was really mad about angular services violating OOP, especially because it can be much cleaner.

But my research surprised me. I was okay with that approach. Why?

The root of all problems

It's the fact, that you have lots of domain logic in your front-end application. Let's assume you have a simple to-do list app and all the data is processed in the backend.

Will you need an actual implementation of the DTO to keep your methods organized? I don't think so.

If you were to only display data, and provide all necessary information, like isAdminUser within the UserDto. Then you won't need this.

The temptation to mix FP and OOP

Another problem I see with Java-/Typescript is the support for FP and OOP. Many times a helper is born because of sheer laziness. Laziness and stress to find out where a function fits.

FP vs OOP. Hard decision for Mr. Helper

Conclusion

Should you worry and create a concrete class for your interfaces? No! It highly depends on the consumed API.

However: working on a big project with lots of business logic woven into your app, you might consider a refactor to encapsulate the behavior and data in one place.

Show some love with the like button if you found this useful!