Get the source code for this video for FREE → the-dotnet-weekly.ck.page/refactoring Want to master Clean Architecture? Go here: bit.ly/3PupkOJ Want to unlock Modular Monoliths? Go here: bit.ly/3SXlzSt
This video is a real find for developers, especially young developers. So much useful info about how to write a cleaner code without a need to come up with this in a few years of own experience (like me)
1. Unit testing is a guard to make sure we not refactoring wrongly 2. Convert Boolean statement to expression 3. Convert magical numbers to const variables for better readability 4. Dependency injection for repository classes so that we can change underlying database knowledge if needed in future along. Also to reduce external dependencies for business logic 5. Business logic should not be part of controller or repository hence achieving single responsibility principle benefits. 6. Use Tuple when we want method to return multiple values of different data types and do not want to create a new type for just single place use. 7. Switch expression.
While watching this video, I get ahead and thought of how I'll do it and it's amazing that we're at the same page. Also, everytime I get to read a clean code like this it just makes me happy. Awesome stuff
I can't agree with the improvements for 'IsValid' refactor. While you saved like 10 lines of code, it's become much less readable IMO. I don't see a need for such minimalism, when it impacts your knowledge of what's happening at a glance. The previous implementation was much faster to comprehend than the refactored one. Just my two cents, other than that, great video as always!
Yeah, I was thinking the same. I have done similar code in the past and when I come back to it I find I struggle to read it and fully understand it. For me, I would even consider extracting out each check into its own method and then you can read the method names, not the syntax. You could argue however that's overkill.
I typically care more about what it does (validation) than what's inside. If what's inside is complex, I can give the method a better, more descriptive name. I don't have a problem with reading bool expressions like this, so that's why I prefer this approach.
@@MilanJovanovicTechadding to it, also such methods helps us in debugging by simply using step over as I can understand what it is doing by just its name.
Great video: the way you've approached each refactor and used the IDE to refactor is really useful! I love refactoring and do enjoy the feeling I get when I've finished. I always share these kinds of videos with my colleagues as I find they help people to adopt the refactoring mindset. Thank you for putting this together.
Nice video on how simple refactoring steps can greatly improve code readability and understandability! One small point: as the logic now factored out into CreditLimitCalculator was an implementation detail earlier, I would have preferred keeping it internal to the assembly by making the class internal 😉
Great video Milan, thanks for posting. Just one question: in practice would you be running the tests more often, in fact after virtually every functional change, to ensure you haven't broken anything?
Hey man, video idea for you, what's the best way to version api endpoints for front end apps. Sometimes not all apps get updated on the front end and some of them do so both old and new endpoints have to be used what's the best way to version these. Also how to keep track which APA version is being used by front and applications and I'm talking multiple applications Android iOS web apps
Could you use a smart enums instead of a simple enums for the company type property? The calculate method depends of the type of client so I would encapsulate this method in theus enums
I would add parameterless constructor with default implementation instansiation so it does not break arrange steps in my tests, not sure how it goona look like in this primary constructor thing though
Great video as always Milan. About the Customer "IsUnderCreditLimit" method, I usually prefer the calculated property in these cases where the calculation is easy, fast and doesn't require additional parameters (relies only on existing properties of the entity itself). What do you think?
It just adds additional boilerplate code for almost no performance benefits... There is even a refactoring pattern called "Replace temp with query" which suggest avoiding it. Also, your property may depend on mutable fields, and then you would have to recalculate it. However, if the property represents a state of an object and not a behavior, then it is probably fine
There are a couple of things that still require some "refactor" for example isValid It should have a method for each conditions instead of having all the logic in one global method for example it should be return isEmailValid(args...) && isTheUserMature(args..) && isWhatever() So you avoid need to add comments on your code explaning what the logic is so in case, of someone want to add a regex to validate the email instead they can change one method or for example have a different rule for age depending on the country it will be better besides readability
@@MilanJovanovicTech hehe maybe, but that is part of been explicit about what your code does. Too much for the video will be adding a Validation Class for each Type which will depends on the level of abstraction that your application is having. But as everything this is subjective to the team point of view.
As I see, the Customer entity has 2 properties HasCreditLimit and a nullable CreditLimit. Don't we know implicitly that the Customer has no credit limit if the CreditLimit decimal has been assigned a null value? So what's the added value of the HasCreditLimit property? You will probably say for readability reasons, but other than that?
We could've made HasCreditLimit => CreditLimit?.HasValue But this is mostly a refactoring sample I found and though I'd make a video about it. Not representative of real-world software.
Am I missing something? How the f... a && b && (c || d) && e became a && b && c && d && e? :) In your IsValid method before the last refactoring in return you have allowed email to have either @ OR . characters, but after refactoring you explicitly want email to have @ AND . So either your tests were not covering valid email in previous implementation or previous implementation was not valid and it allowed emails without dot or @ :) It bothered me through entire video until the end because it's really great video and I expected that your tests at the end will fail just to say "I knew it' :D
It was bunch of “||” before and if u flip it then u need to negate using “&”. If u notice carefully there is a green line on that “If”; which the IDE suggests u to negate. He just accepted the suggestion. So there is nothing wrong saying u like it, or saying u didn’t unlike it.
Get the source code for this video for FREE → the-dotnet-weekly.ck.page/refactoring
Want to master Clean Architecture? Go here: bit.ly/3PupkOJ
Want to unlock Modular Monoliths? Go here: bit.ly/3SXlzSt
Hello sir, Could you please make video on static keyword's appropriate use-cases?
Where is source code?
This video is a real find for developers, especially young developers. So much useful info about how to write a cleaner code without a need to come up with this in a few years of own experience (like me)
Glad you enjoyed it!
1. Unit testing is a guard to make sure we not refactoring wrongly
2. Convert Boolean statement to expression
3. Convert magical numbers to const variables for better readability
4. Dependency injection for repository classes so that we can change underlying database knowledge if needed in future along. Also to reduce external dependencies for business logic
5. Business logic should not be part of controller or repository hence achieving single responsibility principle benefits.
6. Use Tuple when we want method to return multiple values of different data types and do not want to create a new type for just single place use.
7. Switch expression.
Nice 👌
While watching this video, I get ahead and thought of how I'll do it and it's amazing that we're at the same page. Also, everytime I get to read a clean code like this it just makes me happy. Awesome stuff
Thanks a lot, great minds think alike they say 😁
I can't agree with the improvements for 'IsValid' refactor. While you saved like 10 lines of code, it's become much less readable IMO. I don't see a need for such minimalism, when it impacts your knowledge of what's happening at a glance. The previous implementation was much faster to comprehend than the refactored one. Just my two cents, other than that, great video as always!
DRY vs DAMP
Yeah, I was thinking the same. I have done similar code in the past and when I come back to it I find I struggle to read it and fully understand it. For me, I would even consider extracting out each check into its own method and then you can read the method names, not the syntax. You could argue however that's overkill.
@@jinx88909this is such a simple method the name alone should be sufficient to know what is happening. I do agree with more complex logic.
I typically care more about what it does (validation) than what's inside. If what's inside is complex, I can give the method a better, more descriptive name. I don't have a problem with reading bool expressions like this, so that's why I prefer this approach.
@@MilanJovanovicTechadding to it, also such methods helps us in debugging by simply using step over as I can understand what it is doing by just its name.
Great video: the way you've approached each refactor and used the IDE to refactor is really useful! I love refactoring and do enjoy the feeling I get when I've finished. I always share these kinds of videos with my colleagues as I find they help people to adopt the refactoring mindset. Thank you for putting this together.
Glad you liked it!
Great video about refactoring. I learnt again more much. Thank you Milan
My pleasure!
This was a refactoring masterclass for sure!
Thank you very much!
Every client is a "very important client"! :)
True!
Great example, thanks for sharing your time and knowledge
Most welcome!
Awesome video, Milan! Can you make a video explaining the strategy pattern?
Good idea
@@MilanJovanovicTechor better still, you coud do a series talking about desing patterns hahaha!
Oh! You have made my day! Thanks!
Happy to help! 😁
Nice video on how simple refactoring steps can greatly improve code readability and understandability! One small point: as the logic now factored out into CreditLimitCalculator was an implementation detail earlier, I would have preferred keeping it internal to the assembly by making the class internal 😉
Good point! I didn't want to bother with that stuff as it's a practice example. But makes a lot of sense on a real-world project.
Great video Milan, thanks for posting. Just one question: in practice would you be running the tests more often, in fact after virtually every functional change, to ensure you haven't broken anything?
Depends on how complex the refactoring is
Didnt know tuples could be used like that amazing!
Tuples are pretty cool
Great video as always!
But how do you solve the dependency on Time.Now in the function youv'e extracted it into?
Create a service like IDateTimeProvider
Awesome video, thanks Milan!
My pleasure!
Great video, thank you so much.
Glad it was helpful!
Nice tutorials, Milan. But please reposition your mike so it captures your voice directly instead of the voice of your room. :)
I'm not hearing what you're hearing 😅
@@MilanJovanovicTech Used to work in the music industry as a sound engineer.
@@ricodomonkos3053 Thought so. Impressive. I'll see what I can do, appreciate the advice very much!
Good one, thanks :)
Glad you liked it!
Can you create a video on strategy pattern?
Will see
@@MilanJovanovicTech thanks. Waiting
Hey man, video idea for you, what's the best way to version api endpoints for front end apps. Sometimes not all apps get updated on the front end and some of them do so both old and new endpoints have to be used what's the best way to version these. Also how to keep track which APA version is being used by front and applications and I'm talking multiple applications Android iOS web apps
I think that should be dictated by the backend, and there are mechanisms to support this
Could you use a smart enums instead of a simple enums for the company type property? The calculate method depends of the type of client so I would encapsulate this method in theus enums
We could've done that, yeah
What your theme for visual Studio?
ReSharper
@@MilanJovanovicTech so Good, tkss
I would add parameterless constructor with default implementation instansiation so it does not break arrange steps in my tests, not sure how it goona look like in this primary constructor thing though
Parameterless ctor can cause silent failures though
@@MilanJovanovicTech can you clarify what kind of failures they can cause
@@NafisKhalilov What is your code is using variables which has got initialized using overloaded constructors. Just a thought. have to test it.
@@vikas4483 I dont know maybe any client code that uses that class. Overloaded ctor needed to not break backwards compatibility
Great video as always Milan. About the Customer "IsUnderCreditLimit" method, I usually prefer the calculated property in these cases where the calculation is easy, fast and doesn't require additional parameters (relies only on existing properties of the entity itself). What do you think?
? show code
@@sunzhang-d9v something like that
bool MyProp => AnotherProp >= 12345 ;
Or
string FullName => “{FirstName} {LastName}”
It just adds additional boilerplate code for almost no performance benefits... There is even a refactoring pattern called "Replace temp with query" which suggest avoiding it.
Also, your property may depend on mutable fields, and then you would have to recalculate it.
However, if the property represents a state of an object and not a behavior, then it is probably fine
Properties are also methods when compiled, so it's the same idea. 😁
There are a couple of things that still require some "refactor" for example isValid
It should have a method for each conditions instead of having all the logic in one global method for example it should be
return isEmailValid(args...) && isTheUserMature(args..) && isWhatever()
So you avoid need to add comments on your code explaning what the logic is so in case, of someone want to add a regex to validate the email instead they can change one method or for example have a different rule for age depending on the country it will be better besides
readability
I figured it would be too much
@@MilanJovanovicTech hehe maybe, but that is part of been explicit about what your code does. Too much for the video will be adding a Validation Class for each Type which will depends on the level of abstraction that your application is having.
But as everything this is subjective to the team point of view.
As I see, the Customer entity has 2 properties HasCreditLimit and a nullable CreditLimit. Don't we know implicitly that the Customer has no credit limit if the CreditLimit decimal has been assigned a null value? So what's the added value of the HasCreditLimit property? You will probably say for readability reasons, but other than that?
We could've made HasCreditLimit => CreditLimit?.HasValue
But this is mostly a refactoring sample I found and though I'd make a video about it. Not representative of real-world software.
The validation code just become too complex to read.
I didn't think so, at the time
you look like a serial refactorer in the thumbnail
😂
Am I missing something? How the f... a && b && (c || d) && e became a && b && c && d && e? :) In your IsValid method before the last refactoring in return you have allowed email to have either @ OR . characters, but after refactoring you explicitly want email to have @ AND . So either your tests were not covering valid email in previous implementation or previous implementation was not valid and it allowed emails without dot or @ :) It bothered me through entire video until the end because it's really great video and I expected that your tests at the end will fail just to say "I knew it' :D
It was bunch of “||” before and if u flip it then u need to negate using “&”. If u notice carefully there is a green line on that “If”; which the IDE suggests u to negate. He just accepted the suggestion. So there is nothing wrong saying u like it, or saying u didn’t unlike it.
Isn't it a !Contains condition at the start?
@@MilanJovanovicTech Ah nema sanse sada da se vracam :). Nebitno. Odlican si malac, samo tako nastavi!
First 😂
Tough competition this time 🤣
Hahaha
Long time no wave 👋