Subclassing/overloading method with param overloading error

736 views
Skip to first unread message

Peter StJ

unread,
Oct 11, 2012, 7:50:13 AM10/11/12
to closure-comp...@googlegroups.com
I tried really hard to understand the explanation found here about types and subclassing, but evidently I cannot understand things well enough and the compiler complains still, so I am hoping by giving my example someone can tell me what exactly is going on and where is my mistake.

I am constructing a custom button renderer and I want to override the declare method. It works as expected when provided as renderer option to the goog.ui.CustomButton constructor and rendering works fine, code compiles fine.

The problem is I want to force the type checking to make sure the component is indeed goog.ui.CustomButton and not just any goog.ui.Control. So I make the following param declaration:

/** @param {!goog.ui.CustomButton} button ... */

However the compiler says I am incorrectly overrideing the method:

js/client/ui.ButtonRenderer.js:61: WARNING - mismatch of the decorate property type and the type of the property it overrides from superclass goog.ui.ControlRenderer
original: function (this:goog.ui.ButtonRenderer, (goog.ui.Control|null), (Element|null)): (Element|null)
override: function (this:spo.ui.ButtonRenderer, (goog.ui.CustomButton|null), (Element|null)): (Element|null)
spo.ui.ButtonRenderer.prototype.decorate = function(control, element) {

I believe goog.ui.CustomButton is a subclass of goog.ui.Control. Also spo.ui.ButtonRenderer is a subclass of goog.ui.ButtonRenderer. What is the rule when overloading a method, the parameter should be the same, of the parameter should be a superclass of the original method or a subclass of the original method? I don't really get it at this point. The original method knows how to handle goog.ui.Control, goog.ui.CustomButton is subclass of it so shouldn't my class follow the pattern but refine what it can handle? Can this be done, is it correct?

Hopefully someone can explain where I am mistaking.

Ilia Mirkin

unread,
Oct 11, 2012, 7:56:20 AM10/11/12
to closure-comp...@googlegroups.com
https://groups.google.com/forum/?fromgroups=#!topic/closure-compiler-discuss/uE1DHLqcVu0

Long story short, if you're extending a class, you can't change the
type signature of the functions you're overriding.

John Lenz

unread,
Oct 11, 2012, 10:37:44 AM10/11/12
to closure-comp...@googlegroups.com

You can but it must be the same or more accepting in its parameters and the same or more restrictive in its return type.  There was a nice explaination in the closure tools blog.

Peter StJ

unread,
Oct 11, 2012, 11:54:42 AM10/11/12
to closure-comp...@googlegroups.com, imi...@alum.mit.edu
Okay, this was kind of expected, what I want to understand is why is this. Subclassing is (IMO) way to speciallize functionality, So specialized functionality might need more specialized input data, doesn't that make sense?

Eric Williams

unread,
Oct 11, 2012, 2:50:04 PM10/11/12
to closure-comp...@googlegroups.com, imi...@alum.mit.edu
On Oct 11, 2012, at 10:54 AM, Peter StJ <regard...@gmail.com> wrote:

Okay, this was kind of expected, what I want to understand is why is this. Subclassing is (IMO) way to speciallize functionality, So specialized functionality might need more specialized input data, doesn't that make sense?

Subclasses have to be substitutable for the base class, that is, you need to be able to use the subclass anywhere the base class could be used. If this isn't enforced, it breaks type safety.

Guido Tapia

unread,
Oct 11, 2012, 3:20:47 PM10/11/12
to closure-comp...@googlegroups.com, imi...@alum.mit.edu
+1 Eric, which is why John's comment concerns me, this is clearly a bug (that the compiler allows more args).  The compiler cannot stop a developer from violating their interfaces i.e.:
mycontrol.decorate = function(xx) { if (!(xx instanceof MyExpectedType)) throw new Error() }

However, the compiler should definitely not encourage this and accepting more parameters is definitely encouraging the violation of the interface contract.

Not, that its a big issue, I mean its JavaScript afterall, we can violate anything anytime.  However, I would like to see the compiler respect Liskov a bit more.

John Lenz

unread,
Oct 11, 2012, 3:37:01 PM10/11/12
to closure-comp...@googlegroups.com, imi...@alum.mit.edu
Here is the blog post I spoke about:
On Thu, Oct 11, 2012 at 12:20 PM, Guido Tapia <guido...@gmail.com> wrote:
+1 Eric, which is why John's comment concerns me, this is clearly a bug (that the compiler allows more args).  The compiler cannot stop a developer from violating their interfaces i.e.:
mycontrol.decorate = function(xx) { if (!(xx instanceof MyExpectedType)) throw new Error() }

I didn't specifically say anything about adding arguments. You can't require more arguments.  That would break the contract.  You could however add optional arguments and still meet the requirements of the super class function.  That would still be substitutable.

Peter StJ

unread,
Oct 12, 2012, 2:04:27 AM10/12/12
to closure-comp...@googlegroups.com, imi...@alum.mit.edu
So basically, the only way to avoid a team member accidentally using the custom renderer is to subclass the expected optional renderer in the CustomButton annotation and hope that noone will be temted to use it directly (i.e. instead of this.renderer... call an instance directly like ui.CustomRenderer.getInstance(). And I should throw an error if this happens but this will be seen only at runtime then.. I wish I could have done it in such a way as to make the compiler notify the developer beforehand. Is there such a way?
Reply all
Reply to author
Forward
0 new messages