Overriding issue

Monkey Forums/Monkey Programming/Overriding issue

Leo Santos

(Posted 2015) [#1]

Consider this simple example:

Class Vector

	Field x:Float = 0.0
	Field y:Float = 0.0

	Method Copy:Void( other:Vector )
		Self.x = other.x
		Self.y = other.y
	End

End


Class Rect Extends Vector

	Field width:Float = 10.0
	Field height:Float = 20.0

	Method Copy:Void( other:Rect )
		Self.x = other.x
		Self.y = other.y
		Self.width = other.width
		Self.height = other.height
	End

End

Function Main()
	Local test := New Rect
	Local test2 := New Rect
	test2.width = 100
	test2.height = 50
	test.Copy( test2 )
	Print( test.width + "," + test.height )
End

This doesn't work, since I get a "Overriding method does not match any overridden method" error.

How do I manage to extend a class but replace the original method with one that uses the extended type? Can I do this with generics, somehow?

Thanks!

ziggy

(Posted 2015) [#2]

It is not workig because it would violate the liskov substitution principle ( wiki here ). You could leave the declaration as it is, as long as Rect extends Vector and cast to Rect inside the child extended class.

ImmutableOctet(SKNG)

(Posted 2015) [#3]

@Ziggy: I think you misunderstood Leo's example, or maybe I misunderstood it, either way, my comment still describes how you'd get around this:

The Liskov principle has to do with expected behavior when dealing with polymorphism. What Leo Santos seems to want is custom method overloads akin to what can be achieved in other languages (C++ for example). Monkey has had this limitation for years, and if I'm not mistaken, Mark's planning on fixing this in Monkey 2. Basically, if the overload could work with another name, or is valid under the context of a super-class, then there's no reason the overload can't exist. It's a convenience thing. You don't want to use a different name when there's no conflict.

That being said, you should NEVER expect to be able to make an overload that overrides a previous overload, without the exact same prototype. If you wish to call another overload/implementation, this can be done normally from within your method. The only way to add specialization from top-down (Using the base-class, but expecting behavior defined in the derived class) is to do what Ziggy suggests. That's considered bad practice in most situations, even when a virtual override is involved. Dynamic casting and derived-type specialization should be avoided in base classes.

ziggy

(Posted 2015) [#4]

According to the principle, all child classes should implement the same contract to parent classes. They may add additional memebers to the contract, but not replace them

example of how this is a violation:

Function DoSomethingWithVector(vec:Vector)
     Vec.Copy(New Vector())   'Vec can be a Rectangle instance, so copy may expect a Rectangle instead of a Vector, so liskov is not preserved, as this only works for the parent class.
End
Function Main()
    DoSomethingWithVector(New Rectangle)
End

This would crash as we're injecting a vector as a parameter in the Copy override of an actual Rectangle that expects a Rectangle.

This is contravariance violation, and contravariant positions, on any SOLID OOD shoud only be acceptable in resulting types (this keeps type safenes). At the same time, covariance positions (input parameters in extending class members) are safe as long as you do it otherwise. You can extend a method with an override of this method that adds a less specialized object, but not the other way. This is very well explained here: http://www.stephanboyer.com/post/39/covariance-and-contravariance

Simpler example:

Local v:Vector = New Rectangle
v.Copy(New Vector)

Danilo

(Posted 2015) [#5]

It's like function overloading (different argument types / different arguments), so if

Function Copy:Void( other:Vector )
Function Copy:Void( other:Rect )

works, it should also work with method overloading / overriding.

I think it's a perfectly valid example to want to have a 'copy' method within different classes, in a big class hierarchy.

Method Copy:Void( other:Vector )

expects a Vector (or any derived class), and copies the Vector parts.

Method Copy:Void( other:Rectangle )

expects at least a Rectangle (or any derived class), and copies the Rectangle parts.

ziggy

(Posted 2015) [#6]

@Danilo:But that's a bit unelegant if a Vector is a Rectangle. Maybe they should not be parent and child, but both implement the same interface with a generic. That would be much more elegant if you ask me.

Leo Santos

(Posted 2015) [#7]

as long as Rect extends Vector and cast to Rect inside the child extended class

That works! This runs fine:

Class Vector

	Field x:Float = 0.0
	Field y:Float = 0.0

	Method Copy:Void( other:Vector )
		Self.x = other.x
		Self.y = other.y
	End

End


Class Rect Extends Vector

	Field width:Float = 10.0
	Field height:Float = 20.0

	Method Copy:Void( other:Vector )
		Self.x = other.x
		Self.y = other.y
		If Rect(other)
			Self.width = Rect(other).width
			Self.height = Rect(other).height
		End
	End

End

Function Main()

	Local test := New Rect
	Local testRect := New Rect
	Local testVector := New Vector
	
	testRect.x = 3
	testRect.y = 2.5
	testRect.width = 100
	testRect.height = 50
	
	test.Copy( testRect )
	Print( test.x + "," + test.y + ",  " + test.width + "," + test.height )
	
	test.Copy( testVector )
	Print( test.x + "," + test.y + ",  " + test.width + "," + test.height )
	
End

As a side note, does casting things like that have a noticeable impact in performance? This will go inside a collision routine, and will possibly be called lots of times per frame (which is why I'm copying the values from a single global vector, instead of creating new Rects, to prevent garbage) and I'd like to keep this as fast as possible.

If there is indeed a performance hit, then maybe Danilo's solution of simply adding the two overloads in the base class would work too, although now the design wouldn't be so clean, since the Vector class, which previously had no dependencies other than monkey.math, now would have to import the Rect class (in my "real" code those classes are separate modules).

Thanks!!!

Danilo

(Posted 2015) [#8]

@ziggy:
> But that's a bit unelegant if a Vector is a Rectangle.

A Vector can't be a Rectangle, because a Vector is lower in the hierarchy (Vector does not know about it's descendants).
A Rectangle can be a Vector, because it is inherited from Vector (Vector is an ancestors of Rectangle).

ziggy

(Posted 2015) [#9]

@Danilo: yes, I ment a Rectangle is a vector in the previous example.

Danilo

(Posted 2015) [#10]

@ziggy:
If Vector contains the genetic material x:Float/y:Float, and Rectangle inherits everything
from Vector (x,y) and just adds new genetic material (width:Float, height:Float), than
Vector contains only x,y - but a Rectangle contains x,y,width,height.
Rectangle is compatible to Vector using the x/y part, and it extends Vector using the width/height part.
Vector knows only about x/y, and not about the extended width/height parts of its descendant Rectangle.

ziggy

(Posted 2015) [#11]

Rectangle is compatible with Vector, but Vector is not compatible with rectangle if it's derived Copy method expects a more specialiced type (Rectangle). So, they're not interchangeable, so they do not follow liskov nor they are SOLID.

The same example shows it:

Function DoSomething(variable:Vector)
'Here variable can be a rectangle or a vector or anything inheriting vector
Local newItem:Vector
variable.Copy(newItem) 'If variable is a Rectangle, it may fail.
End

In this scenario, when variable is a rectangle, it will try to access the with and height of the newItem vector, which does not have them. Then you can do casting magic into Rectangle class and make a single method (copy) have a per-parameter-type functionality, which si not exactly SOLID.

And the overloading idea. Well, if you now inherit Rectangle and use the Copy method, which version should be called? The one that expects a Vector or the one that expects a Rectangle? Because that new class would be a Vector and a Rectangle and then it would have 2 behavoiors for the same funcionality. It wuold be much more of method shadowing than overloading, which is not very nice

ImmutableOctet(SKNG)

(Posted 2015) [#12]

@Ziggy:

I don't think we're talking about the same thing. The custom overload in the derived class is/should only going to be called from the derived class. The only way to break the Liskov principle here is to override methods. Either way, that principle isn't necessarily relevant here. The original intent seemed to be having a way to copy another object's contents, which was at the same or higher level of the hierarchy. In other words, 'Vector' only handles 'Vectors', and 'Rectangle' only (Implicitly) handles 'Rectangles'. I mentioned how C++ handles this before. In C++11 and beyond, you're encouraged to state that a method is an override to an implementation. This only applies to virtual methods, which can be polymorphic (Mutated by derived classes). For custom methods which do not actually override an existing method-prototype, using 'override' is invalid. This was part of the reasoning behind adding that keyword.

But that's just it, though, you can create whatever custom overloads you want in C++. That's because it's a compile-time problem I'm speaking of. It's not a polymorphic problem (Unless you add extra complexity by making the other overloads virtual). A specific quirk to C++ is making super-class overloads opt-in (Multiple inheritance is likely behind the reasoning). Basically, what Monkey does currently, but only when there's new overloads in the derived class. Compared to Monkey, where any override to one of your super-class's overloads will result in complete encapsulation until stated otherwise. This would be fine, but then you have the issue of not having your own overload choices in derived classes.

Leo Santos's second example does show what you were referring to, but in the context he's doing that, it wouldn't need the runtime overhead if he had chosen a different name for 'Copy' in the derived class, or we had better overloading behavior.

@Leo Santos:
That's not exactly the best option. You're using runtime type evaluation, meaning it's going to have some overhead. It's not too serious, but if it's in a collision routine, you have better options. I suggest either a different name (If possible), or giving in, and placing the overload in the base class. It really depends on how you're using 'Copy'. If you need it to handle the types without knowing them, then you're going to have to use a dynamic cast. Generally speaking, you're supposed to avoid that kind of situation. Dynamic casts aren't free.

Also, your particular example should cache the cast on the stack, so you're not doing it 3+ times. Something like this:

Local R:= Rect(other)

If (R <> Null) Then
	Self.width = R.width
	Self.height = R.height
End

Again, not the best option if you can avoid it. Also, you unfortunately can't use generics to make the method-overload problem any better, because:
A): You couldn't use 'Vector' normally without an interface anyway.
And B): You can't (Currently) reference the current class when inheriting. (Mark's supposedly going to fix this, too)

Leo Santos

(Posted 2015) [#13]

@ImmutableOctet(SKNG)
Thanks, that's really useful!

In HTML5 it's currently taking around 5ms to check a little over 250000 collisions (which includes storing collision data, calculating collision normal, sending appropriate events, etc), which seems pretty good, but in reality my computer is really fast so it's not a good benchmark. I'll try optimizing those methods and see how much more I get, then I'll try it on my old 2008 Mac Mini! :-)

Also, I just saw that you have a Vector module that is waaaaaaaay more advanced than what I'm using, I wish I had checked it earlier!

ziggy

(Posted 2015) [#14]

Leo Santos's second example does show what you were referring to, but in the context he's doing that, it wouldn't need the runtime overhead if he had chosen a different name for 'Copy' in the derived class, or we had better overloading behavior.

That's the point, he was making an override to something that shoud have been an overload and even in this case, overloading solving is not easy when there can potentially be Rectangle childs. I agree that choosing a different name would be just fine.

@Leo Santos: One common way of handling collisions is by splitting screen in "logical" rectangles, each rectangle has a list of its contained items so each item only needs to check for collisions against items in the same rectangle. You have to keep logic to keep each item in the appropriate rectangle when they move.

ImmutableOctet(SKNG)

(Posted 2015) [#15]

@Leo Santos: For reference, Ziggy's referring to a quadtree. They're incredibly useful for keeping relational complexity to a minimum. Since you brought up my 'vector' module, I should probably mention I also implemented a quadtree. Nothing complicated, but it works. Considering that module is kind of specialized to the setup I'm using, it's better as a reference than anything else. If I remember right, I followed this post initially.