DuckMan435 / PortfolioViewer

0 stars 0 forks source link

Exception handing? #6

Open apryiomka opened 8 years ago

apryiomka commented 8 years ago

How would you handle exceptions for the portfolio API, for instance, SQL Exceptions?

DuckMan435 commented 8 years ago

I would catch the exception and throw a new instance of HttpResponseException, so that the user would not see all the details involved with the error. It would make sense to have some internal logging so the team can debug the issue.

apryiomka commented 8 years ago

Would you update the code to demonstrate it?

DuckMan435 commented 8 years ago

Yes, I can update my code for all of the issues you have raised. When do you need to have the updated commits by?

apryiomka commented 8 years ago

No rush, take your time. I am looking at the quality, not the timing or quantity, if you need more time to do it right, take more time. If you can achieve the same result with fewer lines of code, it is much better. You don't have to do it for all controllers / actions, i just would want to see the concept.

DuckMan435 commented 8 years ago

Thank you, this week is a bit difficult for me, but I will be able to look at it on Sunday.

apryiomka commented 8 years ago

Still no exception handling?

DuckMan435 commented 8 years ago

I will take care of this tonight, my last commit was focused on bringing calculations server side and unit testing.

DuckMan435 commented 8 years ago

Added Exception Handling and Logging