Petersoj / alpaca-java

A Java API for Alpaca, the commission free, algo friendly, stock trading broker. https://alpaca.markets
https://petersoj.github.io/alpaca-java/
MIT License
197 stars 82 forks source link

Use enum more if possible instead of String #94

Closed Caceresenzo closed 3 years ago

Caceresenzo commented 3 years ago

Hi

Is there a way to use a Java enum instead of a String field in such cases? (see below)

https://github.com/Petersoj/alpaca-java/blob/d4e5ac830767a6b69259192c60645433272435a6/schema_json/alpaca/streaming/trade/trade_update.json#L5-L8

Here is what I was using for replacement:

public enum AlpacaEvent {

    /** Sent when an order has been routed to exchanges for execution. */
    NEW,

    /** Sent when your order has been completely filled. */
    FILL,

    /** Sent when a number of shares less than the total remaining quantity on your order has been filled. */
    PARTIAL_FILL,

    /** Sent when your requested cancelation of an order is processed. */
    CANCELED,

    /** Sent when an order has reached the end of its lifespan, as determined by the order’s time in force value. **/
    EXPIRED,

    /** Sent when the order is done executing for the day, and will not receive further updates until the next trading day. */
    DONE_FOR_DAY,

    /** Sent when your requested replacement of an order is processed. */
    REPLACED,

    /** Sent when your order has been rejected. */
    REJECTED,

    /** Sent when the order has been received by Alpaca and routed to the exchanges, but has not yet been accepted for execution. */
    PENDING_NEW,

    /** Sent when your order has been stopped, and a trade is guaranteed for the order, usually at a stated price or better, but has not yet occurred. */
    STOPPED,

    /** Sent when the order is awaiting cancelation. Most cancelations will occur without the order entering this state. */
    PENDING_CANCEL,

    /** Sent when the order is awaiting replacement. */
    PENDING_REPLACE,

    /** Sent when the order has been completed for the day - it is either “filled” or “done_for_day” - but remaining settlement calculations are still pending. */
    CALCULATED,

    /** Sent when the order has been suspended and is not eligible for trading. */
    SUSPENDED,

    /** Sent when the order replace has been rejected. */
    ORDER_REPLACE_REJECTED,

    /** Sent when the order cancel has been rejected. */
    ORDER_CANCEL_REJECTED;

    public static AlpacaEvent of(String text) {
        try {
            return valueOf(text.toUpperCase());
        } catch (IllegalArgumentException exception) {
            return null;
        }
    }

}

Since the enum fields returned by Alpaca's API are in lowercase, I see a big usage of https://github.com/Petersoj/alpaca-java/blob/d4e5ac830767a6b69259192c60645433272435a6/src/main/java/net/jacobpeterson/abstracts/enums/APIName.java#L3-L6

while you can use @SerializedName from GSON? (i am a Jackson user, so i can't really tell you is this the correct way to do it, but take a look at this: https://stackoverflow.com/a/18343576/7292958)

Caceresenzo commented 3 years ago

On the same note, why not using BigDecimal instead of String? (for numeric fields)

Petersoj commented 3 years ago

Is there a way to use a Java enum instead of a String field in such cases? (see below) while you can use @SerializedName from GSON?

The reason I didn't originally use the @SerializedName for those enums was because Alpaca was changing them (along with ActivityType's) which would result in a null value for that field in the generated POJO. Those likely won't be changing too much anymore so I'll convert those fields to their enum equivalents in the schema in the next release.

On the same note, why not using BigDecimal instead of String? (for numeric fields)

Which schemas/fields are you referring too?

Caceresenzo commented 3 years ago

The reason I didn't originally use the @SerializedName for those enums was because Alpaca was changing them (along with ActivityType's) which would result in a null value for that field in the generated POJO. Those likely won't be changing too much anymore so I'll convert those fields to their enum equivalents in the schema in the next release.

Ok thanks for telling me

Which schemas/fields are you referring too?

At pretty much any decimal field.

https://github.com/Petersoj/alpaca-java/blob/d4e5ac830767a6b69259192c60645433272435a6/schema_json/alpaca/account/account.json#L21-L24

https://github.com/Petersoj/alpaca-java/blob/d4e5ac830767a6b69259192c60645433272435a6/schema_json/alpaca/order/order.json#L69-L72

https://github.com/Petersoj/alpaca-java/blob/d4e5ac830767a6b69259192c60645433272435a6/schema_json/alpaca/order/order.json#L77-L80

(these are some few exemple)

Petersoj commented 3 years ago

I don't recall the exact reason I decided to use String there, but I believe it is for the following reasons: double was causing some users to get parse exceptions due to overflow as Alpaca/Polygon was sending very large (or small) numbers occasionally and BigDecimal fixes overflow issues, but increases overhead (especially on the market data stream). So I just left it up to the user to either take the string and parse it as a double or BigDecimal themselves. I imagine that double is fine in some places now though, so I can take a look and change it from String to double where appropriate. Hope that clears things up.

Caceresenzo commented 3 years ago

I was expecting that kind of answer for the numeric values.

In contrary, the enum change will reduce overhead :)

Petersoj commented 3 years ago

@Caceresenzo I've added TradeUpdateEvent and ActivityType enums to their associated JSON schemas as seen here on the development branch. I've tested it out on my end, but if you'd like to test it, clone the development branch and run ./gradlew clean build install. I'll integrate into the next release.

Caceresenzo commented 3 years ago

Unfortunately I do not have the time to test it... But I will happily update my code when the update is released :)

Thanks for your work