Lesson 2 of 5

Refactoring Hardcore Skills

Duration: 46:58 Watch on YouTube (external link, opens in new tab)

Markdown version

In this talk we take a long deep look at many different refactoring techniques (external link, opens in new tab), including:

Extract & Override (external link, opens in new tab), Extract Interface (external link, opens in new tab), Instance Delegator, Parametrize Constructor, Add Static Setter, and many more.

In this lesson

Roy Osherove goes through refactoring patterns (external link, opens in new tab), many of them from Michael Feathers' (external link, opens in new tab) book Working Effectively with Legacy Code (external link, opens in new tab): extract interface (external link, opens in new tab) (with a fake (external link, opens in new tab) email sender and a look at inversion of control containers (external link, opens in new tab) such as Microsoft's Unity), extract and override (external link, opens in new tab), parameterized constructor, break out method object, adapt parameter, static setters on singletons, and instance delegator on a static class from BlogEngine.NET. He closes by saying none of these refactorings can be done without acceptance tests (external link, opens in new tab), and that testability and good design (external link, opens in new tab) are not the same thing.

Transcript

6,848 words

[00:03] (external link, opens in new tab) Now the patterns I'm going to show here a lot of them are taken from a book called Working Effectively with Legacy Code (external link, opens in new tab) by Michael Feathers (external link, opens in new tab). How many of you know this book? Okay, more of you should know this book. Okay, this book is worth its weight in gold. Okay, this is one of the books that for me helped to to clear a lot of the techniques that I've been using. So I owe a lot to Michael for for writing this book.

[00:35] (external link, opens in new tab) It's the reason I asked him to write the forward for my unit testing book. So the first refactoring and the easiest refactoring you can make is extract interface (external link, opens in new tab) and a lot of you probably already did it. However, in legacy code (external link, opens in new tab) situation, this is also the safest refactoring because it changes very little in terms of the the structure of the code and the signature of the code being used. And if we talk about legacy called refactoring, we have to consider two parties. We have the party that you're changing.

[01:06] (external link, opens in new tab) That's the class that you hate. But then you have all the classes that already depend on the class that you hate. They will hate you back if you change the signature. So changing the signature is one of the worst or hardest things you could do. Not necessarily worst. We're going to see some patterns for also changing the signature, but in terms of ease, you want to change the sign signature less. Sometimes changing the signature is a great idea, but it takes a long time because there are a lot of people or classes depending on it.

[01:40] (external link, opens in new tab) So extract interface here. Here's a simple example. Let's say that you have a class called storage. Storage takes an email sender as a parameter to the constructor. Or maybe it doesn't even take a parameter. Maybe it just uses new email sender at the beginning. And you want to test that storage actually sends an email when you call save. But the big question is how the hell are you going to do that? Because email sender is a hard dependency and it actually uses the real system.

[02:14] (external link, opens in new tab) There is no way to replace it. So one way to replace things in languages like net and java is to have an interface. So we introduce create a new interface in the system called I email sender with exactly the same signature and we replace the signature of the constructor in a way that doesn't hurt everyone that depends on it because email sender will also implement this interface. So we just add to email sender. Everything else that uses this class will not have to change.

[02:48] (external link, opens in new tab) But we get what's called a seam (external link, opens in new tab) seam. Sea m a seam is something that allows us in our code to test or to replace things. And in this case, having a parameter that's an interface into the constructor allows us to create in our test classes a fake (external link, opens in new tab) email sender. So in our code, in our test code, we could create a new class that implements from I email sender as well.

[03:20] (external link, opens in new tab) However, it will not really send an email. It will just simply implement the interface. And when someone says send email, the send email class will just be an empty class and we'll just save the values. So it's it's a fake email sender. Now we can use this one and send it into storage and storage wouldn't be any wiser. It will just work the same way it did. Production code will work the same way it did. So we refactored the code. So what's the definition of refactoring

[03:59] (external link, opens in new tab) That's why we pay you. Okay. Yes. >> Same functionality another way. >> Same functionality structured another way. That's a great definition. So changing the code without changing the functionality. Perfect. Pay this guy. Now this is important because when we refactor the last thing we want to do is change functionality because that's a very risky thing. We usually don't have tests, but if we had integration tests (external link, opens in new tab), checking the functionality, we might catch it, but it's still a very risky thing to do.

[04:34] (external link, opens in new tab) So, changing functionality is something that we only do in a test-driven (external link, opens in new tab) way. Changing the structure of the code we can do without adding specifically new tests. Here we're changing the structure so that we can add new tests and so that later we can add new functionality. For example, email sender needs to be called twice. But I want to write a test that it's only called that that it should be called twice that fails because it's only called once currently. So for that I will introduce a new interface, create a fake email sender and then I can write a test that says that fake email sender should be called twice but it's called only once.

[05:13] (external link, opens in new tab) Okay. And then this test will fail and then I can add the new functionality which calls it two times. So there is a process here. Here is an example. Class storage takes an email sender. We change the constructor so that now storage doesn't care what is the type of email sender that it has as a private variable and it takes an interface as a parameter. And then in our tests I can basically send in a new fake email sender

[05:44] (external link, opens in new tab) And then call save. And then I can test the result or I can I can assert on something here. This is the fake email sender in my tests. So I can actually do nothing or I can save the message here, put it in a public variable and then assert on it in my test. I can create this on top and then assert against it at the bottom. So I can use it almost like a spy that says, "Hey, go in and tell me what happened to you." This is me using the theme.

[06:17] (external link, opens in new tab) However, when we introduce this interface and what I noticed is that a lot of teams they they they see extract interface and everything, you know, they have a hammer, everything becomes a nail. And so you see these systems that are very very very testable where you have hundreds of interfaces all over the place almost, you know, the worst case is where someone almost created an I integer, you know, but

[06:50] (external link, opens in new tab) But think about it is that this becomes a bigger problem over time. In this case, person logic maybe or storage takes two interfaces. So now in the code, everyone who uses person logic now has to create two objects in know in order to create person logic. That's easy. But what if to create a logger, you also need to have to create another couple of objects because each object has its own dependency. What if to create a logger you need an exception handler and a data writer and to create a validator you need to know to have an I overridable somewhere in a dependency and to create the handler and the overrider you need three new dependencies.

[07:30] (external link, opens in new tab) So now to create this simple person logic you need nine lines of code in your production code just to create this thing. So now you have to create these factory methods. that's okay too. But this is where things like dependency injection (external link, opens in new tab) containers come into play and they help a lot with this problem. So there are a lot of inversion of control containers innet. The three that a lot of people use I see using is structure map unity and ninjage

[08:05] (external link, opens in new tab) May vary. Has anyone here ever used one of these or a different one? Which one have you used? >> Castle. Castle Windsor. That's Castle Windsor is a is an older one that's very widely used, but it's a bit long in the tooth in a way. So, it's getting older in terms of API and the new ones have a better fluent API that to me is nicer and easier to understand. A lot of times, especially since some of them are inherited from Java, you have to configure them via XML and I hate XML.

[08:39] (external link, opens in new tab) So, a lot of the new ones, including structure map and inject and unity, you can configure directly via code with no configuration whatsoever in files. And to me, that's much more maintainable. So here's an example. If we have these two two dependencies, we could create a container. Now, this is a test that uses the Microsoft Unity container. But you you don't necessarily want to use a container in a test or in production code directly.

[09:13] (external link, opens in new tab) You want to usually abstract it away. But this is to show how you use a container. So in this case the Unity container from Microsoft which is an open source project you call register type of I logger set it to fake logger. So if I think about it it's really a smart factory method at the beginning you say look when someone when when someone requires a type and that type needs an i logger give it them a fake logger implementation when someone needs a person validator give them a fake testing valid

[09:46] (external link, opens in new tab) Testing validator. So now if I want to create an instance of person logic b testable which is for my demos I say unity container.resolve and give me an instance of this. I don't say new I just say resolve to the container and the container will take care of everything that's going on in terms of dependencies. And if you've been to one of Mark Seaman's talks he has a tool called autofixure for unit tests. And autofixure in a way

[10:18] (external link, opens in new tab) Works the same way is that it allows you to not think about the signature of the constructor of an object. It will just fill up what's necessary with default fake objects for tests. In this case, this is something that we might use for production. So in production, we wouldn't use fake logger. We would say if anyone in our production code ever wants a logger, they should call resolve or whatever the infrastructure forwards to resolve and say give me something that looks like a logger.

[10:50] (external link, opens in new tab) And if logger has a dependency and the dependency has a dependency etc recursively, this will just work in a single line of code and that saves a lot of typing time and understanding. This also gives you very nice flexibility because if tomorrow you want to add a new dependency into your code, you simply put it in the constructor and if the container is already configured to use that dependency, this type or interface, then you automatically get it.

[11:20] (external link, opens in new tab) You say, "Oh, I need a service configurator." put it in the constructor as a parameter and at runtime magically you have it because someone calls resolve container already knows how to give an iervice something and you don't have to change the signature of anyone that calls you so it does give a lot of flexibility so if you do do a lot of extract interface at some point you might have to start facing the reality of IOC containers

[11:50] (external link, opens in new tab) Now I personally have not used them too I find that when I have a lot of these parameters, I tend to group them all together into a single object. So a common refactoring that I do is when I have three parameters or more, I create a single parameter of an object with those parameters as properties. Okay, that's called parameterized constructor or a create a method object. Not not method object. I I I not I'm I I not remember. I don't remember.

[12:22] (external link, opens in new tab) I not remember. Sorry. Okay. So when you do talk about containers, you shouldn't use them directly in production code because there is that creates another dependency on something and the whole point is that you use containers that you stop wanting dependencies. So it's kind of defeats the purpose. Some people say oh and then it it will be harder to change a container later. I've never seen a project that wants to change a container after a year.

[12:57] (external link, opens in new tab) Okay, the next the next piece of code is a different kind of refactoring. It's a bit more advanced in that it requires a bit more understanding in terms of inheritance etc. Extract and override (external link, opens in new tab). How many people here read my book by the way just so I know. Okay, so you got the book. That's a good first step.

[13:29] (external link, opens in new tab) Okay. So, extract and override is you see all these words written here? Forget about that for a second. Let's I'll show you a demo. Okay. Okay. What should we do?

[14:08] (external link, opens in new tab) Okay. Yes. Yes. Yes. Okay. So, here's an example of extract and override. Extract and override is powerful for almost any situation. That's why I call it extract and override because it's almost too easy to use. At the beginning, it looks a bit convoluted, but when you get used to it, it's very simple. So, here's an example of a method called is login okay. It takes a user and a password

[14:40] (external link, opens in new tab) And it has a dependency on static logger. Now I want to write a test that proves that it calls the logger but logger is static. I cannot replace it and I'm not using a tool like typemach isolator or moles or anything like that. How do I do that? And the act the answer is I cannot without refactoring this code. So how can I refactor this code to test that the static logger was called? I can refactor it into one more level of abstraction.

[15:10] (external link, opens in new tab) And I'm going to say this. If we have this line, I can extract a method. I can make it virtual and I can say I can call it call logger. Okay. However, it's not good enough yet because this method because it's virtual.

[15:42] (external link, opens in new tab) What's going to happen is that in my tests, I will want to override this. That's why it's called extract and override. I'm extracting a method. This method calls directly to the static logger. But I also want to test the variables being sent. So I'm going to extract a parameter here. I'm using, by the way, ReSharper. Who has ReSharper installed? Okay. Who knows more than 50% of the shortcuts in ReSharper? Okay. If you have a tool and you don't know all the stuff that you can do with it,

[16:15] (external link, opens in new tab) That's a waste. Okay. >> probably 90. Okay. So, let's say login requested. I'm going to change it to text. And now I already extracted this method, so I'm just going to use the one I extracted before. But it looks exactly like this. Now you can see that I refactored the code but the functionality has not changed. So if I go back

[16:45] (external link, opens in new tab) To the what the hell if I go back to the usages of the code I can see that we just have this call logger with the variable and that's very good for me because I don't want to change the functionality. However, I've created a scene. So, if I had a scene before where I had an interface I could replace, now I have a virtual method that I can override. That's a different type of scene.

[17:15] (external link, opens in new tab) Okay. So if I go to a class and I say login manager login manager lm with tests manual login manager with statics and I say I create a class called testable login manager with statics. Let's do that all over again.

[17:55] (external link, opens in new tab) So let's say I create a class here. Class testable testable version of login manager. Login manager with statics inherits from login manager with statics. Now this is important. I'm extracting I have to override it somewhere. So what I'm going to do is I'm going to create the exact login manager with statics with only one method being fake.

[18:26] (external link, opens in new tab) To do that I override call logger and I don't call the base. Now what happens is that production code calls logger but logger doesn't do anything. So maybe I can do something here. I can say maybe create a public property and say text written

[19:02] (external link, opens in new tab) Let's do that into a field. Create field and make that into a public. This is what I did. Now this this is all I have to do right now to create a testable version of login manager with statics. Now I have to use it in my test. So I'm going to create a new test here. Test. This is by the way a macro that you can do in re sharper. This is the macro that I use to create my unit tests. If you want to know how I do it, just let me know and I'll show you.

[19:36] (external link, opens in new tab) Okay, one person not enough. and unit of work (external link, opens in new tab) is the thing that I'm testing on. So I'm going to say is login. Okay, this is the entry point (external link, opens in new tab) of my unit test, the public method that starts the unit of work, the use case. And I want to test that it always calls static logger. What else? Now what I'd like to do is to say var lm equals new testable login manager with statics.

[20:14] (external link, opens in new tab) So instead of creating the actual login manager, I create the testable version of it and I can count that the test that the testable version works exactly like the real one because it inherits from it with only a single method turned off. Then I can simply say lm is login okay with a user and a password and assert is not null for lm.ext.

[20:49] (external link, opens in new tab) So I'm actually asserting on a property on the object I'm testing but the property is only specifically from the test domain. So in production this text written thing doesn't exist. this only exists in my test domain. So I created a testable version of this. So again what I did is that I created a virtual method that I can override in a testable version of the same object and then I used the testable version of the object with the logger turned off to check what text was being sent to the logger.

[21:20] (external link, opens in new tab) Okay, it seems complex at first. You have to do it a couple of times. What happens is that once you do it, you get the hint of it and then you can break almost anything with this thing. Almost anything questions. Let's talk about

[21:54] (external link, opens in new tab) Okay just like we extract and override a call to a logger we can also extract and override a call that creates an object. So let's say that we have a dependency on an email that gets created directly in the code. So we have a workflow engine and in the workflow engine we have new transaction manager being created and to be to create it you need a new model reader and an XML store and all that stuff is a bunch of dependencies. It's very very difficult to actually test this and this all happens in the constructor.

[22:29] (external link, opens in new tab) So even creating this class has a dependency on the file system. So it can't really be a unit test and it requires us to have configuration files in the right place and maybe use a lot of app settings to do stuff. So we can take this whole selected piece of code and extract it into a factory method that is virtual that we can override. This will not work in C++ by the way but it works perfectly fine in C. So instead of all this stuff, I created a protected virtual make transaction manager.

[23:04] (external link, opens in new tab) It works exactly the same way. I did not change the functionality in any way. However, it's virtual. So in my tests, I can now create a testable workflow engine. And in the testable workflow engine, I can override making a transaction manager and return a fake one. And now I can maybe assert things on it or recreate conditions. For example, how can you test what happens when your code encounters a memory violation?

[23:40] (external link, opens in new tab) So I can make the fake transaction manager do whatever I want. It's my code. It's in my tests. I can put whatever fields and booleans on top of it. It's my own class. So extract and override works for creating objects. It works for calling objects. It can also work for future objects. So this code hasn't been created yet, but I can control the instances of objects directly created through refactory methods. Can return different objects. I can return the same object.

[24:10] (external link, opens in new tab) I don't I don't really care questions. Okay, here's a simple one. Parameterized constructor. So parameterized constructor is something a lot of us take for granted. But sometimes in legacy code the dependencies are created just like here we have the transaction manager being sent in not not being sent in but created.

[24:42] (external link, opens in new tab) But we could just as well create a parameter for the constructor and make it a dependency that you can send in or inject. So you can create a constructor injection (external link, opens in new tab) pattern here just like we did before. So we can create an interface or just create the class and make it a parameter and then make the class an interface. Here's a one a bit more interesting. Break out the method object. Breakout method object. Let me see.

[25:15] (external link, opens in new tab) Break out method object. Breaking out a method object is what you do when you have a public method that uses a lot of private methods. This is example, by the way, is in C++. I hope you guys can still read it. I know it looks like a bunch of characters swearing in The Simpsons or something, which is the reason I don't like C++ so much.

[25:45] (external link, opens in new tab) However, it's a good example because here we have a class called GDI brush and we have a method called draw and it uses a lot of private methods and it shares the same variables and the same fields with it and that creates a very tough thing to test and the and the class is very very long to test. So instead what we could do is we create a whole new class called renderer and the renderer will take the original class and all the variables that the public method and all the private methods used

[26:18] (external link, opens in new tab) And it will use them as fields in the new class. So, so we create a class just for that one public method and all its private methods because what it does this way it allows us to test just the functionality of that method without interfering with all the other methods in the class. So we so we so we end up with a class that looks like this. We we have this method draw. It actually calls the renderer and it calls draw on the renderer.

[26:50] (external link, opens in new tab) So this is the method that's actually that can actually be tested one and and now and this method has all all the private methods that it needs but none of the other methods from the original class. So it's cleaner. Okay. So if a method uses three or four sub methods in the class and the class is huge, you take one method at a time with all the things it relates to and put them in a separate class and you make all the variables it shares in the class as public fields or private fields in the new class and that and then the original class just calls the new class in a single in a single line of code.

[27:28] (external link, opens in new tab) That makes things much more testable, cleaner to read. Okay, adapt parameter. Let's see if I have Okay, adapt parameter is is one of those signature changing techniques. Now the the signature here has to change if we want to cover one of those solid principles (external link, opens in new tab), solid design principles that you can hear about from Uncle Bob.

[28:07] (external link, opens in new tab) Let me show you the the principle itself. It follows the interface segregation principle and the idea is that no client should be forced to depend on methods that it does not use. So let's say that your method relies on HTTP context. In this case, this is Java, so it uses HTTP server request, but it just might as well be HTTP context. HTTP context has how many methods do you know?

[28:38] (external link, opens in new tab) 117. I have no idea how many, but definitely around that. And here in this class called arm dispatcher it only uses a single method of that class to do its bidding. So what we do is we say okay let's use the interface segregation. We only need to use a single method and that gives us the ability to refactor and wrap it in an interface that only has a single method. So we can create

[29:10] (external link, opens in new tab) An interface called I parameter source which has a method called get parameter for name and I parameter source will use the HTTP serlet but and it will use whatever it needs inside it but it only has this one method and inside it will use whatever serlet method it needs. This is important because this means that we wrap the dependencies of our classes with only the methods that are needed by those classes.

[29:43] (external link, opens in new tab) So it's easier to fake. For example, it's much harder to fake a class or an interface with 100 methods than an interface with a single method. Okay? And if tomorrow we don't rely on HTTP server but on something else, it's very easy to change the production code, but our tests will not break. So the more chatty our interface is, the more likely we will want to to change the signature and say we don't need something with all those 20 methods when we only use three of those. Let's create an interface with only three of those methods or maybe a single one that uses those three

[30:20] (external link, opens in new tab) As an adapter and use that one in the new class in the class that we're testing. And again, if there are any questions, just let me know. This this exercise is it only for testing purposes to do this? >> No, this is not necessarily for testing purposes. This is a basically a design technique although it helps with testing >> but but you use for for instance standard.net objects >> I would >> does it make sense to wrap all these >> does it make sense to wrap?

[30:52] (external link, opens in new tab) Well sometimes it makes sense and sometimes it doesn't. If the interface is very chatty, there is a big chance that tomorrow someone's going to use another extra method just because they can and then you have to fake another method and then you have 50 breaking tests. So in terms of maintainability, it can help you a lot because in terms of maintainability, you wrap this whole thing in an adapter class. Then the adapter class only has a single method or two. So it's much easier for testing. So, but as a general rule, you're going to have a harder time or more brittle tests

[31:25] (external link, opens in new tab) When you use fake objects that have a lot of methods. Doesn't really matter if it's net or not. Okay? Even if it's your own classes and they have 20 methods and they only get used by only two of them get used by your by an object, it usually means something. It means that maybe you don't have a single responsibility principle in those classes. it means that that class works both as something for this class and the other 18 methods work as something different from another class. So it usually means you want to break things up

[31:57] (external link, opens in new tab) But at the very least you want to adapt a class specifically for your class. So it's a single responsibility principle as well in action here. other questions and then then in your test you can create an IP a fake parameter source that you can fake the value to return can create a new fake parameter source set the property to return whatever you want and that gets used here.

[32:35] (external link, opens in new tab) Now static setters there's something that a lot of us are afraid to do. So imagine you have a simple singleton class. Okay, here's a singleton class. Not a very good implementation, but who cares? Singleton is a singleton. Who knows what a singleton is? Okay, for those who don't, a singleton is a class that only has a single instance and it and it it prevents you from creating new instances of that class. So in this case it's message router. If I want to use its instance I would say message router.instance

[33:09] (external link, opens in new tab) Which is a static method that automatically creates a message router and it has a private constructor. So I cannot create it directly. Now in this case the big problem is what happens if message router has a dependency on the file system or configuration files or all that stuff. How can I break those dependencies? How can I create my own fake message router if someone else is using it or I want to test that message router gets called?

[33:39] (external link, opens in new tab) One simple way to do it is to not just have a public getter but also to have a public setter which of course defeats the whole purpose of a singleton which is why a lot of people don't like it but I don't have a problem with it because I'm not religious about these things. However, for those people who are who really really care that and they're afraid that their users of the API will use it wrongly,

[34:10] (external link, opens in new tab) You have to remember two things. First of all, if you are only relying on your API, if unless you're creating an a framework that's going to be used by people all over the world, if your framework is going to be used only by people in your company and you're not sitting with them and doing code reviews (external link, opens in new tab) and you're just relying on your API to tell people how to use your code, they're going to misuse your code anyway. Second of all, one of the nice lessons I've learned from other people is that when you leave things open,

[34:41] (external link, opens in new tab) You're going to get nice surprises of how people use your API so that they use it the way they need to use it instead of the way you designed to use it. It helps them even more. So, if they can override things that you didn't even plan that they can override, that's not necessarily a bad thing. You think it's a bad thing because in your head you're a control freak. But having overridability for things you cannot even imagine now that will happen in two years is something that I really like having. But let's say you still don't want to do it.

[35:12] (external link, opens in new tab) This is what I would do. Instead of having message router instance, I would put a public setter called set testing instance. I would have the word testing in there and I would put the conditional on it. This only gets compiled if the unit test or let's say debug compilation flag is on. And this allows me to only have this during running tests and it doesn't actually get included in production code.

[35:43] (external link, opens in new tab) Simple. This way everyone's happy. I'm happy because I can test this thing. Control freaks are happy because it doesn't exist really in production code questions. Okay, now here is a complex one. Instance delegator. Instance delegator is this is the one place you're going to have a hard time if you get out now. Just so you know,

[36:15] (external link, opens in new tab) Too late. There is there is this pattern that a lot of actual users of ReSharper like to do and that is to let ReSharper dictate whether a method is supposed to be static etc. And so you get all these statics all over the place. So here's an example. Here's a this is from an open-source project called BlogEngine.NET. And BlogEngine.NET net somewhere deep inside it has a static class called manager.

[36:51] (external link, opens in new tab) Why is it static? Nobody knows. What is the static actually mean on classes? Do you know? >> All methods have to be static. Why do all methods have to be static? Nobody knows. So you actually remove room for methods that are not static in this class. We'll talk about why that's bad. And then you have a static send method.

[37:22] (external link, opens in new tab) Okay. Now, now just to explain the the manager has a send method that takes an I publishable. Look, it's an interface everyone. And I publishable is a blog post. And then it has a bunch of stuff like it looks at the blog settings instance. Notice how that's a singleton right there. I want to test the send method. I want to test for example that when I when there are URLs in the item content that I send in then if the blog settings is true then it does something.

[37:56] (external link, opens in new tab) It calls something but everything is static here. I cannot use extract and override. For example, this method here is private static. Now, private static is an example of someone who just extracted a method and it's static by default. But if it was an instance method, I could make it virtual and then I could override it and fake the URLs from the content. I could also if this whole class was not static and this method was an instance method, I could extract this whole thing into a virtual method and say

[38:32] (external link, opens in new tab) Fake that block settings returns false or true by extracting override on this piece of code. But I can't because this method is static. So we cannot use virtual methods. So there's nothing I can do to replace anything. To be able to even start refactoring and replacing things, I have to make this method an instance method. And for that to be an instance method, it has to be a non-static class.

[39:03] (external link, opens in new tab) So in this case, I wouldn't have no trouble of removing the static keyword. It doesn't change the signature of anything. No code that uses this class will break. It doesn't break. This is only an implementation issue of this class and it just hurts our testability (external link, opens in new tab). Let's see if I can go to manager here.

[39:37] (external link, opens in new tab) And what we want to do is to try to refactor this. And so what I'm going to do is I'm going to create a forwarding mechanism into a an instance method. Now remember, one of the worst thing I can do is to change the signature for everyone else who's already using this class. So my primary purpose is to make this testable and still keep the signature.

[40:07] (external link, opens in new tab) And this miracle can happen if I remove the static keyword and I duplicate the method. So I have now two methods called send. Now compiler won't let me do that. So I'm going to say send internal or maybe send instance and this one method

[40:40] (external link, opens in new tab) Is not going to be static. Now this is the original method and I made it non-static. What about the static signature? It's going to be a forwarding mechanism. It's going to say new manager dot send instance of item and item URL. This is the beginning of my refactoring. So I've made the class an instance class. Everyone else who's already using the static method will still be able to use the static method because the functionality has not changed.

[41:18] (external link, opens in new tab) All everything that happens is that now it uses an instance and that instance still uses the same exact methods. Look at this. Right? This is not an easy method to test. But I can take now that it's an instance method, I can start taking advantage of things like extract and override because now this method is running in an instance of a class. So for example, let's say this is a dependency obviously this is a singleton. Let's say I want to make sure without needing a configuration file in my tests

[41:51] (external link, opens in new tab) That enable trackback send is always going to be true. So I can extract and override this stuff into a method. Ctrl Alt M in this case. Make a method make it not static. Make it virtual. And now it looks like this. if enable trackback send. Now in my tests I will have a testable manager class that overrides enable trackback send and instead of this implementation it will just have a field that I set in my test to return true or false also whatever I care.

[42:31] (external link, opens in new tab) So now I have full control. So from being a fully untestable piece of code this has now become a very testable piece of code. And now what about all this stuff? Get URLs from content. This is a private static method. Private static means that nobody else is using it. So there is no harm in me removing the static. There's no harm. Nobody else is using it. Only instance methods

[43:01] (external link, opens in new tab) Which they don't mind using static methods and they're only in this class because this is a private method. So there's no harm in removing the static and making it virtual and making it protected like this. Now this allows me to do more cool stuff like fake that I have two URLs in the content or zero etc. Instead of creating content with URLs that are very very complicated, I can just return whatever fakes that I want.

[43:36] (external link, opens in new tab) So again what I did is that I took an original static method. I created a copy of it. I made the original non-static and the copy of it I simply forwarded to say new class called the non-static version. So now I have an instance delegator. It delegates the call to an instance with exactly the same parameters. And that gives me the flexibility to extract an override or introduce interfaces or whatever I want.

[44:10] (external link, opens in new tab) Questions? Yeah. >> What if there was some kind of member variable inside that class that was static that needed to be static because let's say some data was fetched from the database that you want to make sure it was only fetched once and was reused again and again while restoring your member variable. So what if there was >> what if there was a member variable that was static that you needed in this class >> that you wanted to set from the database but only once and then you would not go to the database anymore retrieve that value from the static so it had to be static.

[44:41] (external link, opens in new tab) >> Mhm. >> Would you have an issue because now you started making things into instances instead of static. So I'm I'm not sure that I understood the the the end result that you're looking for and how we would refactor, but usually with a combination of either extract and override or extract the factory method of the thing that you're trying to get or just having control a public setter on the static member that you can control then you can get around those things. But I would have to look at the exact code. Maybe you can do it after the talk.

[45:12] (external link, opens in new tab) But but you understand the I >> I don't understand the exact issue but I don't want to get into it too much now but we can do it after because it sounds like it's a bit more complicated than this and this is complicated enough. Okay. But I haven't seen yet an issue that I cannot overcome with the combination of these methods. Okay. Other questions. Okay. Let's see.

[45:53] (external link, opens in new tab) Okay so in summary what I want to say is that legacy code is hard alcohol is good however all these stuff that all the refactoring stuff that I just told you about you cannot do them without the acceptance test (external link, opens in new tab) so if there's one thing that you have to start with is not even do the refactoring. Overcome by by by not overruling by bypass all the refactoring and the unit testing stuff at the beginning and start with acceptance testing and see what you end up with.

[46:24] (external link, opens in new tab) You might end up with exactly what you wanted, which is a tested system that you can change without going to the trouble of refactoring the hell out of it later because then you can refactor to whatever you want. Not just for testability, but just for better design. Whatever you want. Testability and better design are not necessarily the same thing. They don't always mean exactly the other, okay? But they sometimes overlap. So don't mistake testability with great design. Okay? Design is a separate skill from writing unit tests.