Say I defined a method's implementation in an abstract/parent class then created a derived class that's expected to use this method. If the derived class doesn't override the method's implementation, is it good practice to still define it and call super for the sake of clarity for someone reading the code?

class Foo{
    speak() { console.log('Hi') }
}

class Bar extends Foo {
  // Should I include this? :
  speak(){
         super.speak()
  }
}

The only benefit this would have is readability and asserting that a Bar is expected to speak(), but it's also redundant since it doesn't add anything in terms of implementation. So I'm wondering what the best practice is when declaring a derived class?

CodePudding user response：

There's no point to doing that, extends Foo is clear that Bar may be inheriting features from Foo, and moreover it introduces a maintenance hazards:

• Suppose you decide that speak should return a value and edit Foo accordingly? Then Bar.speak is wrong, because it doesn't return the result of calling super.speak().
• Suppose you add a method to Foo? Now, if you were to follow the rule that you should redefine it in subclasses, you have to go find every subclass to add a new method — inevitably missing some.

There's also the question of efficiency: you're introducing another layer in the call stack for no reason.

While there's often room for opinions in these things, I think the consensus on this is universal: Don't override methods unless you need to customize the method's behavior.

CodePudding user response：

Personally, I believe that, if the implementation is the same, there is no point in declaring it again as in your question. For me, it does not make the code more readable as it's just an extra block of code without purpose.

I think you should only declare methods with different implementations than their superclass or no implementation at all that throw NotImplemented.

*EDIT: As another answer mentions, overriding methods without any real purpose can create maintenance issues, which should be avoided in general.

CodePudding user response：

Which one is more readable?

When inheritance is used as it should?

class A {
    one(a, b) {
      //implementation
    }
}

class B extends A {
    two(e, f, g) {
        //implementation
    }
}

class C extends B {
    three(l, m, n) {
        //implementation
    }
}

class D extends C {
    four(p, q) {
        //implementation
    }
}

class E extends D {
    five(x, y, z) {
        //implementation
    }
}

Or when it is not?

class A {
    one(a, b) {
      //implementation
    }
}

class B extends A {
    one(a, b) {
      return super.one(a, b);
    }
    two(e, f, g) {
        //implementation
    }
}

class C extends B {
    one(a, b) {
      return super.one(a, b);
    }
    two(e, f, g) {
      super.two(e, f, g);
    }
    three(l, m, n) {
        //implementation
    }
}

class D extends C {
    one(a, b) {
      return super.one(a, b);
    }
    two(e, f, g) {
      return super.two(e, f, g);
    }
    three(l, n, m) {
      return super.three(l, m, n);
    }
    four(p, q) {
      //implementation
    }
}

class E extends D {
    one(a, b) {
      return super.one(a, b);
    }
    two(e, f, g) {
      return super.two(e, f, g);
    }
    three(l, m, n) {
      return super.three(l, m, n);
    }
    four(p, q) {
      return super.three(p, q);
    }
    five (x, y, z) {
      //implementation
    }
}

Perhaps I jumped the gun a bit with the description of the code blocks but here is another question: did you spot the error? While the second block redeclares the methods, one is wrong. In C the method two does not return the value of super.two(). Which means that the inherited method is not the same as the parent one. Even if right now B#two() (the inherited method) does not return a value, it might in the future. Which is the worse scenario.

The suggested approach of re-declaring all methods leads to a lot of useless boilerplate. The above example might be an outlier (inheritance tends to be rather shallow in most cases) but it is not impossible to find in real world code bases. But even a shallower hierarchy of only 3 level inheritance (much more common) would be vulnerable to the same code bloat as this.

The repeated code makes it much harder to read and understand what is happening. Moreover, it imposes hefty maintenance price. Even if there are no bugs now, how do you change any method that is up the inheritance chain? You need to also change all the children downstream. Imagine needing to add a third parameter to one() in A - you need to modify five places that simply redeclare it.

There is a sort of workaround if a method is declared to accept varargs and then forwards them. But that also has drawbacks. First off, here is how it will look:

foo(...args) {
  return super.foo(...args);
}

this method will work the same as the parent one. If called with three arguments, it will forward all three. If called with four, it will forward four. And always returns the same as what the super method returns. With that said, it is not more readable. Consider named parameters:

foo(stockId, newPrice, validFrom) {
  //some implementation
}

Just seeing the names of the parameters makes it much clearer how to use this method. The variadic declaration using rest parameters obscures that meaning. And re-typing the parameters every time is not much better - then you get the same problem with refactoring up the chain. Changing a name to be more descriptive, e.g., validFrom to validFromISODate still requires updating multiple places.

Re-declaring methods in each child is extremely inefficient. It is a problem for maintenance, it has a high risk of introducing subtle bugs and it fails at the goal of introducing clarity.

If what I wrote so far has not been convincing enough, did you spot the second error in my example? The D class swaps two parameters in an inherited method. Or is it an error at all? Is that perhaps correct and intentional? We cannot actually say because all methods are redeclared anyway. Might have been a typo, might have been a refactoring problem, might have been intentional. Hard to figure out the intention. Re-declaration definitely does not help with the clarity.