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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor implementation lookup #146

Merged
merged 1 commit into from
Apr 19, 2021
Merged

Conversation

davidchambers
Copy link
Member

This pull request removes the awful funcPath function, and splits getBoundMethod into getStaticMethod and getPrototypeMethod (which can be top-level functions as they do not reference requirements).

Aldwin and I have made changes to these several times over the years. 馃槀

@davidchambers davidchambers requested a review from Avaq April 19, 2021 17:26
});
prototypeMethodNames.forEach (function(_name) {
typeClass.methods[_name] = function(x) {
return (getPrototypeMethod (_name) (x)).bind (x);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place that Function#bind is invoked. At the other getPrototypeMethod call site all we want to know is whether the method exists.

Copy link
Member

@Avaq Avaq Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, see also 4c9ddd7, where I split the function into two. One for checking, one for binding.

Comment on lines +297 to +298
return x != null &&
getStaticMethod (_name) (x.constructor) != null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test suite passes without the x != null guard. All of this library's type classes that require static methods happen to depend on type classes that require prototype methods that Null and Undefined cannot provide.

These steps demonstrate that x.constructor should be guarded:

  1. Remove the x != null guard.
  2. Remove the Semigroupoid dependency from Category.
  3. Evaluate Z.Category.test (null).

Since users can define their own type classes via Z.TypeClass, it is necessary to include the guard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

users can define their own type classes [which necessitate inclusion of] the guard

That sounds like it might warrant a test.

@davidchambers davidchambers merged commit aaca2bf into master Apr 19, 2021
@davidchambers davidchambers deleted the davidchambers/lookup branch April 19, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants