patriques82/alphavantage4j

Add Foreign exchange

Closed this issue · 8 comments

Needs support for Foreign exchange data

  • Add class ForeignExchange class with single public method CURRENCY_EXCHANGE_RATE in same level as StockTimeSeries and same structure as StockTimeSeries

I will take a look.

Give me a couple of days. ;)

I am working on that but the the code it looks dirty. At this moment is not reusable. The code and the architecture can be improved a lot.

The idea for the wrapper is also a good thing but if you want to create a great application you should organize better the project. At this moment it is not a scalable program.

Anyway I will create the functionality for exchange rate and after that I am thinking to redesign the application with a better design. ;)

Thanks for your feedback, right now the design is being iterated. So I would still say the project is in a immature design phase. You are of course welcome to come with suggestions.

Btw, you can make a PR with your design ideas, or explain them here and we can discuss them. Either way, Im glad for your help. :-)

Ok, first of all I would redesign the packages structure. After that we have to think what we want to do. To reuse code, at this moment there are specific classes to do specific things. This is a problem when you want to add new functionality different from the previous one (the case of CURRENCY_EXCHANGE_RATE or other). In my opinion, it is better to think about a scalable model and after that code.

Another thing, why are you comparing the json copied from the api?

I think it would be better to create an Integration test to check if the api changes (it could take a while but it is just one per functionality). Indeed if you have the integration you can mock the calls to test the most representative things of each task.

I agree with you that the code was not very clean, but my process for designing is to code different solutions and think them through that way, and then refactor. I have only recently started the project so much of the foundation is not there yet. I recently changed much of the code, what do you think of the current design?.

Another thing, why are you comparing the json copied from the api?

The reason is that, since Im iterating different solutions I like to have some regression tests that tells me if I break something. I assume you are thinking about the changeOfAPI test; that test is there only to see that the error handling for a change in the api return the 'correct' error. Although think it is a great idea to have some integration tests to test if changes to the api has been done, I don´t think that should be the focus right now . We should think about the design more and find a good fit, like you say, that is more extensible.

Done. #13

Good work of the other guys.