bitvavo / java-bitvavo-api

Java wrapper for the Bitvavo API
ISC License
9 stars 13 forks source link

Make code more readable and improve overall code quality #13

Closed wouterdedroog closed 4 months ago

wouterdedroog commented 3 years ago

I've tested my own methods, such as createPostFix (which I've now renamed to createUrlParams) and bodyToJsonString. I've also used some methods in example.java to make sure I didn't accidentally break anything.

Overall I think this is close to the best I can do without overhauling the system completely. I'm not a big fan of returning JSON objects as a response here, but it works as intended.

Perhaps the API could be simplified by using a constructor with separate values instead of supplying a JSONObject. Also, I'm not sure why the rate limit is enforced by sleeping. We could create some custom exceptions to indicate when a rate limit has been reached, instead of freezing a thread. That will give the user more control over the way the API is implemented. I would love to hear your opinion about this.

wouterdedroog commented 3 years ago

I've went ahead and also pushed the proposed fix for #7 to this branch.

@joeri-vv since you're the only active contributor to the bitvavo installation, could you review my pull request?

jord1e commented 2 years ago

@joeri-vv

wouterdedroog commented 2 years ago

Hi @jord1e. Thank you for your feedback. I'm currently not planning to improve this pull request, at least not until someone from the @bitvavo team shows interest in the pull request in its current state.

@lucasbento @joeri-vv are these projects still maintained?

elisaado commented 2 years ago

We have adopted @MrWouterNL's fork and it works flawlessly.