[RFC] Check that a value has a method

[CrazyInfin8]

There does not currently appear to be a way to ensure that a value has a certain method (at least not from wren or the public C api).

We can just make classes that extend some baseline class and check that instance is Class but a class can only extend one class. An example like this mostly works:

class Reader {
  construct new() {}
  read() { "a" }
}

class Writer{
  construct new() {}
  write(a) {
    System.print(a)
  }
}

class MassReader is Reader {
  construct new() {
    _readers = []
  }
  add(reader) {
    if (reader is Reader) {
      _readers.add(reader)
    } else {
      Fiber.abort("Argument is not a reader.")
    }
  }
  read() {
    var results = []
    for(reader in _readers) {
      results.add(reader.read())
    }
    return results
  }
}

class MassWriter is Writer {
  construct new() {
    _writers = []
  }
  add(writer) {
    if (writer is Writer) {
      _writers.add(writer)
    } else {
      Fiber.abort("Argument is not a writer.")
    }
  }
  write(a) {
    for(writer in _writers) {
      writer.write(a)
    }
  }
}

// collection of readers
var r = Reader.new()
var mr = MassReader.new()
mr.add(r)
System.print(mr.read())

// Collection of writers
var w = Writer.new()
var mw = MassWriter.new()
mw.add(w)
mw.write("Hello world")

We know that any Reader and Writer will have a read() and write(_) methods respectively so MassReader and MassWriter can check that arguments added to them extend those classes to be sure they have those functions. Although it may be more troublesome to add a class like this:

class ReaderWriter {
  construct new() {}
  read() {
    return "a"
  }
  write(a) {
    System.print(a)
  }
}

It's unclear which class this should extend since it is functionally similar to both reader and writer. MassReader and MassWriter don't really care if it's arguments do contain Readers and Writers but they care more that the items added to them contain the necessary functions.

I was wondering whether something similar to this is worth pursuing:

if(item has read()) mr.add(item)
if(item has write(_)) mw.add(item)

[ChayimFriedman2]

You can extend only one class - but you can overload is(_) (though I claim that is overloading is broken and should be inverted).

Also, I think this is wrong use of dynamic typing. You should use duck typing - do not rely on types, rely on behavior.

And if you really want, you can always Fiber.try().

Edit: Even if this is accepted, I don't think an operator is the right way - I do miss reflection abilities, too, but the solution should be library-based and not language-based. Maybe a new class Reflect on the module meta.

[mhermier]

The is operator is the way to go, if you can make a abstraction class, and make your class adhere to it.

Otherwise, this was discussed a while ago with @munificent, at that time it was concluded that such feature needed to be done in an external module as part of a mirroring API of some sort.

[PureFox48]

Some other languages which don't have multiple inheritance solve this by allowing a class to implement multiple interfaces instead.

However, this introduces its own problems so I don't think we want to go down that route with Wren.

I don't like overloading the is operator either and agree with @ChayimFriedman2 that a better solution would be to add a Reflect class to the meta module.

[PureFox48]

A further thought.

An in or smart-match (~~) operator might be useful here:

if ("read()" in Reflect.methods(item)) mr.add(item)

would be easier to read than:

if (Reflect.methods(item).contains("read()")) mr.add(item)

[mhermier]

How about:

Reflect.hasMethod(item, "read()")
Reflect.getMethod(item, "read()") // For the future, not implementable as this yet

[PureFox48]

Yeah, I think you're right that a Reflect.hasMethod(object, method) would be better for testing whether an object has a particular method or not.

Reflect.methods(object) could be used to return a list of all an object's methods.

[mhermier]

About the module where this should belong, even it looks like it would fit it meta I think it is better to have it in its own module. The reason is that we might want to disable the one, the other or both for security reasons.

[PureFox48]

even it looks like it would fit it meta I think it is better to have it in its own module.

Yes, I'd be comfortable with that (and optional like meta is now) particularly as it may end up being quite a large class.

[mhermier]

I already have some code around like this, since the idea is not new. I will give a try at implementing it (the minimal required version) later today.

[mhermier]

Please take a look at that preliminary implementation at https://github.com/mhermier/wren/tree/mirror. API is slightly different that what was proposed here, but it sounds more logical to me for the future developments.

[ChayimFriedman2]

This API is very limited. I would provide getMethods() and getMethod(), at least.

[mhermier]

For now it is bare-bone and limited to the original request. That said methods and getMethod(_) is a little bit problematic, since there is no internal ObjMethod. So initially it will probably methodsNames only.

[ChayimFriedman2]

I'm well aware and think about solutions.

The simplest one would be to create a Fn on-the-fly, but it would be costly. I look for another way.

[mhermier]

Anyway, I don't want to rush a whole API, so that basic feature get accepted first. So we can use it already and start to build on top of it.

[ChayimFriedman2]

Shouldn't the static hasMethod() be private (hasMethod_())?

[mhermier]

It should but I provided it for speed.

[ChayimFriedman2]

Speed?

[mhermier]

Most usage should be hit and run:

if (ObjectMirror.hasMethod(foo, "bar(_)")) foo.bar(42)

If you use the regular API:

if (Reflect.call(foo).hasMethod("bar(_)")) foo.bar(42)

With the official API, there is a round trip to allocator and 4 function calls, instead of a one-shot function call...

[ChayimFriedman2]

Then it should be on Reflect, isn't it?

Edit: Also, I think Reflect.call() is not a descriptive name.

[mhermier]

In a regular mirror API, reflect is a function and it should return a Mirror based on what you passed. I did not put the hasMethod() there to respect the function nature of Reflect. Technically, it should be an intrinsic/internal function, but I thought it would be better to be near to its normal use.

[ChayimFriedman2]

I can argue of that, but I think it's matter of somewhat personal preference.

[mhermier]

Well this is what I learned from looking at other implementations. Maybe these days it changed a little, but the basic functionality is the same, only the organization change depending on implementation.

[mhermier]

I think I understand what you meant and it is a naming issue.

[PureFox48]

It looks like you're following some sort of factory pattern here as the PR is a bit more convoluted than I was expecting. But the important thing is that it's starting to put some flesh on the bones of this idea.

I agree with @ChayimFriedman2 that Reflect.call(_) is not a good name for the factory method. How about Reflect.upon(_) instead? That reads well - to me at least :)

You could also use Reflect.on(_) but it's probably best to avoid a short word like that in case it's needed as a keyword in a future version.

[mhermier]

It is not a real factory pattern, but more a type dispatch. Doing it functional can work, but that way I can cache some results, while still allowing some functionnal API for faster access. Well if you are really that troubled, I can close the naming debate by making it a real function. It will only make caching a little bit more complicated... Or I can make a base Mirror class, and do Mirror.reflect(foo).

[PureFox48]

I'm fine with the way you've done it if it's more efficient that way.

Or I can make a base Mirror class, and do Mirror.reflect(foo).

Yeah, that would be better.