DEV Community

André Degaspari
André Degaspari

Posted on

Refactoring a Coupled Rental API with Strategy, Rules, and Notification Patterns

#softwareengineering #typescript #backenddevelopment #designpatterns

Introduction

In my experience, the natural tendency of a codebase is to become unsustainable, and the responsibility of a good dev is not only to deliver business value but also to keep the codebase in check, ensuring it stays sustainable for as long as the application is needed.

In this text, I'm sharing an example based on real experience of how I combined the strategy, rule objects, and notification design patterns to ensure a product list API stays sustainable for as long as it's needed.

Context

A vehicle rental store needs to adapt to the new market of electric bikes.

The Problem

The current codebase has an endpoint that lists available vehicles for renting. It started with cars, was later extended for motorcycles, and now needs a new extension to accommodate electric bikes. The issue is that each kind of vehicle has its own set of rules, for example, the minimum age required to rent.

Even with unit tests in place, the code is already a bit messy and hard to maintain. Because the logic for all vehicle types is mixed together, it takes time to understand the code, and a change related to car rentals can cause unpredictable behavior in the motorcycle rental response, due to the coupling.

@Injectable()
export class RentalsService {
  constructor(
    private readonly customerService: CustomerService,
    private readonly vehicleService: VehicleService,
  ) {}

  async getAvailableVehicles(
    customerId: string,
    vehicleType: VehicleType,
  ): Promise<GetAvailableVehiclesResponse> {
    const customer = await this.customerService.findById(customerId);

    if (!customer) {
      throw new NotFoundException(
        `Customer with ID "${customerId}" not found.`,
      );
    }

    this.validateRestrictionsForVehicle(customer, vehicleType);

    const vehicles = await this.vehicleService.findAvailableByType(vehicleType);

    return {
      customerId: customer.id,
      vehicleType,
      allowed: true,
      vehicles,
    };
  }

  private validateRestrictionsForVehicle(
    customer: Customer,
    vehicleType: VehicleType,
  ): void {
    if (customer.hasPendingDebits) {
      throw new BadRequestException(
        'Customer has pending debits on the platform.',
      );
    }

    if (customer.hasActiveViolations) {
      throw new BadRequestException(
        'Customer has active traffic violations registered.',
      );
    }

    if (vehicleType === VehicleType.CAR) {
      if (
        !customer.driverLicenseCategory ||
        !['B', 'C', 'D', 'E', 'AB'].includes(
          customer.driverLicenseCategory.toUpperCase(),
        )
      ) {
        throw new BadRequestException(
          'Driver license category B or higher is required for car rentals.',
        );
      }

      if (customer.age < 21) {
        throw new BadRequestException(
          'Minimum age for car rental is 21 years old.',
        );
      }
    } else if (vehicleType === VehicleType.MOTORCYCLE) {
      if (
        !customer.driverLicenseCategory ||
        !['A', 'AB'].includes(customer.driverLicenseCategory.toUpperCase())
      ) {
        throw new BadRequestException(
          'Driver license category A is required for motorcycle rentals.',
        );
      }

      if (customer.age < 18) {
        throw new BadRequestException(
          'Minimum age for motorcycle rental is 18 years old.',
        );
      }
    } else {
      throw new BadRequestException('Unsupported vehicle type.');
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

The Solution

For this case, the solution was to combine the strategy, rules, and notification design patterns to achieve a cleaner view of each use case.

Usage of each pattern:

  • Rules: A set of rules is created to validate the cases we need. Now each validation lives in a rule that is easy to understand, easy to test, and predictable.

  • Notification: For this use case, I need to tell clients why they can't rent a vehicle, since the goal is to help them complete the flow and rent a vehicle from our company. So, for each rule that restricts the client, we add a new notification.

  • Strategy: For each vehicle type (car, motorcycle, and e-bike), we can have a different strategy for combining rules. For example, renting a car requires being 18 years old and having a driver's license, while renting an e-bike requires being 16 and doesn't require a license. Each strategy handles its own particularities.

Implementation:

1 — Create the notification handler

  • It's a simple class that provides methods to add, read, and check notifications stored in an array. 
export class NotificationError {
  constructor(
    public readonly message: string,
    public readonly context?: string,
  ) {}
}

export class Notification {
  private errors: NotificationError[] = [];

  addError(message: string, context?: string): void {
    this.errors.push(new NotificationError(message, context));
  }

  hasErrors(): boolean {
    return this.errors.length > 0;
  }

  getErrors(): NotificationError[] {
    return this.errors;
  }

  getMessages(): string[] {
    return this.errors.map((error) => error.message);
  }
}
Enter fullscreen mode Exit fullscreen mode

2 — Create rules

  • For this use case we always need to know why the client is restricted, so the decision was to have each rule add a notification explaining why it failed.

//MinimumAgeRule

export class MinimumAgeRule implements RentalRule {
  constructor(
    private readonly minAge: number,
    private readonly vehicleLabel: string,
  ) {}

  validate(customer: Customer, notification: Notification): void {
    if (customer.age < this.minAge) {
      notification.addError(
        `Minimum age for ${this.vehicleLabel} rental is ${this.minAge} years old.`,
      );
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

//DriverLicenseCategoryRule

export class DriverLicenseCategoryRule implements RentalRule {
  constructor(
    private readonly allowedCategories: string[],
    private readonly requiredCategoryLabel: string,
    private readonly vehicleLabel: string,
  ) {}

  validate(customer: Customer, notification: Notification): void {
    const customerCategory = customer.driverLicenseCategory?.toUpperCase();

    if (
      !customerCategory ||
      !this.allowedCategories.includes(customerCategory)
    ) {
      notification.addError(
        `Driver license category ${this.requiredCategoryLabel} is required for ${this.vehicleLabel} rentals.`,
      );
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

3 — Create our strategies to handle each vehicle type

//CarRestrictionStrategy

@Injectable()
export class CarRestrictionStrategy implements VehicleRestrictionStrategy {
  readonly vehicleType = VehicleType.CAR;
  private readonly rules: RentalRule[];

  constructor(
    noPendingDebitsRule: NoPendingDebitsRule,
    noActiveViolationsRule: NoActiveViolationsRule,
  ) {
    this.rules = [
      noPendingDebitsRule,
      noActiveViolationsRule,
      new DriverLicenseCategoryRule(
        ['B', 'C', 'D', 'E', 'AB'],
        'B or higher',
        'car',
      ),
      new MinimumAgeRule(21, 'car'),
    ];
  }

  validate(customer: Customer, notification: Notification): void {
    this.rules.forEach((rule) => rule.validate(customer, notification));
  }
}
Enter fullscreen mode Exit fullscreen mode

//EBikeRestrictionStrategy

@Injectable()
export class EBikeRestrictionStrategy implements VehicleRestrictionStrategy {
  readonly vehicleType = VehicleType.E_BIKE;
  private readonly rules: RentalRule[];

  constructor(noPendingDebitsRule: NoPendingDebitsRule) {
    this.rules = [noPendingDebitsRule, new MinimumAgeRule(16, 'e-bike')];
  }

  validate(customer: Customer, notification: Notification): void {
    this.rules.forEach((rule) => rule.validate(customer, notification));
  }
}
Enter fullscreen mode Exit fullscreen mode
  • Notice how the rules are reused between strategies, but the e-bike strategy only knows the rules needed for the e-bike use case, while the car strategy has two more rules (noActiveViolationsRule, DriverLicenseCategoryRule) that we don't need to check when renting an electric bike.
  • This way, we can easily add new rules for one of the cases without risking breaking other parts of the code, while still keeping the advantage of sharing what's common between use cases.

4 — Enabling the strategies for use

  • To make the strategies easy to use, we can have a class that handles mapping the correct strategy to each vehicle type.

  • The getStrategy function receives the desired vehicle type, for example CAR, and returns the class responsible for handling the car use case, CarRestrictionStrategy.

@Injectable()
export class VehicleStrategyRegistry {
  private readonly strategiesMap = new Map
    VehicleType,
    VehicleRestrictionStrategy
  >();

  constructor(
    @Inject(VEHICLE_STRATEGIES)
    strategies: VehicleRestrictionStrategy[],
  ) {
    strategies.forEach((strategy) => {
      this.strategiesMap.set(strategy.vehicleType, strategy);
    });
  }

  getStrategy(vehicleType: VehicleType): VehicleRestrictionStrategy {
    const strategy = this.strategiesMap.get(vehicleType);

    if (!strategy) {
      throw new BadRequestException(
        `Unsupported vehicle type: "${vehicleType}".`,
      );
    }

    return strategy;
  }
}
Enter fullscreen mode Exit fullscreen mode

5 — Putting everything together

  • Finally, we have a RentalsServiceStrategy, the class that receives the desired vehicle type from the API caller, calls getStrategy, and, based on the strategy returned, validates what should be returned to the client.

//RentalsServiceStrategy

@Injectable()
export class RentalsServiceStrategy {
  constructor(
    private readonly customerService: CustomerService,
    private readonly vehicleService: VehicleService,
    private readonly strategyRegistry: VehicleStrategyRegistry,
  ) {}

  async getAvailableVehicles(
    customerId: string,
    vehicleType: VehicleType,
  ): Promise<GetAvailableVehiclesResponse> {
    const customer = await this.customerService.findById(customerId);

    if (!customer) {
      throw new NotFoundException(
        `Customer with ID "${customerId}" not found.`,
      );
    }

    // 1 - Gets the strategy by vehicle type
    const strategy = this.strategyRegistry.getStrategy(vehicleType);

    // 2 - Creates the notification handler
    const notification = new Notification();

    // 3 - Validates process user to populate restrictions
    strategy.validate(customer, notification);

    // 4 - Verifies if the user is restricted
    const allowed = !notification.hasErrors();

    const vehicles = allowed
      ? await this.vehicleService.findAvailableByType(vehicleType)
      : [];

    return {
      customerId: customer.id,
      vehicleType,
      allowed,
      vehicles,
      // 6 - Returns the restrictions for the user if there is any
      ...(notification.hasErrors() && { reasons: notification.getMessages() }),
    };
  }
}
Enter fullscreen mode Exit fullscreen mode
  • We have more code and more contexts to understand, it looks a lot harder to grasp at first glance.
  • But notice how RentalsServiceStrategy doesn't really know about the validations themselves. We could start renting skateboards or airplanes tomorrow, and it wouldn't make a difference to the getAvailableVehicles function.
  • The complex block of code was broken into small pieces that are easier to test, easier to understand in terms of which rules each strategy validates, and much easier to extend without impacting areas we don't expect.
  • These are some of the characteristics that make a project sustainable in the long term.

The Real Life Situation That Inspired This Text

This text is based on a real refactor I did a few months ago. It all started when I was reviewing a big pull request that had been rushed to hit the sprint goals. I felt the code could be improved, but merged the PR anyway, since the functionality had already been approved by QA. I wrote about code reviews here: How code reviews made me a happier developer and what I'm revealing now might look like the opposite of what I said back then, but sometimes we need to weigh business value more heavily than codebase cleanliness.

I kept that API on my list of things to watch. When an extension request came in, I talked to the team about how we couldn't simply extend the API as it was, and how that could be catastrophic for the business down the line. The real-life situation was much more complex than the example above: it involved more rules, integrations with a number of microservices, and building a context to validate the rules that was a lot bigger than just fetching a user with all the properties needed for validation from the database.

After sharing my concerns, I tested the API and found some bugs I had already suspected could happen. I noticed the rest of the team wasn't aware of the complexity, the business rules were also a bit confusing to the project manager, and the use cases were too coupled, which made it difficult for QA to test everything.

I sat down with the PM to build a spreadsheet mapping out the rules, and that made three things clear: we were missing validations, we had bugs happening, and we genuinely needed some kind of refactor.

When I thought about combining three design patterns, I also wondered if I was over-engineering things, and whether other devs would be able to understand the code. After considering that, I decided to move forward with the refactor. I made myself available to help other devs with questions and created in-code documentation explaining how each part of the "restriction module" works.

So far, I've seen two devs implement the restriction module for their own use cases, and I've had to implement it myself in another use case as well. Since the core of the module already exists, we've ended up with smaller PRs that are fast to implement, easy to review, and easy to test.

Curiosity

Even though this text is heavily focused on design patterns, software architecture, and decision-making, all the code in the examples was generated by AI, either the good one and the bad one; I didn't write a single line of it myself.

AI can genuinely boost productivity, but as software engineers, we should still be the ones making the decisions.

You can find the full example code on GitHub: Rentals store example

Conclusion

Design patterns are not a silver bullet, nor the default approach for every situation, that mindset leads straight to over-engineering. But when used at the right place and the right time, they can be the difference between a system that's sustainable and one that isn't.

That difference didn't come from the patterns themselves, it came from the moment I chose to let a messy PR through, and the moment, weeks later, I chose not to let it slide anymore. Knowing when to fix, when to wait, and when a pattern actually earns its place, that's the call that made the real difference here, the patterns were just how I acted on it.

Top comments (0)