James Taylor tech-test#8
Open
jamestaylordev wants to merge 13 commits into
Open
Conversation
added 13 commits
August 9, 2019 22:27
This file contains hidden or 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
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.
James Taylor: tech-test solution
Notes
Database Scaffolding Scriptssolution folder. I made these scripts idempotent so they can be run over any existing database. Bundled in these scripts is also a migration to addCustomerIdto theOrders.dbo.Orderstable - I made this a safe change, but it doesn't really follow a real world example of how I'd apply a migration since there isn't a data migration to go with it.General Approach
OrderService. This allowed me to make sure the refactoring work I was doing wasn't going to introduce breaking changes.Dtosto be returned/passed into the service layer. I did this to encapsulate the domain model and associated business logic.DALinto contract and implementation projects - this was just to ensure the entire (or parts of) the data layer could be reimplemented without changing the service layer. In real life I would probably use some kind of genericIRepository<T>pattern, but felt that was a bit overkill for this solution.ADO.NETquerying. I considered using EntityFramework, but felt Dapper lent itself better to working with the existing database and this small system.CustomerRepositorywrapping it behind aCustomerRepositoryWrapper- I assumed this static class to be some kind of legacy we couldn't immediately change the signature of so though it was best to abstract it for reimplementation at a later date.Orders.OrderIdtable isn't an auto incrementing primary key - I assumed this was by design so kept it this way - hence a bit of convoluted logic to generate OrderIds in the integration tests.AutomapperCastleWindsorfor IoC - this is only used in the integration tests so is maybe slightly overkill, but I felt it kept them cleaner.