search

evancz / url-parser

Parse URLs into nicely structured data
http://package.elm-lang.org/packages/evancz/url-parser/latest/
BSD 3-Clause "New" or "Revised" License
114 stars 29 forks source link

Should QueryParsers fail, instead of succeeding with a Maybe? #13

Closed krisajenkins closed 7 years ago

krisajenkins commented 7 years ago

I have a pair of views I would like to parse. My user can see a registration page, and when they register they'll get a "click on this link" email with a confirmation key they need to send back to us:

type View
    = Registration
    | Confirmation String

I'd like to parse these out:

parser : Parser (View -> a) a
parser =
    oneOf
        [ map Register <| s "register"
        , map Confirm <| s "confirm" <?> stringParam "key"
        ]

Unfortunately this isn't going to work, because stringParam returns a Maybe String. So as it stands, I have to change my model to Confirm (Maybe String).

That seems to me to be the wrong solution. The data-model is correct as-is. Confirm Nothing isn't a valid view, and so shouldn't have a representation.

Wouldn't it be better if QueryParsers just failed, instead of always succeeding with a Maybe? Then the uncertainty - the maybe-ness - would fall out of the fact that a parser returns Maybe View already.

The downside would be if key genuinely was optional, I'd have to have two rules in my oneOf clause. But that seems like a fair place to put the burden. If key is an optional part of the location, then I do have two URL rules. The code would just be following the duplication in the spec.

process-bot commented 7 years ago

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

krisajenkins commented 7 years ago

I note that the duplication-approach would tie in with the advice in https://github.com/evancz/url-parser/issues/7

etaque commented 7 years ago

if key param is mandatory, it could go into the path instead of the query string? I like to see query params as a bag of optional parameters.

benpence commented 7 years ago

@etaque I agree that the mandatory params should go on the path. That being said, I've seen APIs in the wild that use mandatory query parameters, so I think we should provide this functionality since API conventions are not enforced by any protocol.

evancz commented 7 years ago

This is not for parsing "APIs in the wild" as far as I know. I designed it for parsing your URLs. There is only one example here, and it seems like a one-off problem that could just as easily be done in the path. So based on the data I am seeing, I don't feel there is a compelling case for this change.

Perhaps if @krisajenkins can provide more code that demonstrates how this looks in practice, that could be persuasive. Please open a new issue with further details if this is possible!