-1

I was dabbling with singletons and extending classes and came up with this 'working' code.

let instance = null;

class Motorcycle {
    constructor(engine, color) {
        if(!instance) {
            this.engine = engine;
            this.color = color;
            instance = this;
        } else {
            return instance;
        }

    }
}

class Car extends Motorcycle {
    constructor(engine, color) {
        if(!instance) {
            super(engine, color);
            this.doors = 4;
        } else {
            return instance;
        }
    }
}

class SUV extends Car {
    constructor(engine, color, doors) {
        if(!instance) {
            super(engine,color);
            instance.doors = doors == undefined ? 5 : doors;
        } else {
            return instance;
        }
    }
}

I realized that in the constructor for sub-classes I can set doors with instance.doors and this.doors.

So I have two questions:

  1. Does it matter enough to have a best practice?
  2. What is that best practice?
CryptoRAT
  • 11
  • 5
  • 1
    At some point you might want to have two cars without having to refactor the whole thing. Concerning 2.: Singletons are a bad practice, use objects. – Jonas Wilms Jul 20 '20 at 20:06
  • I tested it trivially, but what this does is only the first one gets created. So if you create an SUV first, that is the instance. So there shouldn't be two singletons. Just two different options for what that singleton might be. Another problem, but this was just for fun. – CryptoRAT Jul 20 '20 at 20:09
  • Your `Car` and `Motorcycle` should extend or "implement" (not really thing in vanilla JS) a `Vehicle` class. – Mr. Polywhirl Jul 20 '20 at 20:10
  • What compels this in the first place? For the extra complexity, ,what were you hoping to gain? –  Jul 20 '20 at 20:13
  • Thank you both for your thoughts. I know the problem statement is ridiculous. I was playing with the edge of my knowledge. I just wanted to know what it would look like to mix singletons in a class hierarchy and made one up. – CryptoRAT Jul 20 '20 at 20:21

2 Answers2

0

Does it matter enough to have a best practice?

Not in your case probably, but there is a difference between them.

What is that best practice?

Do not use singletons. Especially not when subclassing. Your code is unable to create an instance of a separate subclass, is unable to create multiple cars, is unable to create a new SUV after creating a new Car (and also a Car is not a MotorCycle).

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thank you. It hadn't occurred to me that one is an object literal. – CryptoRAT Jul 20 '20 at 20:22
  • @CryptoRAT No, it's not an object literal, but the question of using `this` vs `instance` to refer to the object is exactly the same. – Bergi Jul 20 '20 at 20:39
  • My understanding from the article is that ```instance``` is the object literal in the global scope of this tiny code fragment. ```this``` in another context, not my example code, is so much more. That is why for my problem it doesn't mean much. – CryptoRAT Jul 20 '20 at 20:51
0

Your singleton objects should exist outside the constructor; preferably inside a map and accessed by their class name. Constructors should really only call their super constructor or set instance variables.

You will also need a way to dynamically bind constructor arguments, this is taken care of in the ctorBind function below.

You could also call getSingleton(SUV) a second time without args. This example just shows that the original SUV was not modified upon returning.

// REFLECTION UTILS
const getClassName = (obj) => obj.toString().match(/ (\w+)/)[1]
const ctorBind = (ctor, args) => new (ctor.bind.apply(ctor, [null].concat(args)))()

// MAIN FUNCTION
const main = () => {
  console.log(getSingleton(SUV, 'V6', 'Red', 5))
  console.log(getSingleton(SUV, 'V8', 'Blue', 6)) // Not updated...
}

// BEGIN SINGLETON MAP AND LOOKUP
const vehicleSingletons = {};
function getSingleton(vehicleClass, ...args) {
  const className = getClassName(vehicleClass)
  if (vehicleSingletons[className] == null) {
    vehicleSingletons[className] = ctorBind(vehicleClass, args);
  }
  return vehicleSingletons[className];
}

// BEGIN CLASS DEFINITIONS
class Vehicle {
  constructor(engine, color) {
    this.engine = engine;
    this.color = color;
  }
}

class Motorcycle extends Vehicle {
  constructor(engine, color) {
    super(engine, color);
  }
}

class Car extends Vehicle {
  constructor(engine, color, doors) {
    super(engine, color);
    this.doors = doors;
  }
}

class SUV extends Car {
  constructor(engine, color, doors) {
    super(engine, color, doors);
  }
}

main()
.as-console-wrapper { top: 0; max-height: 100% !important; }
Mr. Polywhirl
  • 42,981
  • 12
  • 84
  • 132
  • `getClassName(vehicleClass)` should be just `vehicleClass.name`, and `ctorBind(vehicleClass, args)` should be just `new vehicleClass(...args)`. (Btw it doesn't *bind* the constructor, it just calls it) – Bergi Jul 20 '20 at 20:42
  • You are getting a singleton each vehicle type. I was exploring, for fun, how to get only one vehicle type. So if you create an SUV first then it can not become a Car or Motorcycle. I'm sorry I have no logical scenario to which to apply it, I just followed my curiosity. – CryptoRAT Jul 20 '20 at 21:01