-
Notifications
You must be signed in to change notification settings - Fork 4
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
Class.getMethods()
returns Object
instead of Method[]
#27
Comments
Hi @DanStevens, I'm not sure which the best approach, by one hand, having an object indexed by names of each function is more easy to use, on the other hand, it messes with inheritance, as a function may be overwritten. BTW, it's the same with class properties. The array may be the simplest approach as it lets the user to implement what it needs, so lets go in that way, but in order to be consistent, properties, constants & others may also follow the same rule. Dan, if you have some time, could you make the changes and send a PR, and I'll merge it. |
This is a fair comment. There is also the issue of breaking consumer implementations by changing the returned value from an object to array. Returning an object is an advantage if you know the name of the method you want to work with. The downside is that it's less type safe for Typescript consumers (ignoring the incorrect type definition in I'm happy to look into this. I'm learning Typescript so it would be a good exercise. |
…tMethods()` methods of the `Class` class as actual implementation returned keyed object rather than array. [Issue glayzzle#27](glayzzle#27)
…tMethods()` methods of the `Class` class as actual implementation returned keyed object rather than array. [Issue glayzzle#27](glayzzle#27)
I've been reflecting on this and I'm not sure if it's best to change the implementation to return an array for these methods as this would be a breaking change, although this is inconsistent with other methods that do return arrays (e.g. However, what I have done, in pull request #28, is corrected the return types in the type definition file for these methods so that Typescript users can use the return result without errors. |
I don't know if this module is massively used, but anyway I can release the change on a major version. It's not quite complete to return a name indexed object as on this cases the objets may be defined multiple times as it may handle inheritance and overwriting of methods, interfaces declaration, etc ... Please could you try a PR with an array implementation, for me it's the best choice as it does not hide information |
The
getMethods
function of theClass
class returns anObject
rather than an array ofMethod
objects, as the name suggests. This also contradicts the type defined for this method in index.d.ts.The text was updated successfully, but these errors were encountered: