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

Return a Try[JsObject] for fromDynamoJson (reads) #1

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

rmmeans
Copy link
Member

@rmmeans rmmeans commented Aug 11, 2016

A JsObject passed into the converter may throw an match exception because the object was:

  1. not actually a dynamoDB JsObject
  2. Has some unsupported types on it that we don't currently support (i.e. Binary ("B") or BinarySet ("BS"))

A JsObject passed into the converter may throw an match exception because the object was:

1) not actually a dynamoDB JsObject
2.) Has some unsupported types on it that we don't currently support (i.e. Binary ("B") or BinarySet ("BS"))
@rmmeans rmmeans merged commit ba89c09 into LifeWay:master Aug 11, 2016
@rmmeans rmmeans deleted the add-try-to-converter branch August 11, 2016 13:07
@russwyte
Copy link

I would recommend something more concrete about the returned failure type than what you get with Try. Or or Either would be much better.

@rmmeans
Copy link
Member Author

rmmeans commented Aug 11, 2016

Hmm, I'm not sure what I would put in the response? The exception is a match error on the Json because the json wasn't actually dynamo formatted Json. I guess the match error itself would be useful as it probably contains the exact item that did not match?

@russwyte
Copy link

http://aboutwhichmorelater.tumblr.com/post/30409572482/scala-util-try

Try is a just a pretty awful type. It would be better to create a real failure type with the information you want to get at clearly encoded.

Or is a much better monadic structure for the return type and would allow you to map over every error - not just one type of "Throwable"

@russwyte
Copy link

One thing to consider would be to use Play JSON's validation facility.

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.

None yet

2 participants