31

Is It good/bad practice to call a child method from a parent class?

class Parent {
    constructor() {
        // if 'autoPlay' exists (was implemented) in chain
        if (this.autoPlay) {
            this.autoPlay(); // execute from parent
        }
    }
}

class ChildA extends Parent {
    autoPlay() {
        console.log('Child');
    }
}

class ChildB extends Parent {
    // 'autoPlay' wasn't implemented
}

const childA = new ChildA();
const childB = new ChildB();
Ali Mamedov
  • 5,116
  • 3
  • 33
  • 47
  • Yes, the parent class has no contract with the child class, it's the other way around. Besides it wouldn't make sense, just move the method into the parent class. – Lucas Steffen Dec 14 '17 at 18:38
  • @LucasSteffen, every class which extends 'Parent' has Its own 'autoPlay' implementation. So It is impossible to move that method to the parent class – Ali Mamedov Dec 14 '17 at 18:41
  • @AliMamedov Is it required to be defined by every child? In that case you should implement it in the parent but throw an error. Something like `autoPlay ( ) { throw new Error( 'autoPlay must be implemented' ); }`. – Paul Dec 14 '17 at 18:42
  • It's in general not a good idea to call overridden methods *from the constructor*, as you get into a bunch of initialisation order problems. Best practice is to not call any methods from the constructor but only do the instance initialisation. Any side effects like `autoplay` should be invoked by the caller who created the instance. (Otherwise, yes it's totally fine to call overridden methods like that, though you usually don't even need that "if defined" check) – Bergi Dec 14 '17 at 18:42
  • @Paulpro, yes you are right. Every child class will have Its own 'autoPlay' implementation. But in some cases 'autoPlay' implementation can be skipped – Ali Mamedov Dec 14 '17 at 18:45
  • @Bergi, can you provide a small sample? – Ali Mamedov Dec 14 '17 at 18:46
  • Possible duplicate of https://stackoverflow.com/questions/47453121/es6-access-to-inherited-classes-properties-and-methods-from-the-parent-class . – Estus Flask Dec 14 '17 at 18:48
  • 1
    Child class should have its own `constructor`. No, it's not a good practice, unless the classes follow some interface, like `init` method. – Estus Flask Dec 14 '17 at 18:49

2 Answers2

29

Is it a good practice to call a child method from a parent class?

Yes, it's a totally normal practise. The parent class just calls some method of the instance, and if the child class has overridden the method then the child method is called. However, you usually wouldn't do such a "has my instance defined this method" test, you just would call it. If you want to do nothing by default, just define an empty method (like in @scipper's answer). If you want to make the method abstract (force child classes to override it), you can either leave it undefined or define a method that throws an appropriate exception.

Is is a bad practice to call a child method from a parent constructor?

Yes. Don't do that. (It's a problem in all languages).

The purpose of a constructor is to initialise the instance and nothing else. Leave the invocations of side effects to the caller. This will ensure that all child constructors will finish their initialisation as well.

A contrived example:

class Parent {
    autoPlay() {
        this.play("automatically "); // call child method
    }
    play(x) {
        console.log(x+"playing default from "+this.constructor.name);
    }
}

class ChildA extends Parent {
    // does not override play
}
class ChildB extends Parent {
    constructor(song) {
        super();
        this.song = song;
    }
    play(x) {
        console.log(x+"playing "+this.song+" from ChildB");
    }
}

const child1 = new ChildA();
child1.autoPlay();
const child2 = new ChildB("'Yeah'");
child2.autoPlay();

Notice how that would not work if the Parent constructor did call autoplay. If you don't like to need an extra method call everywhere after the instantiation, use a helper function. It might even be a static method:

class Parent {
    autoPlay() { … }
    play { … }
    static createAndAutoPlay(...args) {
        const instance = new this(...args);
        instance.autoPlay();
        return instance;
    }
}
…
const child1 = ChildA.createAndAutoPlay();
const child2 = ChildB.createAndAutoPlay("'Yeah'");
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Blast -- so if I say, had a "getDefaultSettings()" method that I wanted to be able to override, I wouldn't be able to call that from the abstract class constructor to populate settings appropriately? (makes sense, just making sure I fully grok) – slifty Apr 29 '20 at 22:15
  • 1
    @slifty `getDefaultSettings` sounds like it could be a static method, then it should work. It just cannot rely on a fully initialised instance. – Bergi Apr 30 '20 at 06:34
  • thanks - nice and easy to understand - I knew you could do this in some languages but I didn't know JS offered it :) – danday74 Oct 02 '21 at 01:12
  • Just wanted to elaborate a little on the note, that calling a virtual method from base class ctor is a problem in _all_ languages. In some languages, e.g. C++, calling overridden child implementation from base class ctor is _not even possible_, i.e. the base class implementation will be called instead. On the other side, in Java, C#, and JavaScript it's technically possible to call child class implementation from the base class ctor, but it's still _not recommended_ due to the reasons described in this and the linked answer. – Alex Che Feb 15 '22 at 10:22
14

It would be better style to define an empty implementation of autoPlay in the Parent class, and override it in the child.

class Parent {
  constructor() {
    this.autoPlay();
  }

  autoPlay() {

  }
}

class Child extends Parent {
  autoPlay() {
    console.log('Child');
  }
}

const child = new Child();
scipper
  • 2,944
  • 3
  • 22
  • 45
  • Well, it's still bad practice.. maybe that's why. – scipper Dec 14 '17 at 18:44
  • Or it's the hat `¯\_(ツ)_/¯` – scipper Dec 14 '17 at 18:46
  • 2
    In this case, `Parent` is referred to as an abstract class definition (abstract because it may not be useful just instantiated by itself). It provides a common definition/interface of methods, but not the implementation of all of them and expects the derived class to finish the implementation. This is a normal object oriented design technique that allows multiple derived classes to share a bunch of implementation in the parent and in some cases, the parent itself is never used by itself. It is a separate issue whether `autoPlay()` should be called from the constructor or not. – jfriend00 Dec 14 '17 at 22:15