0
\$\begingroup\$

I am trying to figure out how Factory pattern works. I implemented it with TypeScript in this github repo: https://github.com/emilecollin/factory_pattern

Basically this is emulating spaceships gaming logic: the program will ask you to input in the terminal one of these options: "alien" or "zombie". With the provided input, the program will select on runtime the proper ship class to instantiate (AlienShip or ZombieShip), and a spaceships fight is mocked. The logic for the decision of which spaceship class to instantiate lives in a «factory method».

The abstract creator:

export abstract class AbstractShipFactory {
  public abstract makeShip(shipType: string): Ship;
}

The concrete creator:

export class EnemyShipFactory extends AbstractShipFactory {
  public makeShip(shipType: string): Ship {
    switch (shipType) {
      case 'alien':
        return new AlienShip();
      case 'zombie':
        return new ZombieShip();
    }
  }
}

The Product interface:

export interface Ship {
  name: string;
  damage: number;
  attack(): string;
}

And the Concrete Product class:

export class AlienShip implements Ship {
  private _name: string = 'alien';
  private _damage: number = 35;

  get name(): string {
    return this._name;
  }

  get damage(): number {
    return this._damage;
  }

  attack(): string {
    return '---->You are being attacked by ' + this.name + ' spaceship and got a damage of ' + this.damage;
  }
}

I'm now sure if I got it right. Still a bit confused about it. Any opinion of this code will be welcome!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Because the desire to improve code is implied for all questions on this site, can you please update the question title to describe what the code is supposed to do rather than your concern about it? Also good would be to add a bit more description to the first part of the question. Tell us what the code is supposed to do and you'll get much better reviews. See How to Ask for examples. \$\endgroup\$
    – Edward
    Dec 17 '19 at 0:28
2
\$\begingroup\$
  1. There's no need to pass a string to makeShip. You should instead create 2 methods: makeAlienShip and makeZombieShip. This way as someone working on your code I can let my editor autocomplete and you won't have to write as much documentation.

  2. As per above, having an AbstractShipFactory is an overuse of abstraction. What you could do is instead just make a ShipFactory that makes different types of ships: allied, enemy, neutral, zombie, pirate, alien etc.

  3. Try to use template strings instead. It makes it much more readable especially if you're spitting out long messages. Like so:

return `---->You are being attacked by ${this.name} spaceship and got a damage of ${this.damage}`
  1. Why is attack() responding with the string "You are being attacked by..."? This is extremely awkward and hard to understand: can a ship attack another ship? Or can a ship only attack the player? Maybe you could change the method name to takeDamageFromShip(). Even better would be to make a method attack(ship: Ship)

Think more about the relationships between objects in your program. You defined what a Ship is, but how does it interact with other objects in your program?

  1. If zombie, alien, etc. are meant to be types of a Ship, add a string to your Ship interface instead.

  2. Why are you not using a constructor to instantiate the properties of AlienShip? What if certain ships do more damage than others?

\$\endgroup\$
1
  • \$\begingroup\$ Thanks @taimoorahmad. About 1 and 2, I'm not sure if you are talking about factory method pattern or a simple factory. The former requires an abstract creator with a factoryMethod, and a creator that modifies it via polymorfism: en.wikipedia.org/wiki/Factory_method_pattern. About 3, 4, 5, 6, those issues have nothing to do with the question itself, sorry if they mislead you. \$\endgroup\$
    – user215126
    Dec 17 '19 at 21:20

Your Answer

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