-
Notifications
You must be signed in to change notification settings - Fork 87
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
defaultsTo shouldn't factor in type #36
Comments
An alternative implementation of
...I'm still a little new to the 'data last' function design, so perhaps the above function can be written more elegantly using |
Thank you for bringing up this issue. I think that current
I like that I have this option even if it is not the most common case. I created this behavior because I needed in practice and I still find it useful. Anyway, your proposal for |
Fair enough re. Reason I posted this issue is because it is a common design pattern for an One such example might be a key value on an I have been working on a package called By default, some built-in transform functions are used unless the user specifies their own or chooses to disable them by passing Anyway, I digress. With regards to const pathOr = curry((defaultValue, inputPath, inputObject) => {
const inputValue = path(inputPath, inputObject)
return type(inputValue) === 'Undefined' ? defaultValue : inputValue
}) |
I see your use case and I am convinced that As for the current implementation with type checking - it will be moved to Rambdax library as method I will add the new version of Btw, your library looks very promising - keep the good work :) If you let me know I will add it as a showcase for Rambda. |
Might I suggest you implement the function I posted above as ...and with regards to // this is how I think defaultTo should work (could rename to 'defaultOr' perhaps?)
const defaultOr = curry((defaultValue, inputValue) => {
return type(inputValue) === 'Undefined' ? defaultValue : inputValue
})
// then you could have your type equality function
const defaultTypeOr = curry((defaultValue, inputValue) => {
return type(inputValue) === type(defaultValue) ? inputValue : defaultValue
})
// pathOr could then use defaultOr
const pathOr = curry((defaultValue, inputPath, inputObject) => {
return defaultOr(defaultValue, path(inputPath, inputObject))
})
// then you could also have
const pathTypeOr = curry((defaultValue, inputPath, inputObject) => {
return defaultTypeOr(defaultValue, path(inputPath, inputObject))
}) |
Ignore what I posted above re. http://ramdajs.com/docs/#defaultTo I also see where the name comes from, so also ignore my suggestion to rename |
Yes, I accept your recommendation to stay as close to Ramda API as possible. Now You can see below bundlephobia.com comparison between |
...so to follow up, here is my final proposal: const defaultTo = curry((defaultValue, inputValue) => {
const inputType = type(inputValue)
const isInputNaN = inputType === 'NaN'
const isInputNull = inputType === 'Null'
const isInputUndefined = inputType === 'Undefined'
const isInputFalsy = isInputNaN || isInputNull || isInputUndefined
return isInputFalsy ? defaultValue : inputValue
})
const typeDefaultTo = curry((defaultValue, inputValue) => {
return type(inputValue) === type(defaultValue) ? inputValue : defaultValue
})
const pathOr = curry((defaultValue, inputPath, inputObject) => {
return defaultTo(defaultValue, path(inputPath, inputObject))
})
const typePathOr = curry((defaultValue, inputPath, inputObject) => {
return typeDefaultTo(defaultValue, path(inputPath, inputObject))
}) |
In the context of not adding additional API, I think it is better to pass an additional argument, which will enable type checking. The other option is to add Let me know what you think about it? |
Adding another argument wouldn't work because it doesn't adhere to ramda's "data last" design. With your proposal of Since the function you desire with value type checking isn't apart of I think this is perfectly acceptable and useful to a lot of people. Just because it isn't apart of It's got a pretty small footprint, so it's only going to add a few bytes right? On a separate note, I've just been looking at the source code of some of your modules and wondered why you weren't using the internal |
Up until now As for your argumentation to add What about |
Yer I think that would also be a really useful function to have for sure! Not sure whether the functions should be named:
or
...basically |
:) I was thinking the whole time that |
Nice one, very much appreciated! |
Closing the issue as version 0.9.1 has now all proposed changes. |
As far as I can tell
defaultsTo(defaultArgument, inputArgument)
is the only method that provides a way for defaulting to a value (providing some 'or' logic). However, the built-in test forinputArgument
being the sametype
asdefaultArgument
limits the use of this function. It is my opinion thatdefaultArgument
should only be returned ifinputArgument
isundefined
.I would like to compose a function works the same as Lodash's
get
method or Ramda'spathOr
method whereby a value is read from a path through a nested object tree and defaults to a value if this path returnsundefined
.The text was updated successfully, but these errors were encountered: