Skip to content
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
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ShaunWatson24
Copy link

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.

image

Possible Future Improvements

  • Implement Caching (e.g. using an InMemoryCache) if application was long running.
  • Additional User Functionality (e.g. refreshing the result & outputting the difference to the previous days rates).
  • Consider Czech Public Holidays when calculating the next available exchange rates.

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

  • Atomic Commits I considered atomic commits only after my first commit, however each commit has an appropriate description which tells a story of how I tackled the task.
  • Primary Constructors I have utilized Primary Constructors in this solution, although personally I'm not entirely sold on them yet as they make some classes slightly less readable.

Example Console Output

image

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
@ShaunWatson24 ShaunWatson24 marked this pull request as ready for review January 15, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant