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

Catch Exceptions In Newtype Constructors #883

Open
adamgfraser opened this issue Mar 4, 2022 · 4 comments
Open

Catch Exceptions In Newtype Constructors #883

adamgfraser opened this issue Mar 4, 2022 · 4 comments

Comments

@adamgfraser
Copy link
Contributor

The following snippet will currently throw a runtime exception since the string cannot be parsed to a UUID:

import java.util.UUID

type Id = Id.Type
object Id extends Newtype[UUID]

Id.make(UUID.fromString("foo"))

Instead, the implementation of make could catch the error and render it as a string.

It is a little debatable whether we should catch thrown exceptions like this in addition to assertion failures but I tend to think with these "smart" types we should catch and render thrown exceptions, which I believe is similar to what would occur if an exception was thrown in assertTrue.

Thanks to @wi101 for reporting.

Copying @kitlangton.

@sideeffffect
Copy link
Member

Currently, the type of make is

def make(value: A): Validation[String, Type]

So we would need to change it to

def make(value: => A): Validation[String, Type]

right?

Since we're already changing the signature, why not even

def make(value: => A): Validation[IllegalArgumentException, Type]

to have richer information about the error?

@adamgfraser
Copy link
Contributor Author

I'm not sure if we need to do the by name parameter since we are doing this at compile time but we potentially could. I'm not sure we get that much value from making it an IllegalArgumentException since if we do anything I think we would want to catch any errors that are not fatal so we would have to return Throwable and at that point we aren't really getting any additional information.

@sideeffffect
Copy link
Member

sideeffffect commented Mar 6, 2022

I'm not sure if we need to do the by name parameter since we are doing this at compile time but we potentially could.

If we didn't do that, the type of the method would tell a different story from what effectively happens, right? I think that could unnecessarily confuse people.

@adamgfraser
Copy link
Contributor Author

I think evaluating something at compile time already kind of tells a different story but I don't think it is a problem to make it by name and looks like that is what we are doing with assertTrue.

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