2

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?

  • Compared to some other language such as Java, this is a limitation but other languages also don't allow this. Whether it's a 'bad' limitation is a subjective question. – JimmyJames Jun 10 '19 at 14:58
  • I understand the benefit of enforcing this in a A to B / B to A scenario. But in a situation where the parent has a list of children and the children knows about its parent, I see it as a bad limitation. – Ray Jun 10 '19 at 15:00
  • Do you want options for solving this problem? Right now the way you have put it, it's a yes/no question. I suggest editing it if you want help with the solution. – JimmyJames Jun 10 '19 at 15:11
  • Fair enough, looking to see if there is a eloquent solution. – Ray Jun 10 '19 at 15:15
1

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;
}
  • 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. – Ray Jun 10 '19 at 21:20
  • And even then, a boat doesn't have to have an ocean. It could be being built at the moment. – Ray Jun 10 '19 at 21:26
  • 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. – Ray Jun 10 '19 at 22:30
  • @Abstract I have answered your comments in the question. – Andy Jun 11 '19 at 5:04
  • How do we move to a chat? Your examples while I appreciate them, appear to make a case that very simple things that are done eloquently in other languages are a very bad idea. You would instantiate the ocean first, then load in your boats. This is how OOP works. Your comment on, "what if theres only one boat, memory overhead" That doesn't matter because in the way you're providing as a solution you would still need an ocean which would be the same memory usage. You wouldn't have to fetch all boats to use the ocean for a single boat. – Ray Jun 11 '19 at 13:23
1

The way I would typically go about this is to use an interface. I'm not familiar with TypeScript but it supports interfaces. It also seems duck typing might be an option but either way the result is the same.

To resolve the circular dependency, I would remove the tight coupling from Boat to Ocean at the very least. For example you could create an interface called BodyOfWater or whatever makes sense. Then Boat doesn't need to know about Ocean and you can also come up with other bodies such as lakes and rivers.

You could do something similar for Boat and create an interface called Vehicle but this isn't necessary to solve the specific problem at hand.

Here's my best attempt at an example (I don't know typescript.)

bodyofwater.ts

interface BodyOfWater {
    getWaterLevel(): number;
}

ocean.ts

import {Boat} from './boat';
import {BodyOfWater} from './bodyofwater';

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

    getWaterLevel() {
        return 5;
    }

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

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

boat.ts

import {BodyOfWater} from './bodyofwater';

export class Boat {
    engineRunning: boolean;
    bodyofwater: BodyOfWater;

    canMove() {
        return this.bodyofwater.getWaterLevel() > 5 && this.engineRunning;
    }
}
  • you'll still have the circular reference problem with interfaces because you'll have to import that interface into your class which would have the imports of the classes you were trying to avoid. – Ray Jun 10 '19 at 15:58
  • I think what you're calling for is adding an abstract layer/middleman class which is really only there because of this circular dependency issue. That would work, however doesn't seem optimal. – Ray Jun 10 '19 at 15:59
  • I don't follow. "which would have the imports of the classes" What would have these? The interface? If so, that is not the case. – JimmyJames Jun 10 '19 at 16:00
  • import {OceanInterface} from './ocean-interface'; class Ocean implements OceanInterface – Ray Jun 10 '19 at 16:01
  • 2
    Also, I would recommend against things like OceanInterface. You should think in more abstract terms when defining an interface. – JimmyJames Jun 10 '19 at 16:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.