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 {
isTaskComplete(id) {
const members = this.taskMembersService.getAll(id);
return members.every(member => member.hasApprovedTask);
}
addComment(user, id, comment) {
this.taskMembersService.assertIsMember(user);
assert(!this.isTaskComplete(id));
}
}
class TaskMembersService {
assertIsMember(user) {}
getAll(taskId) {}
inviteMember(taskId, fromUser, inviteUserId) {
assert(!this.taskService.isTaskComplete(taskId));
this.assertIsMember(fromUser);
}
}
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,
) {
this.userService.on('delete-account', this.onDeleteAccount.bind(this));
this.userService.deleteAccountHooks.add(this.onDeleteAccount.bind(this));
}
}
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) { ... }
process(tournamentOpt) {
this.verifierService.assertValidOpt(tournamentOpt);
this.tournamentService.create(tournamentOpt);
}
}
class TournamentService {
create(tournamentOpt) {
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);
}
}
class TournamentService {
create(tournamentOpt) {
return tournamentOpt.childrenOpt.map(what => ever)
}
manuallyCreate(byUser, tournamentOpt) {
const childTourns = this.create(tournamentOpt);
}
}
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:
Splitting code up into different files
Using hook/events
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!
