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 bijection-hbase module #135

Merged
merged 2 commits into from
Jul 12, 2013
Merged

Conversation

MansurAshraf
Copy link
Contributor

Provides various HBase specific Bijections by wrapping org.apache.hadoop.hbase.util.Bytes

@MansurAshraf
Copy link
Contributor Author

@johnynek need some love on this pull request :-)

@johnynek
Copy link
Collaborator

Looks really nice. I'll look carefully tomorrow and merge if all is good. Quick question: why bijection with tags and not injection? Doesn't really matter since there is a bijection between the two (nice!) but curious on your thoughts.

@MansurAshraf
Copy link
Contributor Author

As I am integrating bijections more with our codebase I am becoming less of a fan of try/attempt approach. What I have seen in our codebase and I suspect is true for other as well is that Injection will not fail 80% of the time and the time it does fail developer will just re throw the exception anyways. IMO a better approach would have been to just indicate through scaladoc/comments that invert on Injection can fail and let the client decide whether they want to handle the exception by wrapping it in Try themselves or let it propagate upstream if they dont care. The current approach forces the client to deal with Try every single time even even when client knows that injection will never fail

So, to answer your question in a very longwinded way, I used Tag to avoid the baggage that comes with Try :-)

@sritchie
Copy link
Collaborator

FYI, we usually deal with that by writing such functions as proper injections, and leaving it up to the user to use toBijectionUnsafe:

https://github.com/twitter/bijection/blob/develop/bijection-core/src/main/scala/com/twitter/bijection/Injection.scala#L177

to convert to a throwing Bijection.

@johnynek
Copy link
Collaborator

@muhammadashraf your explanation sounds fine to me. This function makes a Bijection from an Injection if someone wants it:

https://github.com/twitter/bijection/blob/develop/bijection-core/src/main/scala/com/twitter/bijection/Bijection.scala#L76

or the unsafe one that Sam mentioned.

Added this issue:
#137

so people can choose the Injection or Rep approach without any extra boilerplate.

johnynek added a commit that referenced this pull request Jul 12, 2013
@johnynek johnynek merged commit 219444a into twitter:develop Jul 12, 2013
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.

3 participants