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

Java API questionable choice of Timestamp instead of Instant #39

Closed
RobustCoder opened this issue Jul 27, 2018 · 3 comments
Closed

Java API questionable choice of Timestamp instead of Instant #39

RobustCoder opened this issue Jul 27, 2018 · 3 comments

Comments

@RobustCoder
Copy link

The sole Java API class java_rest_coin_api uses an old class, java.sql.Timestamp, to represent all time instances.

That is a strange choice: this API does no interaction with databases at all, so why use a class from java.sql?

Java 8 introduced a new time API that blows away the old Java classes in every way I know of. If you are unfamiliar, check out this introduction..

In java_rest_coin_api, I think that every current use of Timestamp should be replaced by the new Java time API class Instant.

@Svisstack
Copy link
Contributor

I'm actually don't know the logic behind it as we did not maintain contact with the developer which wrote the JAVA SDK, however main question is: this create any issue? is this could be related to your previous issue with the timezone?

@carwilki
Copy link

java.sql.Timstamp is intended for use with jdbc stuff and is really not something that should be used...really ever at least out side of jdbc. since there are already other libraries required to use this then i would think that you can at least use Joda Time or if you only support jdk 1.8+ then you should be using the JSR-310 java.time implementation.

@RobustCoder
Copy link
Author

carwilki: you are correct.

In the recent refactoring of coinapi's Java SDK, java.sql.Timstamp was totally replaced by java.time.Instant.

That change not only fixed the time zone bug reported in #36, it also simplified the code.

That refactoring addressed a bunch of other issues too. Check it out!

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

No branches or pull requests

3 participants