search

damnhandy / Handy-URI-Templates

A Java URI Template processor implementing RFC6570
https://damnhandy.github.io/Handy-URI-Templates/
Other
202 stars 37 forks source link

Replace SimpleDateFormat with Joda Time to Improve Performance #31

Closed damnhandy closed 9 years ago

damnhandy commented 9 years ago

@asaph recently added PR #30, which replaces SimpleDateFormat with JodaTime. I've been reluctant on adding new dependencies as this library is used in many different Java environments such as Android. I'm not sure how Android plays with JodaTime or how folks in the Android space feel about an additional dependency? Curious to hear from @drdamour and others working on Android and other non-Oracle JVMs.

asaph commented 9 years ago

@damnhandy Thanks for your consideration. Although most enterprise users will barely notice the addition of Joda Time (if they don't already have it in their project), you make a very good point about Android clients. I could submit another pull request that mitigates the SimpleDateFormat performance issue by using ThreadLocal without introducing a new dependency. Would you prefer to go that way instead?

drdamour commented 9 years ago

talked to some droid devs on our teams, they are already on Joda time. If you ever dropped java 6 or 7 support (8 forward) you'd want to revisit and remove Joda i think.

asaph commented 9 years ago

@drdamour Agreed. Specifically, Java 8 introduces java.time.format.DateTimeFormatter which mitigates the issues associated with SimpleDateFormat. But using it would require bumping source=1.6 and target=1.6 to source=1.8 and target=1.8, respectively.

damnhandy commented 9 years ago

Bumping the source isn't really an option either as that will definitely cause troubles for Android developers. Since it appears that Joda is not contentious, I'm going to accept the PR and close this issue.

asaph commented 8 years ago

@damnhandy Thank you for merging PR #30. I see that you added this enhancement to the 2.1 milestone. Do you have an estimated release date in mind for v2.1?

damnhandy commented 8 years ago

I'm going to see what I can do about issue #26 first. I have to jog my memory about what the hell I was thinking the VarExpanders. Realistically, we're looking at mid-October.

four01 commented 7 years ago

This has actually caused issues for us. We didn't upgrade to the this version because it introduced joda, but are forced to recently due to a recent bug fix.

We use threetenbp, so including joda as well isn't ideal. If this can be refactored to use a plugin system which can be registered at runtime, that would be ideal. For now we have been forced to fork your project to remove joda.

Thanks for a great library!

drdamour commented 7 years ago

when you say isn't ideal...do you just mean because you don't want another lib?

four01 commented 7 years ago

Sorry for being unclear.

We're using this library in our Android project where method counts matter. We're already using the excellent threetenbp library to handle dates. If we had the option we'd make URI Templates use threeten already because that's what we already have included. Including Joda instead adds too many extra methods.

Please look at http://www.methodscount.com/?lib=com.damnhandy%3Ahandy-uri-templates%3A2.1.6

Thanks.

drdamour commented 7 years ago

damn dalvik!

four01 commented 7 years ago

A simple plugin architecture could resolve this. Or an interface where we can implement our own approach.

In our app, we don't actually use dates as parameters at all. It seems like although converting dates to strings is a nice bonus feature it doesn't seem like it's a core feature of RFC 6570. Maybe this is something that needs to be exposed as user plugins and let the core library handle the basic/primitive cases.

damnhandy commented 7 years ago

Let's continue this discussion in issue #55 since this issue is closed.