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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add getPropOrError to crocks #509

Open
kierans opened this issue Mar 11, 2021 · 6 comments
Open

Add getPropOrError to crocks #509

kierans opened this issue Mar 11, 2021 · 6 comments
Labels
enhancement good first issue needs work The issue itself needs more details before it can be worked on

Comments

@kierans
Copy link

kierans commented Mar 11, 2021

Is your feature request related to a problem? Please describe.
I'm on my fourth project using Crocks and I keep reaching for the same function getPropOrError (and getPathOrError). Instead of me releasing my own library, or copy/pasting the function around I think it would be good to have this in the standard library.

Describe the solution you'd like
getPropOrError :: String -> a -> Result Error b
getPathOrError :: [ String ] -> a -> Result Error b

Describe alternatives for how you do this now
Doing a maybeToResult on getProp with an Error if getProp returns a Nothing.

Code
Do you have example code or tests in a Repl for what you'd like added?

// getPropOrError :: String -> a -> Result Error b
const getPropOrError = curry((prop, obj) =>
  maybeToResult(new Error(`'${prop}' is mandatory and missing`), getProp(prop, obj))
);

Additional context
Add any other context or screenshots about the feature request here.

@dalefrancis88
Copy link
Collaborator

I think if we were to implement this, we'd want to match getPropOr style and have it so you can specify the error

What may help here is some example use cases as well, think "what would i want to put in the docs?"

@dalefrancis88 dalefrancis88 added the needs work The issue itself needs more details before it can be worked on label Mar 11, 2021
@kierans
Copy link
Author

kierans commented Mar 11, 2021

@dalefrancis88 Quick response!

I see your point, however I see the function more akin to getProp rather than getPropOr as the point of the function is you either get the prop, or an Error because the prop is missing. I don't see it as valuable to be able to specify "the error". Maybe that's just because I structure all my errors the same way 😂

My use cases are often around pulling props out of config objects, JSON responses from an API (where according to the contract the prop is mandatory), etc. Anywhere where the program can't continue because it's missing mandatory input. For example:

// readSecretKey :: a -> Result Error b
const readSecretKey = 
  pipe(getPropOrError("secretKey"), map(decryptKey))

So if the secretKey is missing, we get back an Error at the edge, 'secretKey' is mandatory and missing.


On a related note, something that's confused me a bit when using HM is whether to use Object as a type in the signature (the way we use String or Integer.)

For example, shouldn't readSecretKey be of type Object -> Result Error a? The input to the function should be an object. However from the Crocks docs, functions that should take only objects, just seem take "anything", for example, getPropOr :: a -> (String | Integer) -> b -> a (where b really should be an Object IMHO). Is there something I'm not understanding?

@dalefrancis88
Copy link
Collaborator

so to answer the HM statement (i know i'm going to butcher this explanation, @evilsoft will likely come with a better one), you use a,b,c,etc when it's their to represent that it can be given anything. I can see where your thought is at though on this, but think of it as the any type from TS. When you want to be specific with the structure of you input you can us HM to define a type then use that type in subsequent signatures, you've likely come across this in the docs. lmk if not, i'll find an example

On the main point, I would see this as equiv to getProp, was more calling out that we'd likely want to implement it so that the consumer of the function could specify the error rather than the crocks library having a generic error. i.e. in your second example

// readSecretKey :: a -> Result Error b
const readSecretKey = 
  pipe(getPropOrError("Unable to locate the secret key", "secretKey"), map(decryptKey))

@kierans
Copy link
Author

kierans commented Mar 11, 2021

you use a,b,c,etc when it's their to represent that it can be given anything. I can see where your thought is at though on this, but think of it as the any type from TS.

Thanks for the explanation. That's what my current understanding is. It's more a question of why have functions been typed to work on "anything" rather than Object. You don't get a prop on something that's not an object (we'll leave aside how just about everything in Javascript is an object). I would't call getPropOr(5, "key", 1) for example. So even though it's type correct (with b being an Integer) it's to my understanding nonsensical. So I feel there's something I'm missing, or not understanding 😆

On the main point, I would see this as equiv to getProp, was more calling out that we'd likely want to implement it so that the consumer of the function could specify the error rather than the crocks library having a generic error. i.e. in your second example

Fair point, however I wouldn't find that as useful, because I want the message to be consistent everywhere without me having to specify it. So I'd have to do either:

  1. getPropOrError(mandatoryProp(), "secretKey")
  2. const getMandatoryProp = (prop) => getPropOrError("'${prop}' is mandatory and missing", prop)

(2) isn't completely fuggly and it would allow getPropOrError to be more generic/reusable so I could live it.

@dalefrancis88
Copy link
Collaborator

@kierans given that some time has past, how do you feel about this suggestion, does it still stand? Have you had any further insights that might help with this?

@kierans
Copy link
Author

kierans commented May 19, 2021

@dalefrancis88 I haven't been doing much with Crocks lately as I've been doing other work. However I'm coming back to it.

I don't have new insights, but rereading the comments above, I still agree with past me.

It just depends on whether you/others feel this is a worthwhile addition to the Crocks standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue needs work The issue itself needs more details before it can be worked on
Projects
None yet
Development

No branches or pull requests

2 participants