Nick!!! This is really cool! Exactly what I wanted for a couple of months now, but couldn't find anything similar on youtube. Thanks for amazing content! Looking forward to new episodes on these series!
As someone who (I'm embarrassed to admit) has not done unit testing since the early days of NUnit, I was blown away when you typed Substitute.For and things started automatically happening... I've already Googled and found that comes from NSubstitute, so I'll look into it -- but hoping I'll find more on your channel about how to use this mocking library. I'm an older developer who got stuck supporting legacy apps in a conservative organization for years, but finally we have an initiative to rewrite in Dot Net Core, so it's time to get up to date!
Man, you just open my eyes not only on tests, but also entire dependency inversion and practical use of refactoring. Long time i did not have that big of "Aha" moment ; D Just... Thanks!
I'm rather a C++-Guy then C#, but I really enjoy your explanations, because it's also a good collection of examples of clean code principles! Thank you!
Just wanted to add that this video is great and kind of a rare find on youtube and I appreciate it. That said, if a company asked me to refactor their shitty code on an interview, I'd ask for an hourly rate lol. I've had this happen a few times where they were asking how to fix their architecture and it was way too specific for an interview. If a company asks you to do something that takes hours and you're not an employee, that's consulting for free. This specific kind of exercise is borderline and I'd ask for compensation. If they're small problems around 30 mins to an hour and are not business-specific, it's probably okay but don't let companies take advantage of you for free. An exercise like this one would be a massive redflag for me.
In todays market, if you don’t do it, someone else will, so companies (at least in London) don’t care. It is pretty much part of the hiring process for every company around here
Hi Nick! Can you please drop the original project in gitlab? Or may be it is already there and I didn't find it. In this case can you please add a link in the header of the video. I think a lot of people would like to play with the test. And I am definitely looking forward next your videos. The channel is awesome! Well done!
Great video - but please tell you're going to rename the "firname" parameter - I just can't focus on anything else. Personnaly I would have moved the UserService class in the Services directory without changing it's namespace - it's seems one step closer to the target.
Yeah the firname rename will come in the 2nd video. Yeah that's another way to go about UserService but I personally hate it when the namespace doesn't reflect the folder structure so I left it there. I will address it in the 2nd video as well.
Actually, regarding being backwards compat., method signature of public method includes the name of the parameters. It is a bit far fetched, but reflection legacy code that depends on firname would break.
@@ryan-heath You are completely right and it would definately be a problem if the harness project was using named arguments but since it doesn't we can change it safely. That being said, thanks for the awesome comment because it allows me to address it in the part 2. It could 100% be a question that the interviewer asked. Sort of like, "You renamed the parameter but now you might have broken a consumer using named arguments" which is a fair point.
This is what pair coding delivers when interactive and this is how you'd sound and come across when you have the Experience too easily missed that that he's doing is from a developed skillset that comes with practice. you'd think you're taht good because you can follow it until put upon to do the same and explain what in your mind is the obvious. explaining the obvious the the real work, to explain what you already know well, not neccessarily to crack a problem you are encountering for the first time. yeah?
I would initialize mocks inside test methods - then you have more control what is going on, independent and avoiding situation where one setup can override another.
You never run in this issue with xUnit. They will always run individually and isolated unless specified differently, and NSubstitude will retroactively update the proxy classes.
32:08 I don't code in .NET but I assume that "" and null are handled different for the "Theory" Testcases, as such aren't you missing the "Theory" InlineData for the null checks on firname and surname? Perhaps I'm a bit nitpicky though.
Hi I wonder why you don't create repository each time in the service class? Is each repository opening a new connection to db and has its own context? Thank you! Another question is when you are writing test, is each time you run the test it is going to actually writing to db?
Great video! Sorry if I'm mistaken but haven't you just introduced a memory leak? Removing the using statements locally without implementing IDisposable in UserService is should be it. Of course you cannot implement it, because of the constraints regarding to the Program.cs.
This is a misconception. We actually made it better. By keeping it as a singleton and not disposing, but instead reusing it and subsequently the HttpClient, not only there won't be a memory leak but performance will be better.
@@nickchapsas I see you point but only inside AddUser. In Program.cs, after calling it you still haven't disposed the HttpClient even if you won't use it afterwards. Isn't that a problem?
No not at all. The client will remain alive for as long as the UserService will which is what we want. This is a case of using the using statement for the wrong reasons at least with what we know today. What I didn't mention is that this is a .NET Framework 4.0 project originally. You would be right btw if this was a memory stream instead of a http client.
My only concern with this is that you're refactoring is changing things specifically for testing purposes. With this, every single class will always implement an interface. That seems excessive.
Implementing the Dependency Injection part is where am having a problem, have a webforms project (.net framework 4.8) that I want to do that for. It's layered and am thinking if I can do that just in the application layer. It'll help a lot with testing. Following this, it appears not to resolve. Can you help with another video?
In school we actually learned to name the test methods `Method_When_Should()`... This way you have the result of the operation at the end. What to you should be the standard and why? Is it arbitrary or does it have any advantages I'm missing? Thanks!
I don’t like When Should and I’ve never seen it in the real world either. I use Method should when because it reads like a sentence that a human would write. Try to read what you wrote. It just doesn’t read like English
@@nickchapsas Example from own code I wrote once for a unit test for the LoginController: `Login_ValidationSuccess_ShouldReturnToken()`. (This was using fluent validators) This seems pretty readable to me. I feel like this way it's more like "given A the system should do B". Of course, it's a preference, but feel free to provide your opinion. Convention is important after all.
@@DanteDeRuwe I would prefer something that reads like this: The token should be returned when the validation was successful. This would be written as Login_ShouldReturnAToken_WhenValidationWasSuccessful. I find that this reads better than Login_ValidationSuccess_ShouldReturnToken
Absolutely not. For someone who has been programming for only a month, just focus on baby steps and take it easy. Topics like the one in the video will come later
Isn't ISystemClock an interface in Microsoft.AspNetCore.Authentication? This is a .NET Framework 4.0 legacy project (migrated to 5.0). Even if it wasn't, it has a DateTimeOffset and it is only providing UtcNow, but this project is using DateTime.Now, not Utc so it wouldn't make sense, logicwise.
in the UserServiceTests class, in the constructor, you use "new UserValidator(_dateTimeProvider)", can you use an instance generated with Substitute.For() instead ?
Haven't ever read any books for C#. It's mostly by googling to get problems solved and for the most advanced stuff it's usually a Jon Skeet blog or a MS doc.
10:55 here u might want correct what you are saying, i dont understand passing date of now and subtracting it from now will eventually fail rather it wud give same result everytime ...
Let's say that you don't mock it and you leave it as it is so DateTime.Now. Now you write a test that uses 19/02/2021 as the date of the user. This test validates that if you are 21 or less then you can't use the service. The test will pass if run today because you are 21 and you are checking for a failure. Now let's say that you run the test in 1 year's time. Now the test will fail because since a year passed the check will give you an age of 22 and the test will fail because you are over 21 and creation will succeed.
Shit this looks scary for noob programmer like me. Started my first job for 6 month as junior backand developer C# .net framework and sql database. Just doing CRUD operation most of mine time and writing sql store procedure and table.
Don't worry. Tests like this are designed to see how far you can go and there are different expectations from different engineer levels. What I'm showing here is what would be expected by a senior engineer. A junior engineer would pass this test with way less requirements. Keep in mind that in junior engineers we are looking for potential not skill.
So far 10% of your sub count have watched this video. 10% of the people who have watched the video have liked the video 20% of the people who liked the video commented. It smells of Fibonacci..Until you realise I fudged the numbers because they were all pretty close.
If you have an interview coding test and you want me to create a video solving it, please message me on social media or add me as a collaborator in the GitHub repo that has the code and the test description. Make sure you didn’t sign confidentiality or an NDA.
Zero dislike tells everything, absolutely loved it :) please do more of these . I would love to see if we can do a 30 day challenge ( spend 30 mins everyday youtube Live and build a solution from requirements all the way to production ready). Hit like to convince nick in doing this.
SUT = System Under Testing, in case you are wondering! It's an olddd term that comes from old ITSQB (International Software Testing Qualifications Board) specs.
We have therefore named it "Subject" to make it clear that you test something of arbitrary granularity, and need not be an actual "System" in the original meaning.
Your first question before refactoring any code is why are we touching code that is "sound" and is not causing any issues. The easiest way to introduce bugs is to modify something that is working to "make it better". Normally I would reject a PR which has unnecessary changes unless the developer can justify the change.
Nearly a week ago i got that as a test task from 'X' IT company for junior c# dev position. I failed cause of not using DI....If i watched ur video before, bruh i think i would have a job with x2 higher salary than i have at the moment But big thank you for showing what DI is really being used for
So to summarize, before you can modify the code, you need to test it. But before you can test the code, you need to modify it. Guess I'm fucked then.. 😂
Great video as always, even for advanced developers. What bothered me the whole time, however, was that "firname" typo. My inner Monk was thinking the whole time, "please correct that first". 😉
Ok guys, understand that if this seems easy to you and obvious...then you're qualified for the job. Everyone makes the sins he is fixing in this code when they're self-taught and don't know any better, or have awareness of best practices.
I have a question what to do if in your task you also can't edit clientRepository so you can add to it an interface, also a proxy class with interface is a solution?
Is the legacy project is available somewhere to be downloaded ? I'd be curious to send to try the refactoring exercice myself and even send it to my collegues :)
This version of the test is is available to my Patreons but if you do some snooping around on GitHub you will find other versions of this test as well.
Your UserService class is about 100 lines but the code to test it all is over 130 lines. This means you’re writing more code in the test class than the functionality it is testing. In my 20 year career of being in the real world of deadlines and having to deliver software in companies, I have found there is never the time to write the code itself as well as a huge battery of tests to go with it. Don’t get me wrong, it’s a nice ideal to be able to test every single combination as well as the happy path but in my experience there is just never the time I’m afraid.
My experience differs significantly. In a world where product quality, stability, scalability and performance makes or brakes a company, investing the time to write tests just saves you time that you will spend debuging later. If a company can't see that then you should probably find a better company to work for.
One suggestion I would make is to not group your stuff by "what it is", but by what belongs together for a set of features. That way you open up your folder, and everything you're working on and that might change together is stuck in the same folder.
This only works if your domain is wide enough. I only work with microservices which only focus on one feature, so my vertical slice is my microservice itself. This doesn't apply in this video since this is more of a wider scope service, so I COULD split by feature, but I almost always find it convoluted unless your domain is wide enough
Excellent video! Thanks so much! I do have a question about the IDateTimeProvider. Rather than creating an interface to provide a "current date time" couldn't you just have the birthdate always be 27 (or a variable to modify the age) years earlier? It seems to be more complicated adding an interface when you could just make the birthdate as DateTime.Now and add -27 to the years.
Actually the need for the interface is universal and it isn't specific to this problem. It is a must for unit testing and it's so common that ASP.NET Core now has this built in as the ISystemClock interface. You really should be doing that if you want consistent unit testing.
It's interesting that you write a unit test for the legacy code, because in most situations a refactor of legacy code is required because it's simply not possible or at least very difficult to unit test it without the refactor, so I makes a logical sense to make it testable first as part of the refactor. I guess you just have to be careful you don't break it in the initial refactor to make it testable.
Having a parameter driven DateTime.Now is useful in many way other than just testing. Often a large system will need to be able to act as if it is at another time (month end is a poor example) and this approach to environmental ‘constants’ can save your bacon ( or religiously permissible alternative).
