Is circular reference with Typescript array properties bad design

circular-dependencyobject-oriented-designtypescript

I understand that having circular dependency can be bad design. However, I have a question regarding a certain class structure.

As an example:

ocean.ts

import {Boat} from './boat';

export class Ocean {
    boats: Array<Boat> = [];

    getWaterLevel() {
        return 5;
    }

    createBoats() {
        for (let i = 0; i < 10; i++) {
            const boat = new Boat();
            boat.ocean = this;
            boat.engineRunning = true;

            this.boats.push(boat);
        }
    }
}

boat.ts

import {Ocean} from './ocean';

export class Boat {
    engineRunning: boolean;
    ocean: Ocean;

    canMove() {
        return this.ocean.getWaterLevel() > 5 && this.engineRunning;
    }
}

In Typescript this can't be done without a circular reference problem from the imports. I've also read people conclude that its a sign of bad design. The only other solution I could see is to create a third layer which is something like OceanBoat and manage the two resources. Is this bad design or a bad limitation of Typescript? Are there any better solutions for handling this without merging files into one or creating a abstract layer like "OceanBoat"? Is there anything like require_once() for Typescript?

Best Answer

Your situation is actually language agnostic and I would consider it to be a bad design in any programming language.

The problems with circular references generally include the chicken&egg problem, because when you want to instantiate an object you do not know which you should instantiate first. Should you instantiate an ocean? But then you have to also provide boats, which require the ocean? It's a closed circle.

Without introducing a third layer, this problem can be easily solved by only referencing objects by their identities, and instead of having a direct dependency on a big object as a part of constructor, methods will accept such object when needed.

So, instead of depending on an Ocean instance, a Boat would depend on an oceanId. But because you need the ocean to check the water level, you would pass an Ocean instance to the canMove method:

import {Ocean} from './ocean';

export class Boat {
    engineRunning: boolean;
    oceanId: OceanId;

    canMove(ocean: Ocean) {
        OceanId oId = ocean.Id();

        if (!this.oceanId.equals(oId)) {
            throw new RuntimeException("Passed ocean does not match boat ocean.");
        }

        return ocean.getWaterLevel() > 5 && this.engineRunning;
    }
}

If necessary, you could then refactor the code even further by adding a logical method on the Ocean, that way you could implement a pattern similar to double-dispatch, adding a property to configure required water level for a boat which would be used to determine whether a given ocean has such water level or does not:

export class Ocean {

    hasWaterLevelAbove(waterLevelToCheck: int) {
        return 5 > waterLevelToCheck;
    }
}

export class Boat {
    requiredWaterLevel: int;
    engineRunning: boolean;
    oceanId: OceanId;

    canMove(ocean: Ocean) {
        OceanId oId = ocean.Id();

        if (!this.oceanId.equals(oId)) {
            throw new RuntimeException("Passed ocean does not match boat ocean.");
        }

        return ocean.hasWaterLevelAbove(this.requiredWaterLevel) && this.engineRunning;
    }
}

I would argue this isn't the chicken or the egg problem. A ocean doesn't have to be created for a boat but a ocean has to be created for a boat. Does it not seem redundant to pass through a variable ocean when you intend your boat to be in only one ocean at a time? If it changes ocean, you would change it within its property, that makes real world sense.

And even then, a boat doesn't have to have an ocean. It could be being built at the moment.

When you persist an ocean, I can imagine you could have a method name e.g. loadOcean, loading the ocean from the database. How are you planning to create an instance of an ocean with all its boats which require an instance of the ocean? Sure, you could create an empty ocean object, assign its reference to the boats, modify the ocean reference later on by assigning it other properties and finally also assign to the reference the boats. But this is not a very nice approach.

And when you do persist a boat and want to work somewhere with a single boat, you would have a loadBoatById method to fetch a persisted boat, but only to fetch a single boat you would need to fetch the entire instance of the ocean the boat is associated with which would then force you to fetch all the boats in the ocean (because of your design) causing an unnecessary memory overhead.

Whats even more funny is this would still cause a circular dependency problem because you've type scripted the ocean variable causing the import on boat.ts.

Circular dependencies are not created by importing libraries in a circle. They're created by misusing the imported libraries. Having a boat.ts import Ocean and ocean.ts import Boat in the way I've described is perfectly fine and shouldn't be any problem for any modern compiler.

E.g. in C++, you don't even have to import anything, simply putting the following structure should fail during compilation because it's impossible to determine the necessary stack size on compile-time for the application to even execute (because A needs B which needs A which needs B,...):

class A {
private:
    B b;
}

class B {
private:
    A a;
}

Surprisingly, the following would compile in C++ (since you would know the size of a pointer at compile time) but would still be considered a bad design:

class A {
private:
    B *b;
}

class B {
private:
    A *a;
}
Related Topic