-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shaun Watson Backend Task #673
Open
ShaunWatson24
wants to merge
13
commits into
MewsSystems:master
Choose a base branch
from
ShaunWatson24:feature/BackendTask
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Shaun Watson Backend Task #673
ShaunWatson24
wants to merge
13
commits into
MewsSystems:master
from
ShaunWatson24:feature/BackendTask
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add initial implementation for ExchangeRateUpdater so that you are able to call the cnb api for a given day and receive exchange rate data for specified currencies.
Add JsonPropertyName Attributes to API Models for Clarity and remove unnecessary using statements
Add some structures logging to the Service and Provider classes to clearly highlight when an issue has occurred with relevant supplementary information where required.
Add Unit Tests for the ExchangeRateService and ExchangeRateProvider Classes.
Added a validFor property to the Exchange Rate Class and add this to the console output so the user is able to determine the validity period of the Exchange Rate
Modify ExchangeRate.cs so that it does not use a constructor for initialization due to number of parameters now required.
Added a stryker config file along with instructions on how to run mutation testing as a better identifier of test quality than simple code coverage.
Added a Helper class which calculates when the next exchange rate data will be available, along with unit tests.
Remove the ToString from the decimal as this is handled by the Console.Writeline Formatting.
Added an additional unit test for the Helper to test the scenario where the current time is exactly at the time of update. This test was added as a result of finding a Mutant using Stryker.
Inject TimeProvider into the Service instead of doing DateTime.UtcNow for consistency with other areas of the solution.
Extract the outputting of the exchange rate information to improve readability of the program.cs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR introduces a solution for the assigned Backend Task to retrieve the daily exchange rate data from the Czech National Bank for a collection of desired currencies and to display this information clearly in a simple console application.
Solution
My primary aim for this solution was to keep it simple and readable, following clean code principles and ensuring the solution would be easy to extend and maintain. I've included appropriate Error Handling and demonstrated the use of Structured Logging (I would probably reduce the quantity of logging in a real world scenario to avoid bloat and reduce costs). The solution consists of an ExchangeRateService (which simply calls the CNB api to get the daily exchange rates) which is injected into the ExchangeRateProvider class (which filters and returns the Exchange Rates). I did not adjust the provided skeleton too much as I felt this was not the focus of the task. Additionally, I added a Helper to calculate when the next exchange rates will be available from the CNB api. Finally, I added some formatting to the console output to make it more human readable in a graph format with appropriate columns (see screenshot).
All classes have appropriate unit tests in the ExchangeRateUpdater.Tests Project which ensure 100% code coverage, although given code coverage is not a reliable metric of test quality I have also added a settings file and instructions on how to run Stryker, which is a mutation testing tool to evaluate the quality of the tests.
Possible Future Improvements
Data Sources
Swagger API: https://api.cnb.cz/cnbapi/swagger-ui.html#/
Additional Context: https://www.cnb.cz/en/financial-markets/foreign-exchange-market/central-bank-exchange-rate-fixing/central-bank-exchange-rate-fixing/index.html
Additional Comments
Example Console Output