This is the best refactoring tutorial video I've ever seen! You speak so clearly and slowly that I can understand the content even I'm not native in English. It helps me a lot. Thank you very much !!!
As a developer I also like to give the smallest set of operations for methods on the input parameters. The same applies for model classes. For example, if PayEmployee(List, EmployeeModel) method is not intended to modify the list, it is a bad practice to ask for a List in my opinion. There are many interfaces that can be used to reduce the number of available operations: 1) IEnumerable - Use it if you want only to iterate through the sequence only once. Only once, because this allows LINQ queries to be executed multiple times. 2) IReadOnlyCollection - Use it if you want to iterate through multiple times and/or you need to know how many elements are in this collection. 3) IReadOnlyList - Use it in case of IReadOnlyCollection but you also want to access the elements with the indexer-property. 4) ICollection - Similar to IReadOnlyCollection but allows to modify the collection and check if it contains a specific element. 5) IList - Adds indexer-property, ability to insert item to specific index, to the ICollection.
I'm still on some "magic number progam.cs" type exercices but I've liked your video to come back later when I'll need this step by step refactorization tutorial. Thank you Corey, this is great content for getting through C# beginner programming !
So happy to see that you used do... while here! So many of the tutorials I've read or watched over the years say that the do... while loop is useless when it is very useful under the right circumstances.
I found this very helpful and practical. Tim takes the time to explain what he is coding. He helps you think about what you are doing in the moment but helps you understand where you can take the code going forward. Thank you Tim for being such a great teacher! 👏🏼🙏🏼
Thank you! This was a really simple but powerful exercise. Trying to refactor the program and then seeing how you did it (a lot better than me!) was extremely helpful. I learned a lot - thank you for providing the source code and great explanations.
Late to the party, thanks for this! A tip to speedup the renaming of variables: just use F2, it can be done on every occurrence (not only on the declaration)
Him: min 17:00 use while loop for what they are designed for Me: I know we have while, for, do while, for each, and who knows how many other loops. Why do we need so many and what is the difference. What are they purposes in life? I started to self-teaching myself not too long ago. I have a lot to learn. I need to refactor my simple code. This guy is explaining a lot more than I expected. you just earned a subscriber!!!
Premise: This is a great video. It made me rethink about how methods should only do a single job(to be more precise, it made me reconsider the exact scope of "single job"). The finished code looked so clean! I have, however, noticed two problems: 1 - "BillCostumer" is going to send the same email even if "totalHours" is 0. 2 - The code doesn't check for problems within the user's input(i.e: it doesn't check if the user uses only digits-characters when setting the hours of work done).
True. There are a lot of things this code doesn't do. We still have a long way to go to make it production-ready. I just wanted to show off an example of how to refactor, not necessarily make a production-ready application.
@@IAmTimCorey I see. As an example of how to refactor, it is great. Following the example of this video, I've just spent a few hours refactoring an old program I'd made(a TicTacToe console application). From the 300 lines-long program class it was, it turned into 6 different classes and a Main that holds just 12 lines. Thank you!
Great tutorial Tim, glad to see you back and hope you're feeling much better now. The refactoring could be done even more in places, but it's a good start for anyone new to this sort of thing.
29:04 Huh! I would have totally guessed the opposite. When you plant a new lawn, you start with dirt (brown), but when you maintain an existing lawn, you start with grass (green). 😋
Hi Tim. Before anything, I want to mention that I know what I'm about to say is probably out of the scope for the real purpose of this video, and I will probably be criticized for this comment, but... there's one minor (I think) bug within the code. Here goes: being a user input app, if the user says he 'worked for Acme, and ABC' in the same line of input, he will be acknowledged for both companies with that one statement. That being said, I can't describe how much of a great resource your channel is, even after I have went through these videos before over and over. I have been revisiting your channel very often to improve my C#. In the last couple of years this is my 'go-to' video resource on LINQ, database acces, and many of the basic stuff. Keep up the great work! :-)
Hello Tim. A few things I invite your comment on: 1. having more than 10 lines in a method *could* be an indication to break up the methods. 2. scrolling (as you noted) is a large flag to break up the method. 3. doing unit tests seems to force the developer to think (and test) in small testable units. 4. a unit test that takes over 4 lines to set up is a red flag that the tested method needs to be broken up.
I mostly agree with these, although the last one isn't necessarily true. If you have to mock anything, it will be more than four lines of code to set up your unit test and most method depend on other classes, which means they will need mocking.
20:19 Ahhhhh... RELIEF!!! That was bugging me for the longest time, lol. I was wondering when you were going to fix it and struggling to concentrate on your coding, haha.
Great Job:) The List I would encapsulate in a class. In that class you have 2 methods "BillCustomer" and "PayEmployee". This methods than only have 1 parameter and not 2 -> better to read and the new class have one task: Managing the List.
I would have to see this in action. My concern is that it would cause other problems trying to solve this problem. Maybe try it out and let me know the results. Don't forget to change things after you get it all set so that you see how it responds to changes (do you have to change it too or can it weather those changes?
Tim, would you consider producing a followup video on refactoring specifically in reference to XAML? What are possible ways a typical 2000 line long MainWindow.xaml file might be improved? I am not really clear how to break XAML into smaller parts the way we do in C#. I assume a solution might have much to do with making use of custom controls and resource libraries. But how would that be done in practice, specifically as a refactoring of existing code, while requiring only a minimum of extra boilerplate code and overhead? And are there any Visual Studio features or free extensions that help with this process?
I'll add it to the suggestion list. WPF code is a mess and there aren't great ways to make it better. You can make controls but then you have the issue of hunting all over to modify something.
This video basically is a programming crash course. Or at least it feels like one. It goes back forth between multiple topics and is littered with good tips. It is hard to make time map for this one. without taking things out of context. While watching, take notes ;) Still, I think there are a few mayor sections in this tho: 1:01 - application code walk-through 5:02 - where to start with refactoring 10:20 - splitting code in to discrete methods 25:44 - creating a class library (dealing with repeating logic) 49:48 - simulating loading data from database
The list should stay in the UI typically. How we load the list was moved to the class library but the storage of the list probably stays in the UI because that is where it is needed.
i think the one "mistake" you made was deep in the video where you just assumed the employeemodel had a name. this is unnecessary! this assumes their name is the identifier, it could well be an id. and the assumption for the refactoring video is that the original code was not erroneous. also, near the end when dealing with the employee rate, i was really enjoying that you were keeping the use of variables. it could help to name the overtime rate/pay as well, since these seem to be implicit knowledge as well as making the calculation a bit long/messy. i think for business applications it doesnt hurt to be a bit more explicit with naming and making the business logic clearer.
Still working through the video, but at approximately 21:21, you can right-click the variable and select Rename from any use of the variable and it will rename properly. I wasn't aware of the Ctrl+. to rename prior to your videos, and just used the right-click -> Rename.
Great video overall. Thank you for creating such a great channel. Even though I was already had some rudimentary familiarity with the idea of refactoring, this helped me better understand particular details of the process. I do wonder, though, why you never used Visual Studio's helpful "Extra Method" refactoring shortcut. I noticed another thing conspicuously absent from this video: most other books and articles talking about refactoring say to start by writing automated unit tests. I am not sure how one would do that for the particular console application you used as an example. But anyway I was hoping to see any good demonstration of the unit testing process being applied to existing code, which might be ugly. (And hopefully shown in a matter compatible with the features of VS 2017 Community Edition, that is, if its unit testing framework is more limited than the full version.)
I'm not entirely comfortable with Extract Method. I use it sometimes but too often I'll then forget that I created it and won't find out until I throw the NotImplementedException in my code. It also moves the method away from where I'm at, which is frustrating. As far as unit testing, that is something I would also do but I didn't want to mix those two together in one video. It makes it less clear, longer, and generally messier to communicate (although that pretty much replicates real-world development). I do have a couple videos on Unit Testing and I'll be implementing unit testing in my new course coming to RU-vid.
Hello, Tim! Do you have a video about how should I orginise my classes in the project/solution (in tearms of putting them into different libraries, folders, moduls and thier naming rules)?
Great lesson, love your channel one of the best for sure. I'm fairly new to code I will add. Just a very minor comment. I'm sure there is a reason, but why do you opt on making an empty console line, in a new line of code? Instead of using an escape key like in the output string?
Good question. There are a couple reasons. First, it is obvious. An empty Console.WriteLine() means a blank line. The escape character after the end of an existing line can be missed. Second, I like consistency. Doing the same things the same way helps avoid errors. When I want to write a line to the console, whether it has text on it or not, I use Console.WriteLine().
IAmTimCorey thx fof that answer. Follow up on that though what's the difference between and ? As I understand is equivalent to carriage return on an old typewriter right? And new line, isn't that the same?
Well, the way C# makes it easy is to provide System.Environment.NewLine so that you don't have to worry about it. If you really wanted to use the switches, here is a post on what they all mean (basically, use /r/n): stackoverflow.com/questions/3091524/what-are-carriage-return-linefeed-and-form-feed
Love the tutorial great work thank you. However, one small niggle about what you're saying about Console.Write* cannot be used in other project types. You simply have to redefine what the standard out is in your solution ( It needs to inherit from streamwriter) and console will direct output to that writer.
I usually make a DisplayText class that has a Print method. Use DI or DJ to have implementation for the print either update a form element's text, stdout, a webpage elem, etc. based on the project.
Not in a refactor. It would only be if you are going to work specifically on that code that you would consider that. A for loop is actually a bit faster than a foreach and changing something like that would lead to a lot of code changes without a lot of benefit.
Hello Tim! I'm not sure that this course is about my following question but you sometimes use string interpolation with format specifier such as "{number:D2}" where we need to remove the white space before and after the interpolation content. I have to admit that I'm a little "psycho-rigid" and would love to see spaces.. So, is there really no way to add spaces. In VBA I often "fake it" with non-breaking space (alt+0160) so I can for example use "type " as variable while Type is a reserved word (no case sensitivity). VS tells me that there can't be trailing whitespace (but non-breaking space is different.. Thanks!
Hi, First of all, I love your videos. You are such a good teacher. I wonder, is there a specific reason to write x instead of for example customer? customers.ForEach(x => BillCustomer(timeSheets, x)); Isn't this easier to read? customers.ForEach(customer => BillCustomer(timeSheets, customer));
You missed the one essential before refactoring - automated tests. After every change, run the tests - nothing broken, fine. What you show, every so often running the program, entering data by hand and checking it is a poor substitute. It doesn’t have to be "proper" unit tests, just adding a bit of code to enter and check the results is enough to show the principle. Of course, first of all you'd have to refactor the code a little to accommodate that...
At 40:55-41:05 of the video Tim says, "You know it's in those couple things you changed." I wish that were true!! But, no. It is not that simple. Or at least with code that has failed to use sufficient defensive programming techniques, it isn't that simple. It is quite possible for changes only made to one part of the code to trigger bugs caused by bad code elsewhere! In fact, I had this happen to me just recently and it was annoyingly time consuming to debug. My code has a method that takes some time to generate a list of strings. The list would be consumed two ways: one portion of the program would use the list as part of some business logic, and another part of the program displayed the list in a listbox full of comboboxes within a WPF GUI. The later needed the comboboxes to add a blank item at the top of the list, so the blank line was added to the list within the getter of the property bound to the itemssource of comboboxes. At that point, after running the program it appeared everything was working okay. I then decided the method that generates the list should be improved so as to avoid unnecessary repeated generation of the exact same list when the method called several times in a row with the same arguments. So I then I proceed to modify just that method, AND NO OTHER PART OF THE CODE, so that it would check if the the arguments were the same as the last time it was called, and if so, just return the same list it returned before (stored in a static variable). At that point, my program started failing in more than one way. The business logic part of the code starting throwing exceptions. And in the GUI, the ComboBoxes now showed multiple blank items at the start of the list, instead of just a single blank item. That was no good! I took an hour trying to figure what could possibly be wrong with the code I changed and found nothing. I only found the bug after setting aside the mindset the bug MUST be in the part of the code of the method I had been working with. I eventually figured out that the problem I had was caused by code outside that method, code which inadvertently all now held references to just one List object. So when a blank line was added to the start of the list for display purposes, it also affected the list used by the business logic (which didn't expect the blank item at the start.) Even worse, it would add another blank line every time the list was used by the GUI. Just a mess. It took some more refactoring to fix those issues. One thing I realized to do was use a ValueConverter in the XAML to add the blank entry needed by the ComboBox without modifying the list. But I probably still should find a way to fix my list-generating method to protect its list from being modified outside of the method. (Is the best way simply make a clone of the list before returning it?) But anyway, I wish the bug was "in those couple things I changed", so I wouldn't have had to study the rest of the code to find it!
I've been there. However, I would say this is more interpretation rather than misstatement. The error was caused by the couple things you changed. It wasn't there before you put the code in and now that you put the code in, the error manifests. The tricky part is that the fix, as you found out, doesn't necessarily get put in the few lines you changed. Sometimes the fix is somewhere else. The key, though, is that you know what caused the issue (your new code). Even if that code is right, it was the cause of the issue. Now you can track down why it caused the issue. If you had changed 100 lines of code, the issue would have been caused by code in one of those 100 lines. That means tracking down what each line does all the way back to the beginning to even identify which line triggered the issue. Then you have to find the actual issue. Much harder.
That was well stated, Tim. I have a followup question related to finding the best solution for fixing the bug I had, and preventing the same sort of issue from happening again. What standard approaches exist for ensuring that objects passed out of a method are not modified when they shouldn't be? One approach I see, might be to only ever pass out a copy of the object that would be returned and not pass out the original. But that could waste enormous amounts of memory if the exact same object needs to be passed out multiple times. I also see as an approach passing out an object that has no public setters, only getters. But that doesn't appear to be the best solution either, especially when passing simple collection objects such an array of strings; is it really wise to store such things in wrapper classes all the time? Would that not hurt efficiency? So what OTHER approaches exist for preventing that sort of error? Does C# have a way to somehow freeze an array or other object so that it is made immutable before it's passed out?
IAmTimCorey Before I forget to ask, is there any chance you can make a video about memory leaks and memory profiling and how to detect and fix them? I have been facing a lot of out of memory issues in the last applications I made.
@@IAmTimCorey do you then go out and change this for the namespace and class as well? It's like learning the difference between value types and reference types. IN C#, if you don't specify the accessibility, it's private. If you don't teach that, it won't prevent people from making that mistake.
