search

amnaredo / test

0 stars 0 forks source link

Non-side-effecting Read #271

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

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.

ID: 312 Original Author: peter-empen

amnaredo commented 3 years ago

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 Original Author: lihaoyi

amnaredo commented 3 years ago

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? Original Author: peter-empen

amnaredo commented 3 years ago

I won’t have time to take a crack at it any time soon, so go ahead

On Mon, 1 Jun 2020 at 7:56 PM, peter-empen notifications@github.com wrote:

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?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/lihaoyi/upickle/issues/312#issuecomment-636814679, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHEB7GXMLETGIT5XC6DU6LRUOJPTANCNFSM4NPW6Q3A .

Original Author: lihaoyi

amnaredo commented 3 years ago

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? Original Author: peter-empen

amnaredo commented 3 years ago

yes currently read always fails fast with an exception Original Author: lihaoyi

amnaredo commented 3 years ago
  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? Original Author: peter-empen
amnaredo commented 3 years ago

@lihaoyi Waiting for your kind review of https://github.com/lihaoyi/upickle/pull/317... Original Author: peter-empen