Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support isSubclassOf / isSuperclassOf / isAncestorClassOf / isDescendantClassOf #3416

Open
aspiers opened this issue Jan 12, 2018 · 16 comments
Labels
comp: class library SC class library

Comments

@aspiers
Copy link
Contributor

aspiers commented Jan 12, 2018

We already have isKindOf and isMemberOf for comparing an instance with a potential superclass, but we should also be able to cleanly compare a class with a potential superclass or subclass, e.g. something like these (untested):

isSubclassOf(class) { ^this.superclass == class }
isSuperclassOf(class) { ^class.superclass == this }
isAncestorClassOf(class) { ^class.superclasses.includes(this) }
isDescendantClassOf(class) { ^this.superclasses.includes(class) }

except without breaking when fed Object as one of the two inputs like the last two of these would, because Object.superclasses is nil. Similar breakage would occur if the implementations instead used subclasses and allSubclasses.

@telephon
Copy link
Member

telephon commented Jan 13, 2018

In most cases, it is not a good style to write code based on inheritance, because it limits polymorphism (It's generally better to ask respondsTo, if such a thing is needed).

But in principle, this is a reasonable suggestion, as long as we don't encourage its use too much.

@muellmusik
Copy link
Contributor

Some semantics are slightly vague. By isSubClassOf you mean a direct subclass of. Technically anything in a class's subclass tree is a subclass.

That aside, isDescendantClassOf is the same as isKindOf, just slower I think as it's not a primitive. isAncestorClassOf is the same as isKindOf with the receiver and argument swapped, which is pretty trivial to do.

Checking for immediate sub/superclass is different then isKindOf, though I can't imagine that distinguishing between immediate and somewhere in the tree is something that would be often needed, though maybe you have a use case? As your examples show, the code to do it currently is pretty straightforward anyway, and similar in length.

So while I can see what you want to do with these, for my 2 cents I'm not so keen on these additions, especially given we'd want to discourage their use anyway.

@aspiers
Copy link
Contributor Author

aspiers commented Jan 13, 2018

In most cases, it is not a good style to write code based on inheritance, because it limits polymorphism (It's generally better to ask respondsTo, if such a thing is needed).

But in principle, this is a reasonable suggestion, as long as we don't encourage its use too much.

Sure, I'm aware of that. But this is different; it's not about comparing an instance with a class. Also, there are perfectly valid reasons for needing to do introspection on the class hierarchy. For instance, I submitted this issue because I built a plugin framework, and in my loadPlugin(pluginclass) method I want validate that plugin class is a subclass of the base plugin class.

@aspiers
Copy link
Contributor Author

aspiers commented Jan 13, 2018

Some semantics are slightly vague. By isSubClassOf you mean a direct subclass of.

Correct.

Technically anything in a class's subclass tree is a subclass.

Sure, but if anything this is less vague than the existing Class API; e.g.

ArrayedCollection.subclasses // -> [ class RawArray, class Array ]
ArrayedCollection.allSubclasses.size // -> 12

I chose those 4 method names in an attempt to be consistent with the distinction made by this existing API, but I'm not wedded to the names; they were just examples for illustration.

That aside, isDescendantClassOf is the same as isKindOf

No, it's fundamentally different. The receiver of isDescendantClassOf has to be a Class, whereas the receiver of isKindOf is any instance.

isAncestorClassOf is the same as isKindOf with the receiver and argument swapped, which is pretty trivial to do.

Not true, for the same reason as above. isAncestorClassOf's two inputs must both be classes, whereas only one of isKindOf's inputs has to be.

Checking for immediate sub/superclass is different then isKindOf, though I can't imagine that distinguishing between immediate and somewhere in the tree is something that would be often needed, though maybe you have a use case?

What you're talking about already exists in SuperCollider as isMemberOf, and the documentation for that quite rightly says:

Answer a Boolean whether the receiver is a direct instance of
aClass. Use of this message in code is almost always a design
mistake.

However that is fundamentally different the isSubclassOf and isSuperclassOf I proposed, for the same reasons as above: isKindOf and isMemberOf both expect one of their inputs to be an instance. Try this to see the difference:

Array.new.isKindOf(ArrayedCollection)  // evaluates to true
Array.isKindOf(ArrayedCollection)      // evaluates to false

That's why I italicised "class" in the original report above ;-)

@muellmusik
Copy link
Contributor

What you're talking about already exists in SuperCollider as isMemberOf, and the documentation for that quite rightly says:

No, that tests whether something is a direct instance of class, not whether class is its immediate superclass.

The receiver of isDescendantClassOf has to be a Class, whereas the receiver of isKindOf is any instance.

Ah, you're right of course, sorry! I stand by my point about the existing way of doing this being pretty trivial though.

@aspiers
Copy link
Contributor Author

aspiers commented Jan 13, 2018

Ah, you're right of course, sorry!

No problem ;-)

I stand by my point about the existing way of doing this being pretty trivial though.

But half of my example implementations don't work properly, as explained in the original report ;-) I wrote:

except without breaking when fed Object as one of the two inputs like the last two of these would, because Object.superclasses is nil

(You can't call includes on nil.)

@muellmusik
Copy link
Contributor

But half of my example implementations don't work properly, as explained in the original report ;-) I

A primitive would be best.

Is there any reason why isKindOf shouldn't be modified such that

Float.isKindOf(Number);

returns true? Intuitively I'd think it should.

@muellmusik
Copy link
Contributor

Because of the implementation this does return true:

Float.isKindOf(Meta_Number); 

But the semantics of isKindOf suggest the first example should as well.

@mossheim
Copy link
Contributor

mossheim commented Jan 13, 2018

Is there any reason why isKindOf shouldn't be modified

Yes; it would change the contract of that method AND require a technique similar to isDescendantClassOf in order to disambiguate, because now 5 and the class Float are both "kindsOf" Number. In addition, someone looking at the code won't be immediately able to tell which of those you intend to test.

But the semantics of isKindOf suggest the first example should as well.

isKindOf is a predicate on the type of an object. That is fundamentally different than a predicate on a relation between types.

@mossheim
Copy link
Contributor

I support adding at least the ability to test easily and directly whether one class is in the inheritance tree of another class. The rest I'm not so sure of, since the direct superclass/subclass methods IMO can be expressed more clearly with the actual equality test, and the isAncestorClassOf can be achieved by just flipping the parameters (or am I missing something obvious?).

I don't support adding a blanket discouragement; if the docs are going to take an opinion, they should provide cases where use is justified, and an example of how misuse can be remedied through redesign.

@telephon
Copy link
Member

yes, the concept of parallel object and class (Meta something) hierarchies is fundamental, and better not blurred. The ambiguous English semantics of " is kind of" is really the problem here. This basically goes back hundred years, where the difference between element of a set and part of a set was just discovered.

@telephon
Copy link
Member

telephon commented Jan 13, 2018

I support adding at least the ability to test easily and directly whether one class is in the inheritance tree of another class.

could be aClass.inheritsFrom(anotherClass)

@muellmusik
Copy link
Contributor

The ambiguous English semantics of " is kind of" is really the problem here. This basically goes back hundred years, where the difference between element of a set and part of a set was just discovered.

Well, yes. There is certainly a precedent for methods that respond to different argument types. @brianlheim I'm not sure there's really a 'contract' in the sense that you mean. But probably Julian is right about the usefulness of the distinction.

@muellmusik
Copy link
Contributor

could be aClass.inheritsFrom(anotherClass)

this is at least clear.

@mossheim
Copy link
Contributor

I'm not sure there's really a 'contract' in the sense that you mean

'documented meaning' then? let's not split hairs like this, it would be a very significant change to what the method does.

@muellmusik
Copy link
Contributor

muellmusik commented Jan 13, 2018 via email

@mossheim mossheim added the comp: class library SC class library label Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: class library SC class library
Projects
None yet
Development

No branches or pull requests

4 participants