Avoiding circular dependencies

Doing so can help you write code that's easier to update and delete.

A circular dependency is when two pieces of code (files, classes, functions, etc) need each other to work (they're typically bad practice since they make code too intertwined). We rarely intentionally add circular dependencies, but they often arise as logic and business requirements become more complicated. They usually occur because two requirements depend on each other (e.g. When sending a team invite to an email, check if a user with that email exists. When deleting a user, remove them from all teams).

Solution 1: Moving Code to a New Service/Module

Moving code is typically the easiest solution. Say we have the following requirements:

  • Allow members of a task to add comments to incomplete tasks

  • Members can invite other users to incomplete tasks

  • A task is complete once every member has approved of it

class TaskService {
  // * A task is complete once every member has approved of it
  isTaskComplete(id) {
    const members = this.taskMembersService.getAll(id);
    return members.every(member => member.hasApprovedTask);
  }

  // * Allow members of a task to add comments to incomplete tasks
  addComment(user, id, comment) {
    this.taskMembersService.assertIsMember(user);
    // Don't allow more comments once complete
    assert(!this.isTaskComplete(id));
    // ... Add comment ...
  }
}

class TaskMembersService {
  assertIsMember(user) {}
  getAll(taskId) {}

  // * Members can invite other users to incomplete tasks
  inviteMember(taskId, fromUser, inviteUserId) {
    // Don't allow inviting to a complete task
    assert(!this.taskService.isTaskComplete(taskId));
    this.assertIsMember(fromUser);
    // ... Add inviteUserId member ...
  }
}

TaskService <--> TaskMembersService

The logic is completely reasonable, and it makes sense how this structure would naturally evolve (e.g. comments existed before we started having task membership). But, of course, we have issues:

  • Before adding a comment (TaskService), check if they're a member of the task (TaskMembersService)

  • When inviting a member (TaskMembersService), check that the task is still open (TaskService)

  • When checking if a task is open (TaskService), make sure all members (TaskMembersService) have approved of it


The problem is that we've made the TaskService a bit too generic. Tasks should be able to exist without members or comments; however, the two methods in our TaskService depend on those.

Without members, isTaskComplete() is nonsense, because a task is complete when all members approve of it. Additionally, addComment() could be described as "Allows a member to add a comment to an open task". This means addComment() correctly depends on isTaskComplete(), especially because we would want addComment() to error if we ever removed the isTaskComplete() function (because to meet the above description for the function, we need a way to check if a task is complete).

So the new service dependencies could look like:

TaskCommentService -> TaskStatusService -> TaskMembersService -> TaskService

We can do a sanity check by ensuring that, if we remove any service, we expect the dependency to produce an error (e.g. if we remove the MembersService, StatusService should break because isTaskComplete() requires the existence of members). This is under the assumption that the business requirements for all other services remain unchanged.

Solution 2: Adding events/hooks

If your requirement can be described as "When X, do Y", you can likely use events or hooks. For example, when a user deletes their account, remove any teams they're on:

class TeamService {
  constructor(
    private readonly userService: UserService,
  ) {
    // An event is called and forgotten about
    this.userService.on('delete-account', this.onDeleteAccount.bind(this));
    // onDeleteAccount(user) {}

    // In contrast, a hook will wait until the callback is completed
    // before deleting the account. This ensures we're never
    // left in a position where we're trying to delete an
    // account but fail to remove them from teams.
    this.userService.deleteAccountHooks.add(this.onDeleteAccount.bind(this));
    // onDeleteAccount(user, sqlTransaction) {}
  }
}

The UserService does not need to know that teams exist at all. If you don't totally know what needs to depend on what, consider the direction of data flow. If I delete the team and team_members table, does the user table still work? Yes? Then the UserService should not know that teams exist. This gives us more flexibility:

  • If we want to delete teams and the TeamService for any reason, there's less depending on it. "Remove a user from teams when their account is deleted" is inherently team logic and should be removed when deleting the TeamService (because that logic doesn't make sense without teams).

  • If we want to expand TeamService for any reason, there's less depending on it. For example, if we want team members that can either be a user or an AI (why not), we don't have to worry about a single line of code in the UserService going "oh no, how do I handle the case of an AI team member?"

What about a more direct circular dependency?

Consider the requirements:

  • When the CreateTournament job is triggered, we should create a tournament

  • After a tournament is created, schedule any child tournaments (if applicable)

class CreateTournamentJob {
  add(tournamentOpt, atTime) { ... }

  // This is called when it is time for the job to run
  process(tournamentOpt) {
    this.verifierService.assertValidOpt(tournamentOpt);
    this.tournamentService.create(tournamentOpt);
  }
}

class TournamentService {
  create(tournamentOpt) {
    // ... Create the tournament ...

    // Schedule child tournaments to be created 5 minutes from now
    for (const childOpt of tournamentOpt.childrenOpt) {
      this.createTournamentJob.add(childOpt, Date.now() + FIVE_MINUTES);
    }
  }
}

There are a couple of ways we can handle this, and it all depends on what our requirements are:

Hook on "create tournament"

class CreateTournamentJob {
  constructor(private readonly tournamentService: TournamentService) {
    this.tournamentService.onCreateTournamentsHooks.add(async (opt) => {
      for (const childOpt of opt.childrenOpt) {
        this.add(childOpt, Date.now() + FIVE_MINUTES);
      }
    });
  }

  add(tournamentOpt, atTime) {}

  process(tournamentOpt) {
    this.verifierService.assertValidOpt(tournamentOpt);
    this.tournamentService.create(tournamentOpt);
  }
}

This only makes sense if deleting the tournament job should make it so we no longer schedule child tournaments. If this isn't your requirement, you will run into issues. For example, perhaps you manually added tournaments to the tournament job in the past and no longer need that. You delete CreateTournamentJob and poof, suddenly child tournaments aren't generating anymore... but there weren't any errors!

Return from create()

Perhaps in another world, it makes sense to return the child tournaments so they can be acted upon. This would make sense only if we wanted to schedule child tournaments when calling from the scheduler, but handle them differently elsewhere:

class CreateTournamentJob {
  add(tournamentOpt, atTime) {}

  process(tournamentOpt) {
    const childTourns = this.tournamentService.create(tournamentOpt);
    // ... Schedule child tournaments ...
  }
}

class TournamentService {
  create(tournamentOpt) {
    // ... Create the tournament ...
    return tournamentOpt.childrenOpt.map(what => ever)
  }

  // Maybe called from an admin panel, or if users are allowed to make their own tournaments
  manuallyCreate(byUser, tournamentOpt) {
    const childTourns = this.create(tournamentOpt);
    // ... Handle child tournaments, e.g. maybe we create them immediately in this case
  }
}

Or, leave the circular dependency

Or, the final case is that the circular dependency makes sense. If we remove "creating tournaments", we WANT the job to fail. If we remove the job, we WANT "creating tournaments" to fail because otherwise it can't handle child tournaments correctly.

Wrapping Up

We covered 3 ways to resolve circular dependencies:

  1. Splitting code up into different files

  2. Using hook/events

  3. Returning data so the caller

But ultimately, in some cases it may even make sense to leave a circular dependency in place if it aligns with your business requirements. Updating and deleting code are preferable over new code, but with messy depdenencies, they're likely to be more pain than they're worth. Make it easy to update and delete and you'll find yourself with a smaller codebase doing the exact same thing.

The easiest way to check if you have your dependencies set up well is to ask yourself, for any given piece of code: If I delete this code/edit the types & functionality, are all the errors representative of all the areas that need their business requirements changed? Or is it merely busy work to fix the error? By doing so, you can avoid potential pitfalls and create software that is easier to understand, maintain, and expand. Happy coding!