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

Non-side-effecting Read #312

Closed
peter-empen opened this issue Jun 1, 2020 · 8 comments
Closed

Non-side-effecting Read #312

peter-empen opened this issue Jun 1, 2020 · 8 comments

Comments

@peter-empen
Copy link

Hi Li,

what is your attitude towards an additional read method that will never throw an exception? I mean a signature like
def maybeRead[T: Reader](s: ujson.Readable): Either[String, T]
where String would be the message of the exception you are throwing.

Although this is easy to achieve on top of the current read, I still wonder whether it was worth to supply.

@lihaoyi
Copy link
Member

lihaoyi commented Jun 1, 2020

Hi Peter!

This sounds alike a reasonable either (maybe called readEither?).

One issue with uPickle is I do not think it is sufficiently rigorous with what exceptions are thrown, when and why. Trying to stuff it into an Either or Try would be a good time to start adding tests and documentation for all the error cases, so we know exactly how uPickle behaves in each case

@peter-empen
Copy link
Author

peter-empen commented Jun 1, 2020

Yeah, readEither sounds really good, I'm not opinionated. In Circe decode returns the same.

Is that something you'd do soon or do you prefer me to give it a try?

Would it suffice to check for Exception and extract the massage where ever it was instantiated?

@lihaoyi
Copy link
Member

lihaoyi commented Jun 1, 2020 via email

@peter-empen
Copy link
Author

OK.
To finalize the signature, is it correct to assume that read always fails fast? Otherwise we better use a type that allows to return any number of failures.
And, if we just want to return a single failure, does it make sense to have a ParseFailure class instead of just using String?

@lihaoyi
Copy link
Member

lihaoyi commented Jun 1, 2020

yes currently read always fails fast with an exception

@peter-empen
Copy link
Author

peter-empen commented Jun 17, 2020

  1. Concerning PR 317, how can I compile against Scala 2.11 in mill locally? I tried upickle.jvm("2.11.12").compile() but it does not work.
  2. Also, what is your preferred strategy to replace method calls that were not available in 2.11?

@peter-empen
Copy link
Author

@lihaoyi Waiting for your kind review of #317...

@lihaoyi
Copy link
Member

lihaoyi commented Jul 13, 2024

Closing this as a wontfix together with #317

@lihaoyi lihaoyi closed this as completed Jul 13, 2024
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

2 participants