Add typeOf() function. #381
Conversation
andrew-coleman
left a comment
andrew-coleman
left a comment
There was a problem hiding this comment.
Hi Jacob, many thanks for implementing this. Just a couple of review points... ;)
src/functions.js
Outdated
| if(['string', 'boolean'].includes(typeof value)) { | ||
| return typeof value; | ||
| } |
There was a problem hiding this comment.
Array.includes() is an ES6 function, which means that the transpiled jsonata-es5.js will not work in ES5 environments such as IE unless we explicitly put the polyfill into polyfill.js.
However, in this case, I think it would be easier to remove this block and replace it with two if statements for the string and boolean cases.
docs/object-functions.md
Outdated
| ## `$typeOf()` | ||
| __Signature:__`$typeOf(value)` |
There was a problem hiding this comment.
I think I prefer the name $type as you proposed originally in #208. In general I favour names as single words, provided the word is a meaningful representation of what the function does. I think in this case $type would be ok?
There was a problem hiding this comment.
Sure. Most languages (e.g. JavaScript) call the operator typeof so I went with that pattern. If single word function names are preferred, we can use that.
* Rename `typeOf()` to `type()` * Avoid use of Array.includes()
|
excellent - many thanks! |
3 participants
Proposed in #208