Better C# - Refactoring

[Music] hey guys welcome back to better c-sharp this time we're going to be talking about a super important topic and that is refactoring so refactoring is the practice of modifying code in such a way that it does not alter its behavior and this lets us change the structure of code and even the algorithms behind it without modifying the external behavior of that code and uh this is a necessary step after getting it working right so you've got some code like your manager is like hey we need we need this thing to do this and then you know when you're done with it go ahead and double the result and then that's that and like you you have some code and you wrote it and you've got it all in one method and it works and you commit it no you don't commit it you have to you have to refactor it like you need to make sure that it follows the solid principles that it is easy to read easy to maintain that it doesn't have any crazy comments that say something that's not even true you get a you got to make it ready to be checked in getting the code working is just the first step so if you don't have a code review at your place you should probably be your own code review on this anyway so this is a necessary necessary step after getting it working too many times code is left after the just got it working stage and you guys know what this code looks like you've seen this code everywhere you've been and you've had to clean up this code by rewriting it every time you have to work on it right um so i've got an example here imagine if a car company they just had the engine sitting on the roof and i realized that's not like applicable that's not probably mechanically very possible but let's say it's let's say it's a battery-powered car and they have like all the battery electronics stuff sitting on the roof of the car to make it easy to swap in and out parts and then let's say that they got it working and they're like okay it's working let's just give it to the customer like no that's not that's not how things work um that's what it's like though when you commit code without refactoring it to get it into a state that's maintainable and if you're doing that stop doing it there's an easy solution just go back look at your code refactor it then check it in um if you have code reviews this should be something you look for like if you're giving other people code reviews don't let them check crap in that's like not not good like don't let them check it in if it's going to be a maintenance nightmare like give it back to them like give give them reasons like these are objective reasons like it's not following the open close principle when we want to modify this to have a new implementation of this thing like it's going to be a nightmare you know it's not you know this comment says it does this but it doesn't do this you know whatever just talk to them about it give them objective reasons why it's not good and let them fix it and you know you'll both be happier once it's fixed if you don't have code reviews and you're responsible like for checking your own code before you commit it check it like don't just get it done and then commit it that's i mean like you know people that are still taking programming classes can get it working like you're if you're you know a software developer being hired by somebody like you're you need to be above that you need to not only get it working like it needs to be good be proud of what you write and part of being proud of what you write is like taking responsibility for the maintenance down the road even if you're not the one that's going to be doing it you need to be nice to the people that follow you and even more so if that person is you like it you know it sucks like when you write some code and you come back to it a year later and you're like oh my gosh what does this even do where does this start and end and then down here we've got another one like if you keep adding stuff onto an existing code base like without refactoring it like you know you've got this thing that works and your manager is like okay we'll just add one to it and you go in there and you add one and then they're like okay well now we want to divide it by three and you go in there and you divide it by three like if you do all this stuff without like refactoring it to make it better ultimately it's going to be this long like convoluted thing if you don't refactor it's just going to get hard to work on this is how projects lose speed and this is how projects ultimately get rewritten is people that go in and they just add some code add some code add some code they don't ever restructure things they don't rename things they don't modify things like they they just add and add an ad and then like before you know it you've got a 6000 line file where you know 2 000 lines of that are duplicated code because you didn't want to change anything that already existed and like you know a big part of that's probably not having unit tests and stuff like that so that's one reason unit tests are important but like even if you don't have unit tests like refactor stuff if your copying and pasting from up here and putting it down here and you're like okay we're gonna do it slightly different now for this type instead of this type like i just don't like refactor it like that needs to be a separate method and you call it with different parameters like just refactor your stuff it's super important like this is what separates good developers from crappy developers like you want to be a good developer you need to make your code good before you check it in anybody can make it work a monkey can make it work you want to make it good um i guess i'm a little bit passionate about this just now so it's just it's something i'm passionate about i want i want to work on good code i want to keep good code refactoring is something i'm actually very happy to do myself so like you know when i see code that's bad like i just i can't leave it that way i have to fix it and you know does that slow me down on current tasks yes but it speeds up future tasks and those tasks might not even be mine like what if you know a developer that's not as familiar with the code comes behind me and has to make modifications since these things are refactored it's going to be that much easier for them to understand it than this 6000 line monstrosity right so you got to take care of your code so i've got some examples here probably the one that i see the most is methods that do too many things and they do too many things at different layers of abstraction right so like this thing is supposed to get account of words from something and you see something the definition of it up here right um so it's got an id and two strings and we're trying to get a word count out of it uh and we're supposed to get this thing by id right so like this method it just does too many things right um and so like it's constructing something it's going and getting something first part from a file the second part from some web server and then it's like doing this logic in here and then it looks like maybe this got copied and pasted down here or something to add more of it like maybe they only wanted the first part at some point now they want both um this is like the methods that are like this end up being nightmares like this one's fairly simple it's only you know however many lines how many lines is this this is uh [Music] let's see 36 lines you see down there at the bottom right like this one's fairly benign but like already it's more than a page and if somebody wants to modify something here the way it's structured right now they're just gonna add to it like they're not gonna make this any better than it is so it's it's up to you when you do it to make it better and really what this should be is like you know all of this should be you know maybe one part of a method right and so like you know maybe it's called get something by id right and then this part is like you know we're getting the counts of the words right so but you can see like these two sections of code so like from here to here and from here to here these are very similar pieces of code the only thing that's changing are the first part so this should also be a method that we can extract right so you know something like that and maybe we don't want to take in these guys but you know maybe we want this to be something like get word out or part or string or whatever you know and so this could be record dot first part and then delete so there oops yeah yeah sorry um and then maybe in here we just want to bring a part whatever so part that length or whatever so and then part right and then we'll do something like word count zero and so now up here this is like that and then we can do the same thing right here word count plus equals get word count for part and then record that second part right and then if you look at this like this is this is way better this reads exactly like the way you would think right and what you'll notice is like what we did is we took some code that had a comment above it right add up the words in each part by looking for spaces like any time you see a comment that's like describing what the code does below it a lot of times that's like a red flag that should be like this code below it should be abstracted and that comment should be the name of the method right so like you know we didn't do it this way but like if this was right here and this said you know get words for part two like that's what we should name the method get words for part and passing part two right so uh so yeah word count possible get word count the part and record that part too so now but this thing does like this reads the way it should read and then even get something by id like we're doing like this i would probably let this one slide but really these two should also be methods right like i would think like because there are different levels of abstraction like at one level we're creating our something and then as another we're like reading files and reading http requests right so like that's not these are a different level so i would probably have tracked these out too so you know maybe something like you see the comment get the first part from file right so let's extract the method and you know name this thing right let's [Music] get first part right and then you know we just pass in the id and say record dot that's part equals that's thing and there we go so now we've got the first part here and this doesn't need to be void it needs to be spring and this just needs to change change to the equals return okay right so that's basically we don't need this comment anymore because the code says what it does and we're going to talk more about comments in a minute let's go ahead and do the same thing we're just going to extract another method here um we're going to get rid of that and just say record that second part equals this thing let's rename this to get second part right and we'll say change to the equals return and this needs to be a string and this didn't get picked up second part and there we go so now this like already reads way better right like all of these things are at the same level of attraction if that makes sense like we're saying like get first part get second part where we're like building something we're not like opening a file anymore you know what i mean and so what you'll notice is also there's another thing i said up here that a method should contain one level of abstraction and that level should be exactly one level below the name of the method so like get kind of words everything in here should be one level of abstraction below get count of words so i get kind of words is like a thing that you can do but then each of these things needs to be just one level below that like get something and then count the words you know what i mean so hopefully that makes sense let's go look at [Music] data mover this one's a fairly simple one it does too many things so basically this is violating the single responsibility principle this should be two different classes like we should have some data loader and some data center instead of doing two things in one uh you know we would probably want to extract an interface for this thing um and then you know we probably want to move this to like another interface right and so like it just you would want these to be on two different things because if you wanted to change how you load data you're going to be messing with the same code that sends data and that's not good so this is you know single responsibility principle so that's a fairly easy one to talk about um let's talk a little more about comments uh comments are generally a bad thing in code in my opinion they should only really ever tell you like why you do something not how you do something and so like and even then like you have to be careful because they can lie to you and so you guys might see this right here like you know we should return early at the file name is test and that's not what this code does right like these are swapped this should be not equals and so like the comments can just lie straight to your face and the thing is like somebody might this code might have been correct at one point somebody came in here and maybe changed this to a double equals because the logic was wrong and now the comments out of date and nobody fixes comments like that's just not something that happens comments are like a color that you already ignore and like they don't get compiled with the code so they can't be checked unless you're using rust which you know that's cool um but like the the language in a comment cannot be semantically checked right so comments in general are just bad you should just have code that likes does what it says and you know have your code say what it does another thing on that note is something like this you see this a lot in older code bases that were eventually moved into source control that were not originally in source control but like any time like you see something like this just delete it seriously this is what source control is for like you need to have commit messages that describe what your changes are we're going to do a video on source control and like best practices on that very soon but this should definitely be something that's in source control and not not something that's in a comment because like the comments lying added one to the sum that's not what happened like the sum is adding three somebody came in here after td and added two more to it right and they didn't update the comment because why would they the comment doesn't get compiled like it doesn't there's no compilation error for a bad comment like this so comments are bad just don't have them um and on that note if you have commented code like if this were like this delete that too get that out of here like that code all it does is serve to distract and like then when it's more cognitive load when you're coming through here and you're like why why is this like this why is this commented out did how did that commented out code ever work does it matter that it's commented out was it an accident like just delete it if you see it delete it check it back in like don't let it come back like let this code die um let's see we're gonna go on to the next thing uh this is names names are super important i'm gonna read this but you guys can read it as well names are incredibly important the example below is a pretty clear example of why that is the case even though the logic is simple even without the names it doesn't show intent having the names shows intent and i'm sure there's other plenty of other logical problems that this class could solve than the one we chose to use it for so i'm going to give you guys an example we've got this class a we get this redominant y we're taking in an n we're assigning it to y like none of this makes any semantic sense right but as soon as you change this to be like age checker right and then like you know uh minimum age and this right here to minimum age and this right here to you know has id and this right here to uh age of person then we can name this can purchase alcohol or something like that right like did that work okay it did work like now this code shows intent but the interesting thing is like without the names it doesn't show intense so if you're coming through code and you have if you see bad names in the code rename things like give people better hints like don't let people get away with bad names and code rename them it's it's up to you like clean up clean up the code uh and in fact like this what i was mentioning with the could solve more than the problem like what if this was like you know something like uh redline indicator right and this is like you know uh minimum rpm and you know same thing here minimum rpm and then you know engine is on and you know current rpm right whoops like this this describes a completely different problem with the exact same logic right and so the names are super important to give you info about what your code is actually talking about because otherwise it could be talking about anything this could be talking about you know selling alcohol um this needs to be changed good show red line indicator right like this could be talking about red lines in an engine it could be talking about selling alcohol to an individual without id or with id um like it just without good names there's literally no meaning at all so name things well don't let people get away with bad names if you see bad names in code rename them all right um let's go to the last one that i have here uh because we're getting pretty long um so you should not have magic constants and strings like around your code base like this right here this one right here so logic and programs that compare magic constants or magic strings as a recipe for hurt when those constants and strings inevitably change and they will change or the reason you use them will change or like things about them will change that cause you to have to look around your code base for the constant one and you don't want to be looking around your code base for the constant one it's not fun um or strings for that matter you know and like you know this is not the best example of this but like this should not be one this should be it should have a name give these things a name like and this comment like without this comment this one means nothing like this does not mean anything if this comment were deleted what if you know somebody came in here and changed it to another number because they're like okay well we don't want to react on the ui layer we want to react on the sub ui layer because we have a minion menu open or something right like so they change this to a 2 and the comment ends up staying the same like all of these things work together to make your code very annoying to work with so like don't have magic constants like this should be something like uh scene layer dot ui right and then you can just go ahead and define this thing uh you can you know if you want it to be there's a couple ways you could do it you either static like ant like public static ant uh ui equals one right so that's one way you could do it but like i would probably use an enumeration for this right so public and then uh special layers and then you know something like ui equals one right and then down here you can just go to the end special layers.ui right now this code and well that part is annoying but there's ways around that as well we're just gonna cast it to a net for now um for this example but like this has way more semantic meaning than just having a one there right um and that that's kind of what i want to get at i don't know if that will fix it i think you have to have like an implicit bomber so we'll just keep it casted doing it for now we'll we'll talk about implicit and explicit casts in another uh in another video so i think that is about all i had um yeah that looks like it and my dog is starting to whine now so he probably wants out so i'm gonna go ahead and wrap up if you guys like uh the stuff that i've kind of preached about here give me a thumbs up and give me a like and maybe subscribe if you want to hear more of this stuff uh there will definitely be more videos coming so that's all i got i hope you guys have a wonderful day and thanks for watching

Channel: Tony Dwire

Views: 1,539

Rating: 4.6923075 out of 5

Keywords: development, programming, C#, .NET, dotnet, .NET Core, dotnetcore, dotnet core, refactor, refactoring, comments, names, srp, single responsibility principle, solid, solid principles, s.o.l.i.d., extract method

Id: Eat9nM9TU4o

Length: 23min 25sec (1405 seconds)

Published: Thu Nov 26 2020