Correct type of $.test #121
Conversation
index.js
Outdated
| @@ -2405,7 +2416,7 @@ | |||
| Date: Date_, | |||
| Error: Error_, | |||
| FiniteNumber: FiniteNumber, | |||
| Function: def('Function', {}, [Array_(Type), Type], Function_), | |||
| Function: def('Function', {}, [Array_(VariadicType), Type], Function_), | |||
There was a problem hiding this comment.
I changed this one, but I'm not sure that I should. I'll ask you first. :)
There was a problem hiding this comment.
I think this should remain unchanged. It doesn't make sense to define Array -> Array or Either -> Either.
Avaq
force-pushed
the
avaq/correct-typeof-test
branch
from
e182592 to
ae60bce
Compare
| 'sanctuary-def/VariadicType', | ||
| '', | ||
| function(x) { | ||
| return test([], Type, x) || test([], AnyFunction, x); |
There was a problem hiding this comment.
I tried different ways of testing:
test([], Type, applyParameterizedType(x))- This blows up if someone would provide, say[x => true]asenv.test([], Type, x) || test([], Function([Type, Type]), x) || test([], Function([Type, Type, Type]), x)- This effectively behaves the same asAnyFunction, becausetestdoes not applyxto determine its types.- This left me with the method seen above.
The decision, really, is between 1 and 3, where 3 is overly permissive, and 1 blows up with an ugly error. Maybe to get the best of both worlds, I should combine them as:
test([], Type, x) || test([], AnyFunction, x) && test([], Type, applyParameterizedType(x))There was a problem hiding this comment.
This predicate looks fine to me. I think the ultimate solution is to require values such as $.Array($.Unknown) and S.EitherType($.Unknown, $.Unknown) in place of $.Array and S.EitherType. I hope VariadicType will be short-lived, so the predicate need not be perfect.
There was a problem hiding this comment.
So you vote for not combining 1 and 3? In that case, shall I undo the diff that introduces applyParameterizedType (singular)?
index.js
Outdated
| type.apply(null, Z.map(K(Unknown), range(0, type.length))) : | ||
| type; | ||
| } | ||
|
|
There was a problem hiding this comment.
See the reason for this diff in my other comment
There was a problem hiding this comment.
I'd prefer not to include this refactoring to keep the diff as clear as possible. If you explained your reasons for making this its own function in another comment, I haven't been able to find the comment. ;)
There was a problem hiding this comment.
The reason was to enable the test([], Type, applyParameterizedType(x))-way of testing VariadicType membership. I left it in for illustrative purposes.
There was a problem hiding this comment.
Ah, I see. I didn't make the connection, I'm sorry.
Avaq
force-pushed
the
avaq/correct-typeof-test
branch
2 times, most recently
from
b10964e to
62754fc
Compare
|
🍏 |
index.js
Outdated
| @@ -762,6 +762,14 @@ | |||
| typeEq('sanctuary-def/Type') | |||
| ); | |||
|
|
|||
| var VariadicType = NullaryType( | |||
There was a problem hiding this comment.
Let's add a type signature along these lines:
// VariadicType :: Type | Type -> Type | Type -> Type -> Type | ...The made-up nature of the signature will draw attention to our naughtiness.
Avaq
force-pushed
the
avaq/correct-typeof-test
branch
from
62754fc to
043d176
Compare
|
I'll close this in accordance with #123 (comment). |
Opened the WIP pull-request for discussion. Comments will follow.