0

I'm managing the UI for a small product line. All of these products are hardware that more or less have the same modules, but as a unit themselves, may have extra global features. These global features interact with these modules, often providing multiple modules the same features. I am trying hard to maintain an OOP approach with logical class inheritance, but sometimes it seems impossible to not have to copy-paste some functionality.

In this example, I illustrate the case with LightSwitch. Both Product A and Product B can have a LightSwitch. Product B also has an AlarmSwitch. Product B's implementation of LightSwitch and AlarmSwitch also require a timer with exactly the same functionality. Product A does not support a timer.

The only solution I see here is to copy-paste across modules. I can't think of any other way to extend this without breaking the model.

// Common components

class Switch {
  constructor() {
    this.state = false;
  }
  open() {
    this.state = true;
  }
  close() {
    this.state = false;
  }
}

class LightSwitch extends Switch {
  constructor() {
    super();
    this.colour = "white";
  }
  changeColour(colour) {
    this.colour = colour;
  }
}

// Product A 

class LedSwitch extends LightSwitch {
  constructor() {
    super();
    this.blink = false;
  }
  blink(state) {
    this.blink = state;
  }
}

// Product B
// AlarmSwitch and ProductBLightSwitch both need a "timer"

class AlarmSwitch extends Switch {
  constructor() {
    super();
    this.sound = "alarm.wav";
    this.timer = 5000;
  }
  changeSound(sound) {
    this.sound = sound;
  }
  resetTimer() {
    this.timer = 0;
  }
}

class ProductBLightSwitch extends LightSwitch {
  costructor() {
    super();
    this.timer = 5000;
  }
  resetTimer() {
    this.timer = 0;
  }
}
yallo
  • 29
  • 5
  • Don't feel compelled to wrench everything into one paradigm. Use whatever language features you have at your disposal to write natural solutions. Perhaps a compositional approach where you create a separate `Timer` object that provides the generic timer functionality, like `reset()`, and receives a callback function for whatever specific functionality is needed when the timer goes off. Then whatever classes need a timer can have their own instance of one. –  Apr 27 '17 at 13:02
  • I recommend you search for `favor composition over inheritance`. There are cases where you use inheritance and cases where you use composition. I understand that you are running through an "inheritance hell" case. You can try to think things naturally. If is not natural to consider that Y is X then maybe you can think more in terms of Y has a relationship with X. Even you in your question use "something has other-something" vs. "something is other-something". Maybe an AlarmSwitch is a LightSwitch but maybe ProdA is not a LightSwitch but has a LightSwitch. – andreim Apr 27 '17 at 13:06
  • I actually do what both of you are describing. I move things into sub components when it makes sense. In this case, the `timer` is a good example. A timer can exist on its own. But on its own, it does nothing. It needs to hook up to `AlarmSwitch` and `ProductBLightSwitch` in this case. And the thing is, for both of those classes, it hooks up in the exact same ways... – yallo Apr 27 '17 at 13:28
  • I would definitely not use inheritance and classes to model a product catalog. You will be tweaking and reorganizing it for the rest of your life. –  Apr 27 '17 at 13:33
  • @yallo: Right. Nothing wrong with having a generic `Timer` that receives functionality from outside. And actually, having your objects fulfill an interface, and passing themselves to the `Timer`, which invokes the expected method, is probably even better. ...like this: https://jsfiddle.net/bzf9a5z6/ –  Apr 27 '17 at 13:37
  • I think we're edging past the point. The point is, I have many products that have many modules. Both the products and the modules are nearly identical. However, one product may provide functionality to all of these modules that another product does not. Since I cannot go "under" (or perhaps inject midway into) the inheritance chain, I can only go "on top", but I must copy-paste this product-specific functionality to each module... if that makes sense. – yallo Apr 27 '17 at 13:42
  • @squint I see the value in your example. It might just work..Essentially this sub-component could add features to the module simply by passing the `invoker` (`this`). Although this would be dangerous if done directly, because multiple components will likely have the same method names. – yallo Apr 27 '17 at 13:49
  • Not sure of the dangers you're thinking about. But having the objects fulfill an interface is just one approach. They could also provided the needed functionality in `Timer` via a callback function. https://jsfiddle.net/hmzqx8z0/ The main point is to not rely on inheritance exclusively. People have abandoned inheritance models due to some ugly code that's been written for it. I don't think it needs to be thrown out entirely. There's good middle ground to be found. –  Apr 27 '17 at 14:00
  • I was thinking if the sub-component sort of latched on to the main component by extending it with new methods. If you pass `this` to the sub component, it could add anything from properties to prototypes to it. That would be dangerous though if multiple sub-components did this and they had the same method names. However you are not really suggesting this. My issue is just much more complex than a timer with a callback, but I will consider your ideas. – yallo Apr 27 '17 at 14:13

1 Answers1

0

One might consider already refactoring and decomposing the // Common components into (I) function based mixins / traits that carry behavior and into (II) kind of hull classes that are just responsible for a type's signature and for gluing together already existing code (code reuse via composition and inheritance). In the same time one also might care more about how to access/hide an object's current state. Such an approach is going to be demonstrated with the following refactored example code of the OP ...

// Common components

var
  withSerializableTypeProxy = (function () {
    function toString(type) {
      return JSON.stringify(type);
    }
    function valueOf(type) {
      return JSON.parse(JSON.stringify(type))
    }
    let defineProperty = Object.defineProperty;

    return function trait (stateValue) {
      const
        compositeType = this;

      defineProperty(compositeType, 'toString', {
        value: (() => toString(stateValue))
      });
      defineProperty(compositeType, 'valueOf', {
        value: (() => valueOf(stateValue))
      });
    };
  }());


function withChangeableColorProxy(stateValue) {
  const
    compositeType = this;

  compositeType.changeColor = ((colorValue) => stateValue.color = colorValue);

  Object.defineProperty(compositeType, 'color', {
    enumerable: true,
    get: (() => stateValue.color)
  });
  stateValue.color = 'white'; // set default value.
}


class Switch {
  constructor(stateValue) {

    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};
    stateValue.isOpen = true;

    const
      switchType = this;

    withSerializableTypeProxy.call(switchType, stateValue);

    switchType.open = (() => stateValue.isOpen = true);
    switchType.close = (() => stateValue.isOpen = false);

    Object.defineProperty(this, 'isOpen', {
      enumerable: true,
      get: (() => stateValue.isOpen)
    });
  }
}


class LightSwitch extends Switch {
  constructor(stateValue) {
    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};

    super(stateValue);

    withChangeableColorProxy.call(this, stateValue);
  }
}


var
  lightSwitch = (new LightSwitch);

console.log("lightSwitch : ", lightSwitch);

console.log("(lightSwitch instanceof LightSwitch) ? ", (lightSwitch instanceof LightSwitch));
console.log("(lightSwitch instanceof Switch) ? ", (lightSwitch instanceof Switch));

console.log("(lightSwitch + '') : ", (lightSwitch + ''));
console.log("lightSwitch.toString() : ", lightSwitch.toString());
console.log("lightSwitch.valueOf() : ", lightSwitch.valueOf());

console.log("Object.keys(lightSwitch) : ", Object.keys(lightSwitch));

console.log("lightSwitch.isOpen : ", lightSwitch.isOpen);
console.log("lightSwitch.close() : ", lightSwitch.close());
console.log("lightSwitch.isOpen : ", lightSwitch.isOpen);
console.log("lightSwitch.open() : ", lightSwitch.open());
console.log("lightSwitch.isOpen : ", lightSwitch.isOpen);

console.log("lightSwitch.color : ", lightSwitch.color);
console.log("lightSwitch.changeColor('pink') : ", lightSwitch.changeColor('pink'));
console.log("lightSwitch.color : ", lightSwitch.color);
console.log("lightSwitch.changeColor('light-blue') : ", lightSwitch.changeColor('light-blue'));
console.log("lightSwitch.color : ", lightSwitch.color);
.as-console-wrapper { max-height: 100%!important; top: 0; }

The very same approach then repeatedly could be applied to // Product A with its LedSwitch example and finally to both examples of // Product B that are AlarmSwitch and ProductBLightSwitch with both being the real target of the OP's question. The final result then might look like that ...

// Common components

var
  withSerializableTypeProxy = (function () {
    function toString(type) {
      return JSON.stringify(type);
    }
    function valueOf(type) {
      return JSON.parse(JSON.stringify(type))
    }
    let defineProperty = Object.defineProperty;

    return function trait (stateValue) {
      const
        compositeType = this;

      defineProperty(compositeType, 'toString', {
        value: (() => toString(stateValue))
      });
      defineProperty(compositeType, 'valueOf', {
        value: (() => valueOf(stateValue))
      });
    };
  }());


function withChangeableColorProxy(stateValue) {
  const
    compositeType = this;

  compositeType.changeColor = ((colorValue) => stateValue.color = colorValue);

  Object.defineProperty(compositeType, 'color', {
    enumerable: true,
    get: (() => stateValue.color)
  });
  stateValue.color = 'white'; // set default value.
}


class Switch {
  constructor(stateValue) {

    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};
    stateValue.isOpen = true;

    const
      switchType = this;

    withSerializableTypeProxy.call(switchType, stateValue);

    switchType.open = (() => stateValue.isOpen = true);
    switchType.close = (() => stateValue.isOpen = false);

    Object.defineProperty(this, 'isOpen', {
      enumerable: true,
      get: (() => stateValue.isOpen)
    });
  }
}


class LightSwitch extends Switch {
  constructor(stateValue) {
    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};

    super(stateValue);

    withChangeableColorProxy.call(this, stateValue);
  }
}


// Product A

var
  withBlinkBehaviorProxy = (function () {
    function setBlinkValue(stateValue, blinkValue) {
      return stateValue.blink = blinkValue;
    }

    return function trait (stateValue) {
      const
        compositeType = this;

      Object.defineProperty(compositeType, 'blink', {
        enumerable: true,
        get: (() => stateValue.blink),
        set: ((blinkValue) => setBlinkValue(stateValue, blinkValue))
      });
      setBlinkValue(stateValue, false); // set default value.
    };
  }());


class LedSwitch extends LightSwitch {
  constructor(stateValue) {
    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};

    super(stateValue);

    withBlinkBehaviorProxy.call(this, stateValue);
  }
}


var
  ledSwitch = (new LedSwitch);

console.log("ledSwitch : ", ledSwitch);

console.log("(ledSwitch instanceof LedSwitch) ? ", (ledSwitch instanceof LedSwitch));
console.log("(ledSwitch instanceof LightSwitch) ? ", (ledSwitch instanceof LightSwitch));
console.log("(ledSwitch instanceof Switch) ? ", (ledSwitch instanceof Switch));

console.log("ledSwitch.valueOf() : ", ledSwitch.valueOf());
console.log("Object.keys(ledSwitch) : ", Object.keys(ledSwitch));

console.log("(ledSwitch.blink = 'blink') : ", (ledSwitch.blink = 'blink'));
console.log("ledSwitch.blink : ", ledSwitch.blink);
console.log("(ledSwitch.blink = true) : ", (ledSwitch.blink = true));
console.log("ledSwitch.blink : ", ledSwitch.blink);


// Product B
// AlarmSwitch and ProductBLightSwitch both need a "timer"

function withPlayerBehaviorProxy(stateValue) {
  const
    compositeType = this;

  compositeType.changeSound = ((soundValue) => stateValue.sound = soundValue);

  Object.defineProperty(compositeType, 'sound', {
    enumerable: true,
    get: (() => stateValue.sound)
  });
  stateValue.sound = 'alarm.wav'; // set default value.
}

function withTimerBehaviorProxy(stateValue) {
  const
    compositeType = this;

  compositeType.resetTimer = (() => stateValue.timer = 0);

  Object.defineProperty(compositeType, 'timer', {
    enumerable: true,
    get: (() => stateValue.timer)
  });
  stateValue.timer = 5000; // set default value.
}


class AlarmSwitch extends Switch {
  constructor(stateValue) {
    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};

    super(stateValue);

    withPlayerBehaviorProxy.call(this, stateValue);
    withTimerBehaviorProxy.call(this, stateValue);
  }
}

class ProductBLightSwitch extends LightSwitch {
  constructor(stateValue) {
    stateValue = ((stateValue != null) && (typeof stateValue == 'object') && stateValue) || {};

    super(stateValue);

    withTimerBehaviorProxy.call(this, stateValue);
  }
}


var
  lightSwitch_B = (new ProductBLightSwitch),
  alarmSwitch = (new AlarmSwitch);

console.log("lightSwitch_B : ", lightSwitch_B);
console.log("alarmSwitch : ", alarmSwitch);

console.log("(lightSwitch_B instanceof ProductBLightSwitch) ? ", (lightSwitch_B instanceof ProductBLightSwitch));
console.log("(alarmSwitch instanceof AlarmSwitch) ? ", (alarmSwitch instanceof AlarmSwitch));

console.log("(lightSwitch_B instanceof LightSwitch) ? ", (lightSwitch_B instanceof LightSwitch));
console.log("(alarmSwitch instanceof LightSwitch) ? ", (alarmSwitch instanceof LightSwitch));

console.log("(lightSwitch_B instanceof Switch) ? ", (lightSwitch_B instanceof Switch));
console.log("(alarmSwitch instanceof Switch) ? ", (alarmSwitch instanceof Switch));

console.log("lightSwitch_B.valueOf() : ", lightSwitch_B.valueOf());
console.log("alarmSwitch.valueOf() : ", alarmSwitch.valueOf());

console.log("Object.keys(lightSwitch_B) : ", Object.keys(lightSwitch_B));
console.log("Object.keys(alarmSwitch) : ", Object.keys(alarmSwitch));

console.log("lightSwitch_B.resetTimer() : ", lightSwitch_B.resetTimer());
console.log("lightSwitch_B.timer : ", lightSwitch_B.timer);

console.log("alarmSwitch.changeSound('ringtone.wav') : ", alarmSwitch.changeSound('ringtone.wav'));
console.log("alarmSwitch.sound : ", alarmSwitch.sound);
.as-console-wrapper { max-height: 100%!important; top: 0; }
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37