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

Introduce Guava (and maybe Joda Time)? #57

Closed
gary-rowe opened this issue Feb 20, 2013 · 14 comments
Closed

Introduce Guava (and maybe Joda Time)? #57

gary-rowe opened this issue Feb 20, 2013 · 14 comments
Labels

Comments

@gary-rowe
Copy link
Collaborator

Guava is a Google support library that provides a vast array of fixes to common programming problems.

Although we have a general requirement to be a minimally invasive to downstream clients I think that the addition of Guava would make the internal code more robust, and we may want to consider supporting some of it's constructs in our client-facing APIs.

To get a flavour of what it can do have a look at Optional to remove the need for checking for null all the time. This alone is a compelling reason to adopt it.

@timmolter
Copy link
Member

👍 Sounds good to me. I always wanted to use Guava anyway, but I never had the time to get into it. If you want to introduce it, by all means, go for it. In addition to making the code more robust, I can spy on how you use it for my own education. :)

@mmazi
Copy link
Contributor

mmazi commented Feb 20, 2013

I love Guava! I use it all the time in all my projects. But it's a single monolithic library of 2.2 MB, and I don't know how many dependencies it pulls in (though I think not many, if any).

@mmazi
Copy link
Contributor

mmazi commented Feb 20, 2013

It might also introduce some problems with older versions of Android.. I think this should be checked first.

@timmolter
Copy link
Member

Too bad it's a single monolithic lib. 2.2MB is quite large.

@gary-rowe
Copy link
Collaborator Author

Yeah, I keep thinking of XChange in terms of desktops and forget about the impact of a heavy library.

Guava only brings in a "provided" dependency on findbugs, so is essentially self-contained.

We could look at using the Maven Shade plugin to merge their library with ours at build time. That way only the classes that we actually use would be included in the XChange JAR. It does have the effect of introducing a com.google package parallel to the com.xeiam ones.

@gary-rowe
Copy link
Collaborator Author

Thinking more about this I think we'll have to leave it for the time being. Shading Guava will just cause consumers with a full Guava available to see classpath issues.

It's a shame, but until they can split up the library into smaller modules we'll probably have to skip it.

@timmolter
Copy link
Member

Agreed.

@mmazi
Copy link
Contributor

mmazi commented Feb 20, 2013

Guava might still be useful in tests; we might wanna add it with
test for now, if it makes writing tests easier.

A side note: the same goes for JodaTime. It's very easy to create fixed
dates (eg. 2013-01-20), eg. for test assertions, with JodaTime and
relatively difficult with java.util.Date/Calendar.

On Wed, Feb 20, 2013 at 4:34 PM, Gary Rowe notifications@github.com wrote:

Thinking more about this I think we'll have to leave it for the time
being. Shading Guava will just cause consumers with a full Guava available
to see classpath issues.

It's a shame, but until they can split up the library into smaller modules
we'll probably have to skip it.


Reply to this email directly or view it on GitHubhttps://github.com//issues/57#issuecomment-13837757.

@gary-rowe
Copy link
Collaborator Author

I would recommend that we convert our APIs to use JodaTime if possible. It is far more robust as well as all the extra goodness mentioned by Matija.

It comes in at around 550K which isn't too painful.

@mmazi
Copy link
Contributor

mmazi commented Feb 20, 2013

One of the benefits of using java.util.Date is that jackson provides out-of-the-box conversion from long (unix timestamp) and several String formats in json to java.util.Date. If we used JodaTime everywhere, we'd need to write custom deserializers for the JodaTime classes.

I also think that public APIs should in general use java.util.Date and not JodaTime, for easier interfacing with the users of XChange and other reasons. I see the main benefit of JodaTime as internal utility library (ie., not visible in public API). But I don't know if this brings enough benefit to XChange in production code. (I still think it's useful in tests.)

I think XChange used JodaTime a while ago, but Tim removed it at some point?

@timmolter
Copy link
Member

Yes, I removed it because it was barely used and it couldn't be justified. It was used in a couple places and I easily replaced it with java.util.Date stuff.

@gary-rowe
Copy link
Collaborator Author

I don't think that writing our own Jackson serializer would be too onerous (see http://stackoverflow.com/a/3272244/396747), but Tim's comment about Dates not being well-used might be enough to swing it for me.

Overall, I think we've thrashed these two out. Literally. So I guess we'll be sticking with what we have right now for the foreseeable future. It's a shame, but there are valid reasons which we've now documented.

OK to close?

@timmolter
Copy link
Member

Ok to close.

@mmazi
Copy link
Contributor

mmazi commented Feb 20, 2013

BTW, I've added some custom deserializers recently (for String -> Date and "Yes"/"No" -> Boolean), in case someone needs them (directly or as an example).

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants