Add CoinbaseRateProvider implementation to address issue #3 #42
Conversation
|
|
||
| private void loadSupportedCurrencies() { | ||
| try { | ||
| HttpClient httpClient = HttpClient.newHttpClient(); |
There was a problem hiding this comment.
We should probably check if the httpClient is an instance of AutoCloseable and if so #close() it.See JDK-8304165
There was a problem hiding this comment.
Thank you for the feedback! After your suggestion, I considered using a try-with-resources construct to close the connection. However, while reviewing this issue, I realized that java.net.http.HttpClient is not supported in JDK 8, which is the version we’re using for this project.
Rather than upgrading the JDK version, we can switch to Apache HttpClient, which is compatible with JDK 8 and provides similar functionality. CloseableHttpClient in Apache HttpClient implements AutoCloseable, allowing us to use the try-with-resources construct for proper resource management.
I've added a new commit with these changes.
| JsonNode dataNode = jsonNode.get("data"); | ||
| dataNode.forEach(node -> supportedCurrencies.add(node.get("id").asText())); | ||
| } catch (IOException | InterruptedException e) { | ||
| log.severe("Failed to load supported currencies from Coinbase API: " + e.getMessage()); |
There was a problem hiding this comment.
The current code throws a MonetaryException, it may be worth considering keeping this behavior.
There was a problem hiding this comment.
Sure! I've kept the MonetaryException behavior as suggested.
2 participants
This PR introduces the
CoinbaseRateProviderclass, addressing the issue raised in issue #3, which highlights the need for newExchangeRateProviderimplementations following the discontinuation of MtGox.This change is