irufus / gdax-java

Java based wrapper for Coinbase Pro (Formerly known as GDAX API and Coinbase Exchange API)
MIT License
177 stars 131 forks source link

Split gdax-java into library and client #56

Open irufus opened 5 years ago

irufus commented 5 years ago

There are three separate projects within this repo.

robevansuk commented 4 years ago

Attempting to break this apart this evening into a multi project build - will publish any commits here. Might not get all the way through but certainly feasible to break it apart.

robevansuk commented 4 years ago

https://github.com/irufus/gdax-java/tree/%2356_Split_gdax-java_into_library_and_client branch here (Work in progress)

stokito commented 4 years ago

It also would be nice to split feed API. For example I need only it in my project

robevansuk commented 4 years ago

I have a working branch here: https://github.com/irufus/gdax-java/tree/%2356_Split_gdax-java_into_library_and_client for splitting the GUI and API apart. I haven't looked at it in a while but I think (if I remember correctly) that it removes some spring boot dependency from the lib part of the codebase and puts it on the desktop side of things. It may be useful to try get this merged as a major version some day soon. I probably won't get around to it any time soon but worth a look.

stokito commented 4 years ago

please take alook on my PR https://github.com/irufus/gdax-java/pull/60 which also have some dependencies cleanup

robevansuk commented 4 years ago

reviewed - just one item I can suggest. Good to see some library bumps

irufus commented 4 years ago

I will etch out some time this weekend to review

irufus commented 4 years ago

@robevansuk Does @stokito 's changes address similar things you were fixing in that branch?

robevansuk commented 4 years ago

I have a huge commit but somethings broken on the live orderbook which I've been looking at for the past week or two... I haven't managed to fix it yet but am working on this branch https://github.com/irufus/gdax-java/pull/62

robevansuk commented 4 years ago

Once this all works, we can then look at "publish to maven" plugins for the module that contains the library code

robevansuk commented 4 years ago

I found the problems and am in the process of fixing them. in a nut shell, the new JSON parsing doesn't deserialise the type field for websocket feed messages. This breaks the live orderbook which depends on this information. I'll repair the deserialisation code and test it again. If all works this can be merged and more incremental changes used to update the repo.

stokito commented 4 years ago

Yes, Jackson by default don't populate the type property. The logic still works because then we'll use class info itself and we don't need the type. If you want the type to be populated then we can add a flag into the Jackson annotation. But IMHO it would be better just to remove the field for two reasons:

  1. Performance consideration: the string field takes quite a lot of memory and it's deserialization wastes resources.
  2. Code stability: To avoid misusing of the field e.g. in if msg.type == 'MATCH', which can't be checked by compiler and force developed to use OOP i.e. if msg instance of MessageMatch
robevansuk commented 4 years ago

I've added changes to populate it but in all honesty my exposure to jackson isn't that great. It works for now but I need to look at the live orderbook before I can merge my changes. Once its working I'll merge it and look at simplifying afterwards

robevansuk commented 3 years ago

The GUI now removed. I'll work on publishing the libs to a maven repo next so that the lib can be used more freely