tannerntannern / ts-mixer

A small TypeScript library that provides tolerable Mixin functionality.
MIT License
379 stars 27 forks source link

@mix multiple classes and passing constructor args #48

Closed liamdefty closed 2 years ago

liamdefty commented 2 years ago

Firstly, this lib is neat - thanks for making it!

One thing I'm struggling with (I don't know if it's even possible or how it would work) is how to mix two classes that require constructor args. Here's an example:

import { mix } from 'ts-mixer';
import { Class1, Class2 } from './example';

export interface IMixed<T1, T2> extends Class1<T1>, Class2<T2> {

}

@mix(Class1, Class2)
class Mixed<T1, T2> implements IMixed<T1, T2> {
  constructor(args) {
    super(args); // How would you pass args to the correct class instance here?
  }
}

Is there a different pattern I could follow to get a similar result? Any help would be appreciated!

tannerntannern commented 2 years ago

@liamdefty, what you describe should be possible provided that the constructor signatures are compatible (if they're not, a mixin is probably not the best pattern for the job).

I can't say for certain since I can't see the source for Class1 and Class2 or the error you're experiencing, but I think you may have meant to spread the constructor arguments in your example (constructor(...args){ super(...args))

liamdefty commented 2 years ago

@tannerntannern Ah apologies, the issue I was having was because the constructor signatures aren't compatible, so that makes sense. I guess a mixin probably isn't the best solution here - thanks for the response!

tannerntannern commented 2 years ago

No worries @liamdefty. I could possibly give a more helpful suggestion if I knew more about your use case, but I will leave that up to you. Closing for now

bintoll commented 1 year ago

Hello! Sorry for reopening it, I also have this problem having different constructor arguments in mixins and I am curios how it could be solved other way, may be you have any ideas? I use mixins to add some extra logic (class methods) to the final class and different mixins requires different constructor parameters, which I think is totally fine in this case. So I think the mixins could be a good solution if it would work. My proposal would be to be able to pass these different constructor parameters for each mixin class one by one in same order as they were listed in Mixin function.

tannerntannern commented 1 year ago

I can't say much without knowing more about your use case but in general, when the mixed classes have incompatible constructors, that's a strong indicator that inheritance (mixins) is the wrong tool for the job. You either need to use composition or rewrite your constructors to be compatible.

bintoll commented 1 year ago

Thanks for the response. I have in mind dummy example but I think it should show the idea.

class FightMixin {
   constructor(private readonly arm1, private readonly arm2) {}

   protected fight() {
      // arm1 and arm2 will be used here
   }
}
class RunMixin {
   constructor(private readonly leg1, private readonly leg2) {}

   protected run() {
      // a leg1 and leg2 will be used here
   }
}
class Robot extends Mixin(FightMixin, RunMixin) {
   constructor(private readonly armsAndLegs) {
      const {arm1, arm2, leg1, leg2} = armsAndLegs
      super([arm1, arm2], [leg1, leg2]) // proposal - to list arguments in same order how they will be passed to Mixin classes
  }

   public onWasAttacked() {
      this.fight()
   }

   public onWasScared() {
      this.run()
  }
}

So with mixings I add abilities (using methods of mixing classes) to the Robot class but I need to use there the dependencies that were provided to Robot class.

tannerntannern commented 1 year ago

First of all, changing the way mixin constructor arguments are passed not an option. It would be a major breaking change and inconvenient for cases where constructor signatures are compatible. This leaves you with two options:

The first option (that I'd strongly recommend) is to rewrite the code to use composition rather than inheritance. The result isn't all that different:

class FightService {
   constructor(private readonly arm1, private readonly arm2) {}

   protected fight() {
      // arm1 and arm2 will be used here
   }
}
class RunService {
   constructor(private readonly leg1, private readonly leg2) {}

   protected run() {
      // a leg1 and leg2 will be used here
   }
}
class Robot {
   private FightService fightService;
   private RunService runService;

   constructor(private readonly armsAndLegs) {
      const {arm1, arm2, leg1, leg2} = armsAndLegs
      this.fightService = new FightService(arm1, arm2)
      this.runService = new RunService(leg1, leg2)
  }

   public onWasAttacked() {
      this.fightService.fight()
   }

   public onWasScared() {
      this.runService.run()
  }
}

The second option, if you're determined to use inheritance, is to pass all arguments to all constructors and have each mixed class just pluck out the values they need. Here's what that could look like:

interface Options {
   arm1: Arm,
   arm2: Arm,
   leg1: Leg,
   leg2: Leg,
}

class FightMixin {
   private readonly arm1: Arm;
   private readonly arm2: Arm;
   constructor(options: Options) {
      const { arm1, arm2 } = options;
      this.arm1 = arm1;
      this.arm2 = arm2;
   }

   protected fight() {
      // arm1 and arm2 will be used here
   }
}

class RunMixin {
   private readonly leg1: Leg;
   private readonly leg2: Leg;
   constructor(options: Options) {
      const { leg1, leg2 } = options;
      this.leg1 = leg1;
      this.leg2 = leg2;
   }

   protected run() {
      // a leg1 and leg2 will be used here
   }
}

class Robot extends Mixin(FightMixin, RunMixin) {
   public onWasAttacked() {
      this.fight()
   }

   public onWasScared() {
      this.run()
  }
}

Hopefully this gives you some ideas

bintoll commented 1 year ago

Thanks for response with examples, I got your point. In this case composition of course will be a better option than inheritance with with union constructor parameters for each mixin. It just would be not so comfortable as inheritance because the methods and properties of subclass would not be automatically exposed to superclass. But okay, as it is. Thank you for explanation, I think it could be closed again.