A Refactoring Dillemma

[00:04] (external link, opens in new tab) Hello everyone and welcome to session number four. Raymond, how are you? >> I'm doing great. >> Cool. So actually, you know, I've been thinking a lot about our code and I'm not really really really happy with it. Although we've definitely made progress, I'm not happy with the design that's slowly emerging. I don't think we're refactoring enough. I don't think the code is is nice enough and maintainable enough as it is. Although it does work, but just the fact that it's working doesn't mean that we should leave it the way it leave it the way it is.

[00:39] (external link, opens in new tab) By the way, we've also installed Mighty Moose just to see how things are going. And so far it looks pretty cool. So every time we're going to save, it's going to run all the affected tests in the background. And it's a free plugin for Visual Studio. It's pretty amazing. So, for example, you can see here the number 12. It means there are 12 tests covering this method right there and they're all passing. There are 14 tests with board

[01:12] (external link, opens in new tab) Cuz 14 tests create board. But I'll tell you what I'm unhappy about. What I'm unhappy about is basically the rules class. specifically there is this in komi mode right there. >> Mhm. >> and this does bother me. I mean this little if thing that's usually a sign that we can we can get rid of it by refactoring to some sort of either a strategy or some sort of chain of responsibility any kind of polymorphism here really would work.

[01:48] (external link, opens in new tab) Are you familiar with the strategy pattern? >> A little bit, but I'm not sure how a strategy would be used here. >> Okay, so the way I'm thinking about this is that the get winner there are currently two strategy strategies or actually three for determining for determining the winner. There is the komi mode strategy. Okay. And that strategy is only should only be active when no moves have been made yet.

[02:21] (external link, opens in new tab) Okay? >> Right? >> And then there is another strategy where moves have been made and and you just look for whoever won some some cells so far, >> right? >> And the third strategy is the last player, >> Okay. and and right now all these three strategies are in the get winner method. So they all work but they're definitely not separated. So the single responsibility principle (external link, opens in new tab) definitely doesn't apply here.

[02:53] (external link, opens in new tab) >> Okay, >> does that make sense? >> Right? >> So the way I think about it is that what we can do is we can slowly refactor this into three strategies and then decide where these strategies get get applied. For example, I would create a class. I would basically copy this whole thing.

[03:24] (external link, opens in new tab) Create a class here. We can move it later. Class komi komi winner strategy. Okay. And that it will inherit from a find winner finding strategy.

[04:04] (external link, opens in new tab) And all it has is a public stone color get winner. >> H. Okay. >> and it takes in a board. And this is all the code that this komi strategy will have since there is no if. It's always white when we're in komi.

[04:34] (external link, opens in new tab) Right. >> Mhm. >> Right. >> and so this code will end up being just for now I'm just refactoring. I will say a new actually no I will say

[05:07] (external link, opens in new tab) Winning yeah new komi control alt space maybe nope That doesn't work for me. That on a map that doesn't work well. Control space. Damn it. Komi winner strategy. Get winner. This

[05:37] (external link, opens in new tab) Actually bored. Okay, obviously this doesn't compile yet. We just need to create the base class here. >> create class. Yeah. And we can make this method abstract.

[06:08] (external link, opens in new tab) Just just make sure first of all let's see what happens. Okay, ran successfully. Six test pass. Okay. >> Okay. >> so we didn't break anything yet. But now I'm just going to refactor this and say pull members up or is it down for the base class? I think it's I think it's up.

[06:38] (external link, opens in new tab) Let's see. >> So now this method will have to be in each winning strategy. And let's save. Okay. Now the strategy has an abstract class. Now it's overriding an abstract class method here.

[07:08] (external link, opens in new tab) >> And I'm thinking that it probably just needs the the board in the in the constructor instead. What do you think? yeah. So we can say is there a No, there is no. So we can say

[07:40] (external link, opens in new tab) Public winner finding strategy or B. Am I missing something? Semicolon. >> but it's not abstract. It's just a constructor. Okay. >> Oh, I see. >> in which case we would just use introduce initialize field B.

[08:17] (external link, opens in new tab) And let's not make it private. We just want to make it protected. So, we can use it in the base class. Mhm. >> Then get winner doesn't need a board B. It just uses it if it needs it. Okay. >> why do I Oh, yeah. Okay. Let's see what happens. Well, I'm not initializing it correctly now. So,

[09:02] (external link, opens in new tab) Hm. Oh, did I miss that in the Yeah, I missed it in the base class. So, create a constructor. awesome. Now everything should pass again. And it didn't. >> Correct. >> It did. Okay,

[09:35] (external link, opens in new tab) Cool. Like I like it every time I save that the test run. Isn't it cool? >> It is cool. >> Okay, so after we factor for the three the the three strategies, we could use like a factory method that determines which strategy to use right now. >> Okay. >> Okay. Or or B we we might not even need this the the the factory mode but let's see let's just see where it leads us. Now this whole thing

[10:07] (external link, opens in new tab) >> is one strategy right? >> Mhm. >> So what we're going to do we're going to factor this We'll create another class

[10:37] (external link, opens in new tab) Class. First cell winner strategy which is a winner finding strategy. What else? Create a constructor. H. >> Oh, that wasn't what I meant.

[11:20] (external link, opens in new tab) And implement members and it needs the rules. >> Sorry. It needs reference to the rules rules object >> actually is fully surrounded by >> either it needs reference to the rules object or it can just we can just move fully surrounded by here. >> Mhm. >> What do you think?

[11:51] (external link, opens in new tab) >> Okay. Yeah, >> we can we'll copy it for now and then we'll see if we can get rid of it. >> I see. Yeah. >> Now, where is it? Why is it not complaining about board? Interesting. Oh, because that's a stat. That's a const. Okay, that's pretty cool. So right now we're just saying this is fully surrounded by

[12:26] (external link, opens in new tab) Like this. And I'm I'm definitely doing this incrementally because I'm changing a lot of stuff. So, I want to make sure I don't break anything. Now, here's the cool thing. Change all board to

[12:59] (external link, opens in new tab) Nice. Okay, let's see what happens. Oh, it's not going to compile yet. Still have a problem here. Yeah, I have one extra. Why is it undo? If public override, this is the public method for int. Oh, we still have to return something, right? >> Mhm.

[13:41] (external link, opens in new tab) >> what do you recommend? >> I recommend putting a variable that that we're going to return and then set it inside the if statements maybe.

[14:15] (external link, opens in new tab) I think it's one above that above that bracket. >> Maybe you need You need to type explicitly. Okay. Sure. Let's make it nullable. Who cares? I just want everything to tell me everything is okay.

[14:47] (external link, opens in new tab) Control S. I think what >> the return type >> oh it has to be a nullable as well. >> Interesting. Yeah, true. And stone color equals no let's set it to empty for now. Okay. >> To pass, right? >> Yeah. We can see that right here at the bottom.

[15:18] (external link, opens in new tab) All right. It just mentioned six tests. I wasn't sure if it mentioned six tests passed or what. >> Well, it only runs the affected tests. >> Yeah. >> What's interesting is that it says that nothing actually used this one >> because we haven't refactored yet to use it. >> Isn't that cool? >> Yeah. >> so it's not really covered because we haven't refactored to use it. >> I see. So here

[15:49] (external link, opens in new tab) We might say check if it's empty and then return. >> Well, the default is empty. I'm not sure. >> Well, you you for your function, if it did not find any, it would return empty. >> If it did not find any that are surrounded, it would be empty. >> True. pools. New first

[16:19] (external link, opens in new tab) Strategy board get winner if result I'm used to reading now. That's why I use that or not equals

[17:02] (external link, opens in new tab) Cool. Cool. Very cool. Why does this say it's still not covered yet? Interesting. >> I think it said that also for the auction. No, the komi winner says that. >> Let's just make sure. Okay. >> Yeah, we really didn't break anything. So that's not making them really comfortable for now.

[17:32] (external link, opens in new tab) >> But the other one was Oh, that was updated. Maybe it just takes a while. I guess >> doing doing in the background. >> Cool. Five me it's yellow risk analysis 44. That's great. Thanks. Okay, so now we almost have all the strategies we need, I think. The current winner is always the last one, right? >> Mhm.

[18:07] (external link, opens in new tab) Where is that being set? Let's see. Control Alt F7. It's always a notify stone added. >> Mhm. >> Okay. So I think what we would do in this case is we would have like default winner strategy, >> right? Okay. >> Or like a like a hard-coded winner strategy in a way, >> right? >>

[18:39] (external link, opens in new tab) What I mean is >> Yeah, like a hard-coded winner strategy every time the last one wins. That's possible, >> you know. If we switch if we switch the two strategies around, >> I think we can get rid of the current winner because the reason we had the current winner was because yeah, but I want to get rid of it last. I want first of all to just replace the current thing because then we would do too many things at the same time.

[19:11] (external link, opens in new tab) >> Okay? >> So, I want to replace the existing structure and then maybe remove some of it. >> Okay? Because the point here is to do it things incrementally. It's part of the exercise as well. But are you saying we could get rid of the whole thing? We don't need a default one. >> Yeah. If if we switch the komi and the fully surrounded part. So let me take over. >> Sure. >> If we where's the two strategies part?

[19:47] (external link, opens in new tab) >> it's above just above at the top. Yeah. >> Okay. >> It's so short now that it's hard to define. >> Yeah. >> I think if we move this my control X okay to here >> and replace >> and I think we could return we can remove this one >> and then you don't need >> actually. No, you're right. It won't never mind. You're right. We still need it because the the komi would be false. You're right.

[20:19] (external link, opens in new tab) Actually this might you might even remove the in komi then I think you can get rid of the in komi the if and komi part and then remove the yeah and try running that >> yeah interesting >> that's nice >> let's make drawer.

[20:50] (external link, opens in new tab) >> Sweet. Now I like this better. >> Oh, cool. Okay. That went nice. So now, now we have to decide I still want to get rid of those ifs cuz we shouldn't need them. >> Okay. Right. Now the way I think about it is that

[21:30] (external link, opens in new tab) Hold on. Let's just make sure. Let's get rid of everything. We don't need in komi mode. We don't need current winner anymore. Let's see. Building projects. And

[22:00] (external link, opens in new tab) Okay, cool. Now we've ran for tests. Actually, we don't need a notify stone added right now anyway. >> Yeah, >> that's pretty cool. But let's leave it for now. We still need to decide what to do, how to get rid of those ifs. And also, I'm going to get I'm going to move these away. Just move to outer scope.

[22:36] (external link, opens in new tab) Will not be accessible then I will make it accessible dude Yeah, that's not going to work. We have to make that let's move that as well. move to another file because we need that in the file anyway.

[23:13] (external link, opens in new tab) Make it public. what did what does that say? Oops. I think it has to do with the fact that we haven't moved those inms here. I don't know. I'm not sure.

[23:47] (external link, opens in new tab) Move that. >> No, I think the one of them is not public. the the komi strategy is not public. Is that the reason why? >> Could be. Let's see. Komi strategy. >> yeah, this will tell us exactly rules.

[24:26] (external link, opens in new tab) Hold on. Call me >> No, but ReSharper also is kind of smart. So, it's telling us cannot access private >> private constants. >> Just make it >> because you moved it out of the class, right? >> Sure. >> So, >> okay. Save everything.

[25:01] (external link, opens in new tab) Okay. Now let's continue. Just move those things away. Move to another file. What's going on? >> So, I think every time you move the file, it does all the mapping over again.

[25:35] (external link, opens in new tab) >> Mhm. >> That's why it's taking a while. >> Well, it doesn't say any test tra. So, what I'm going to do, I'm going to just press alt R. 14 pass. That was faster. And then we have the board again. What else? What did I do? Double click. No, that's not what I meant. Like this.

[26:05] (external link, opens in new tab) No. Let me just make sure I'm putting all everything I need. Okay. We have the board. We have still the komi winner strategy. So let's get rid of that. Function F6 move to another type.

[26:38] (external link, opens in new tab) Actually what I meant is move to another scope. It's awesome. And again file

[27:10] (external link, opens in new tab) Cool. And I'm going to press Alt R just to make sure as well. I'm not sure it helps me if I keep running alt R as well. What does that mean? I wonder bookmark navigate with control 2. Okay. Okay. So now things might start to look a bit more more reasonable if we look at the structure. So what what what I'm going to do is is I'm going to do create a

[27:49] (external link, opens in new tab) Nope, I'm going to do this here. Add there should be an add folder. >> Yeah, I think it might be. Is that one? >> That's the window for you. >> Okay. >> so we call that find winner finding strategies.

[28:20] (external link, opens in new tab) And then basically we have winner. Just move that over here. First cell winner. And here, just to make sure, let's save everything. Okay, cool. So, I think it's beginning to look a bit more interesting now. >> Mhm. >> Any thoughts so far?

[28:52] (external link, opens in new tab) >> I think it's looking better. >> Yeah. >> Okay. >> No way, Team Explorer. No one wants you. Okay. So now let's let's see how we can get rid of these ifs. So one way to get rid of ifs is to is to basically do again polymorphism. That well in in this case I'm not sure

[29:23] (external link, opens in new tab) Actually because it's not a flag at the class level. If it was at the class level, I would say well, we have rules komi rules and we have non-komi rules basically, >> right? >> But in this case, I am not sure. Maybe in this case we we might say just go through all the rules and if the first one that returns a non- empty would work.

[29:59] (external link, opens in new tab) >> Yeah. >> What do you think? >> Yeah. But how would that work? That still sounds like there's still a logical if there. >> but but this but this way the first rule the rules get to decide if they return something or not. So the logic is not in the loop. Okay, >> because a rule gets to decide, well, I'm the one that should be applied now, >> basically. >> And if I and if I don't matter, it just returns stone color empty.

[30:30] (external link, opens in new tab) >> Mhm. >> What do you think? >> Yeah. >> so that's one way to think about it. Another another way is to is to get a like a a factory method for the rules for the winner strategy. Mhm. >> but we still have to see if one of them actually has worked. >> so we wanted to get rid of the komi flag. Right. >> Right. >> So right now we just wanted to re refactor.

[31:02] (external link, opens in new tab) >> So here's here's here's another way that is something that I've I've done in past projects. We can have a fa a field that says public winner finding strategy winner finder. Okay. >> Mhm. >> Equals null. >> I see. I see. And in notify we would basically say that the winner finder

[31:32] (external link, opens in new tab) Equals new first cell winner strategy. >> Wow. >> in board and but by default I'm sure we have a constructor here, right? >> Right. >> Do we? Yeah. No, we don't. >> no, I'm not sure. So by so actually we can just default it to new komi winner strategy

[32:04] (external link, opens in new tab) In board. >> Wow, that's pretty interesting. That's pretty cool. >> Actually, that won't work because board hasn't been assigned yet. So we do need a constructor. Alt insert. Can you run alt insert on your end? >> Yeah, constructor. Yeah. With do we need board? Yeah. We'll do we'll do board later. For now, just just you just just press enter.

[32:37] (external link, opens in new tab) And now the basic rule is that winner finder by default equals new winner strategy.

[33:13] (external link, opens in new tab) Just to make sure we would just say here return Now we just need to replace the calls to a new board. >> You need to change it to null. First this part right here. Which part? >> Here. >> True. >> There you go. >> Okay. And one last fix is is new rules

[33:44] (external link, opens in new tab) With this. 12. >> That's awesome. What just happened? We went from nothing to

[34:16] (external link, opens in new tab) Cool. Let's see. What the hell happened? Let's see. >> I think maybe right click might tell give you more information. >> Yep. Okay. Or not. >> No. No. Forgot what they said. Oh, it say D. D. What does D say? >> I. You just click I and I guess. >> Okay. I >>

[34:54] (external link, opens in new tab) Message object reference not not send instance of an object. So we get a null here at some point. Okay. You know what? I'm going to run the the TestDriven.NET for now because it's easier for me to understand what's going on. >> this is where we're getting a null. Mhm. and add stone.

[35:25] (external link, opens in new tab) Check stones and remove if needed. What's null here? Let's see. >> I think the only thing that could be null is the board, right? It's passing a null board. >> Oh, yeah. because we're not initializing it in the constructor. Yeah, we we definitely have a bug is that the rules is not setting the board. It's just it's just using it. So >> board

[36:04] (external link, opens in new tab) >> interesting. So black doesn't own Okay. Termin winner. Black doesn't own any cell but played once. White should win by komi but we actually got empty. Interesting. That's because

[36:35] (external link, opens in new tab) >> I guess that's because we had before it passed because of actually no I don't know that's a little weird because it should be the logic should be the same as before right >> so in that case what we can do is go back to the place where we changed most of it and say instead of just get winner, we can go back and say return you call me.

[37:10] (external link, opens in new tab) Oh, it's it's because yeah, because we had it by empty. Now, before we we would use the komi if the result was empty, but now we're not using the komi result at all. The komi winning finding strategy, >> are we not? >> It didn't get away. It get it didn't get rid of the >> the what? >> Didn't get rid of If it returns empty, >> but it used to work. That's that's my thing is that we broke it and I'm not sure why. If you're sure why, I'd love to see how you'd fix it.

[37:42] (external link, opens in new tab) >> Umh, >> but for now, I'm just gonna say >> how we fix it. So, the thing the reason we moved it down here Oh, >> yeah. Sorry. >> Is the reason why we set it right here is because we thought, oh, we don't need the komi anymore. But what we had before was >> Yeah, sure. >> If before we had was if result equals empty, right? >> Oh, yeah. And so the bug is that komi might still apply even if there was a play. >> Right.

[38:12] (external link, opens in new tab) >> Right. It's right. Komi applies even if there's a play if there was no if nobody owns any positions. So yep. Okay. That that's okay. So that's the bug is that we shouldn't have just applied komi by default and then it's never going to be used again. I think the the easiest solution right now is to actually make something that makes sense where we actually just counted the

[38:47] (external link, opens in new tab) Number of positions that are surrounded for each color and then add add to it and that would actually give the winner. >> Do you know what I mean? >> Sure. So, but in that case, so let's let's >> it kind of kills off the rest of what we done before though, the komi winner strategy and the first cell winner strategy. >> I'm not sure because I think later we're going to have different ways to determine the winner, right?

[39:21] (external link, opens in new tab) Different strategies. Well, it's in the end the bas the only strategy you just well I guess the strategy could return the number of points and you just add the strategies together. So there would be a strategy for counting the number of >> remember you had the strategy for determining the winner either by points lost or by points won. >> Yeah. So yeah. So I mean the strategies basically return how many points based on each strategy

[39:52] (external link, opens in new tab) I guess. Okay. So for now let's just let's do a loop and and we'll we'll get rid of it. You think we don't even need a loop? Cuz I just want to keep it that way for now. >> Well, we could do it inside the first first cell strategy. Oh Wrong one. Where did I do? Okay. We do inside here where we do the loop here.

[40:23] (external link, opens in new tab) >> Mhm. >> And we just add it. >> Add what? >> So like >> but then we would remove the single responsibility principle, wouldn't it? Wouldn't we? >> Right. >> So I don't I I want to try and avoid that. Because then we've just moved the whole thing to just a different place, but it's still it's still the same ugly thing. You're saying that there can only be one strategy, right?

[40:54] (external link, opens in new tab) >> Well, the strategy could be get this strategy. We could rename it to get number get points for white. get >> yeah but we don't have the need for points yet we need to have a test for that >> okay >> I want to I want to do this incrementally so that we can show a demo >> so for now so we can show a demo I I will just say var result

[41:26] (external link, opens in new tab) New actually I'm going to undo yeah v result equals new first So when your strategy board get winner just like we did before. >> Mhm. >> And if result equals

[42:05] (external link, opens in new tab) Return result. Okay. Turn you. Sure. Okay. We don't need this. And we don't need this. Okay. Let's make sure everything is passing. Okay. Now, let's commit. First let's so that we can do some commit the refactoring to source control.

[42:50] (external link, opens in new tab) I forgot to git ignore. Okay, I'll do that later. >> Sure. Okay. >> okay. I think that's all the time I actually have right now. So, sorry to leave you hanging. >> no worries. >> But, we'll do we'll do more of these.

[43:22] (external link, opens in new tab) Okay. >> Okay. >> So, next time we're just going to add test to say Okay. Now, the winner has to be determined by counting actually, not just the first one. The first one to win. >> Okay. >> Okay. Ray, it was great. And I know time passes so quickly when we're doing this, isn't it? >> It does. >> Okay. Thanks everyone. >> See you later. >> All right. Bye.