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

Update errors for RawRepresentable types. #277

Closed

Conversation

paulyoung
Copy link
Contributor

This aims to close #275.

Using my Provider example:

enum Provider {
  case Facebook = "facebook"
  case GitHub = "github"
  case Twitter = "twitter"
}

Before:

TypeMismatch(Expected .Some(Provider), got .None)

After:

TypeMismatch(Expected Provider, got String(google))

@tonyd256
Copy link
Contributor

I'm not sure that After is better. Would you mind posting the code that would throw the error as well?

I'd like to maintain type information while showing the user what the original value was. I liked your original proposal better with this format: TypeMismatch(Expected X, got Y from/with Z).

@paulyoung
Copy link
Contributor Author

Would you mind posting the code that would throw the error as well?

In this case, the code that causes the error is essentially self.init(rawValue: "google") where "google" is the value in the JSON but doesn't match a raw value in the enum.

@tonyd256 is that what you meant?

@tonyd256
Copy link
Contributor

Gotcha. Looking through Argo, this is the only place we use fromOptional. And because of that, this is the only place we're not showing the actual json but instead just focus on the Optional. I think this is a good change!

@paulyoung
Copy link
Contributor Author

How about this?

TypeMismatch(Expected rawValue for Provider, got String(google))

@tonyd256
Copy link
Contributor

Would it be too much to print out all options? Something like:
TypeMismatch(Expected one of [facebook, github, twitter], got google)
This would probably require some reflection but is the error explicitness worth it?
Also, do you think this would get too big in large enum cases?

@paulyoung
Copy link
Contributor Author

Would it be too much to print out all options?

Personally, I'm happy to go and look at the enum if I'm not sure of the expected raw values. I think the fact that obtaining the actual raw value required making the request again was the real pain point for me.

Also, do you think this would get too big in large enum cases?

I have an enum of ISO 3166-1 country codes that I definitely wouldn't want printed out!

This would probably require some reflection

I'm not too familiar with reflection in Swift so can't speak to any performance implications or safety. I've avoided it so far but think the other points I mentioned mean it might not be worth it.

@tonyd256
Copy link
Contributor

tonyd256 commented Nov 2, 2015

Ya, I figured that might be a bit much. I like your second suggestion best I think: TypeMismatch(Expected rawValue for Provider, got String(google))

@gfontenot any thoughts here?

@gfontenot
Copy link
Collaborator

Yeah, I had the same thought re: using crazy reflection stuff to print out the expected values, but I agree that it's probably overkill and not worth the time/complexity. I think that the proposed solution (Expected rawValue for Provider, got String(google)) is a solid improvement without going crazy.

@paulyoung
Copy link
Contributor Author

Updated!

@gfontenot
Copy link
Collaborator

Awesome. Merged in as da89baf.

@gfontenot gfontenot closed this Nov 4, 2015
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

Successfully merging this pull request may close these issues.

Better error message for enums/RawRepresentable types
3 participants