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

Added selective field getters for JsObject #63

Closed
wants to merge 1 commit into from
Closed

Added selective field getters for JsObject #63

wants to merge 1 commit into from

Conversation

cvrabie
Copy link

@cvrabie cvrabie commented Aug 5, 2013

When writing a custom JsonFormat the currently proposed way of using pattern matching/extractors on JsObject.getFields is elegant and concise especially when all the fields are required.

However we often meet the situation where, even though the fields in the domain object are required, we can provide default values for some of these fields, thus making some part of the incoming json optional.

For example, a User class might require a language and timezone parameters but we we can use the system defaults if the incoming json does not specify these fields.

I experimented with multiple options that would make easier the writing of JsonFormats that allow optional fields and I think the best one is to add specialised field getters in the JsObject class. Then flatMap/map or a for comprehension can be used in the JsonFormat to extract the fields as demonstrated in the test.

@cvrabie
Copy link
Author

cvrabie commented Aug 5, 2013

This can be done better with the decorator pattern. I'll close and re-submit.

@cvrabie cvrabie closed this Aug 5, 2013
@jrudolph
Copy link
Member

jrudolph commented Aug 6, 2013

Actually, there's already a version 2 of spray-json in the work which has support for optional / default parameters. See 7352db2#L5R86

Unfortunately, we currently have no time to finish that work before spray 1.0 is released.

@cvrabie
Copy link
Author

cvrabie commented Aug 6, 2013

Is there any way I might help with that? It doesn't look like rocket science in itself though I'm not sure how many dependencies it has.

@cvrabie
Copy link
Author

cvrabie commented Aug 6, 2013

@jrudolph by the way, i refactored it so it doesn't change any of your classes. You can just drop the rich wrapper in a project and you can use it: #64

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.

2 participants