DuckMan435 / PortfolioViewer

0 stars 0 forks source link

Portfolio value is calculated in the UI? #7

Open apryiomka opened 8 years ago

apryiomka commented 8 years ago

What would be the benefit of doing the calculation in the UI? What about unit tests for the portfolio calculation? Do think if we do it on the server side, we can write better unit tests for it?

DuckMan435 commented 8 years ago

I originally was going to do the calculations server side, but thoughts on UX I felt that it would be a better experience to do the work in the browser. Additionally, I was using the yahoo api to retrieve information about the Stock/Fund quotes client side, which swayed me that direction. Looking back, it would probably make more sense to do the work on the server and serve it up to the client afterwards. Also, as you said, we would also be able to write unit tests for it.

apryiomka commented 8 years ago

Yes, you can write better unit tests coverage on the server side as well as you can "hide" the implementation. In this example it might not be an issue, but in real life application you might not always want to expose your business logic to any potential malicious user.

DuckMan435 commented 8 years ago

Yes, I agree. As primarily a back-end engineer, I should have gone with this idea from the start. I must have been too focused on showing both my back-end and front-end skill-set.

DuckMan435 commented 8 years ago

Calculations have been moved to back end.

apryiomka commented 8 years ago

I can see you moved the calculation to the FuncController. Do you think there is a better way or place to do calculation in the app? For example, to calculate stock price you need quantity, current price and stockDevident. Most of those values would be, I would argue all of them should, part of the StockModel. Do you think we can change where we do the calculation to make it a bit more object oriented design rather than functional / procedural design?

DuckMan435 commented 8 years ago

That makes sense. I went with the FuncController because I was pulling current price and stock dividend from the Yahoo API client side. I ended up repeating the process for the other two types for uniformity. I can make the Yahoo API call server side and move everything to the individual models.

DuckMan435 commented 8 years ago

Security calculations moved to the Security Models and removed the FuncController