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

Improve fromBytes type signature #2

Open
MartinSStewart opened this issue Aug 12, 2019 · 3 comments
Open

Improve fromBytes type signature #2

MartinSStewart opened this issue Aug 12, 2019 · 3 comments

Comments

@MartinSStewart
Copy link

This function should never return Nothing, but it uses Bytes.Decode.decode, which returns a Maybe String.

If this is the case, can't you modify fromBytes to look like this?

fromBytes : Bytes -> String
fromBytes =
    Decode.fromBytes |> Maybe.withDefault ""

The default value will never get used (if it does there's a bug which should be fixed anyway) and the user gets a nicer API to work with.

@danfishgold
Copy link
Owner

The only thing keeping me from changing the signature of fromBytes is the possibility of getting Nothing from Bytes.Decode.decode even though the decoder should be able to parse any sequence of bytes. Unfortunately the documentation on this is a little vague (what does it mean to have "corrupted" bytes?)

I submitted elm/bytes#17 which would hopefully clear this up, but until then I'd rather keep this Maybe here. Especially if there is a way to get Nothing I hadn't considered, I don't think implicitly handling the error and returning an empty string is a good user experience for the package.

I do have a commit that changes the function's signature and adds a test with a fuzzy sequence of bytes to make sure it always return Just _, so whenever we get an answer on that issue, version 2.0.0 shouldn't take long :)

@Malax
Copy link

Malax commented Oct 29, 2019

First of all, thank you for creating this package! :)

I found it because I've been looking for a base64 package that has a proper signature for base64 encoding that does not introduce an error type to the result and uses a proper input type. This package has almost everything I was looking for, except the return type.

I see the reasoning behind keeping the Maybe from the standpoint of this library. I agree that, in a vacuum, keeping the Maybe seems to be the correct solution.

However, when used in another package this leads to that package using the same reasoning to keep the Maybe from fromBytes as well, passing it up to their users. This leaves the end-user to resolve the Maybe. Unfortunately, every new layer adds more uncertainty if the error case can be safely ignored.

My concrete use-case is with a package I wrote that can create Thumbor URLs. I use https://github.com/romariolopezc/elm-hmac-sha1 to sign those URLs as required by Thumbor. romariolopezc/elm-hmac-sha1 does provide a function that returns a base64 version of the signature, but introduces a Result type, inherited from https://github.com/waratuman/elm-coder. This is the same situation, even though base64-bytes is not involved.

This results in https://github.com/itravel-de/elm-thumbor/blob/1.0.0/src/Thumbor.elm#L162 where I have to resolve this with a special fallback string. I wanted to open a PR for romariolopezc/elm-hmac-sha1 that removes the Result type. I was hoping to just make a little change so it uses base64-bytes and call it a day. :)

I think resolving this "impossible state" should happen as early as possible so that it does not "infect" more code. Have you had the chance to look more deeply into this since the last comment? I am happy to help out if I can! :)

@danfishgold
Copy link
Owner

I still haven't gotten a response on elm/bytes#17. The minute (or week) I do, I'll update this package, but until then I don't feel comfortable changing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants