crazzyghost / alphavantage-java

Fluent Java wrapper for Alpha Vantage API
MIT License
80 stars 35 forks source link

Bug in Asynchronous call for indicators #8

Closed ghost closed 4 years ago

ghost commented 4 years ago

Hello,

Thanks you for great work for this API client. I really like your hard work.

I found one bug when running import for few indicators async as its implemented, one thread is using the same method as other and then issue in parse as it receives data from other indicator to parse. For example i found while debug code, call 1 is made to TimeSeries call 2 is made to BBANDS call 3 is made to STOCHRSI call 4 is made to MACD

then data from STOCHRSI is parsing inside method of MACD Parser.

Can you please help me to find a solution?

may be one option is to not make Async call, and just use Sync call OkHttpClient,

OkHttpClient client = new OkHttpClient.Builder().build(); Request request = new Request.Builder().url(BASE_URL + "/delay/2").build();
Call call = client.newCall(request); Response response = call.execute();

Can you please add new method with Sync call to method. its difficult for me to add this.

Thank you.

crazzyghost commented 4 years ago

@krswin TI think this is happening because of reuse of the entry point classes (TimeSeries, Indicator, Forex, etc). Currently only one instance of each is used. I have pushed a bug fix (v1.3.1) to address this issue. You can take a look here. Support for synchronous calls is already under consideration. Thanks for pointing this out

ghost commented 4 years ago

Thanks for quick fix.

I have found one difference, I have changed, in

class IntraDayRequest

private IntraDayRequest(**Builder** builder) {
    super(builder);
    interval = builder.interval;
    outputSize = builder.outputSize;
}

Your change was :

private IntraDayRequest(IntraDayRequest.Builder builder){ super(builder); this.interval = builder.interval; this.outputSize = builder.outputSize; }

as in other timeseries request, Builder is used.

And now I have tested everything works good. thanks

And thank you for considering adding support for synchronous calls, it would be very useful. Can you please tell me when can I expect new method for synchronous calls? It would be helpful for me as I am running on very slow CPU. And you can also give me some hints how can I implement it, then I will fork it commit and you can check and merge it. thanks

crazzyghost commented 4 years ago

@krswin synchronous support will be implemented after support for sector performances are added (the next version). I'll be closing this issue since it's been fixed. Discussions on the synchronous feature can be moved to another issue.