Refactoring

Speaker 1 [00:03]

Great. Thank you very much, Theodore. Dear friends of Python, welcome to the refactoring tutorial. I'm very happy to see a full room on the third day of a conference that has been fantastic so far. I also like to say hello explicitly to everybody who is attending online. I cannot see you. I cannot hear you. But Theodore is having a close look on Slido. And if you have a question, please post it there. and I'm happy to take it into account. I'm going to give a maybe three-minute introduction on what we are about to do. And then we will dive into code and see what we can get done in 90 minutes. So I'm a Python trainer. I live in Berlin. I teach courses. I write stuff, blog pages, books. I've built a diverse team over the past four years and it was a great experience. I would do that again. I'm currently open to work. But today I'm going to talk about refactoring. No, I actually would want to do refactoring together with you. And what I want to do, we have a couple of setup steps. I mentioned those already to those who were in the room a bit earlier. I want to say what refactoring is, then we look at problems in the code, and then I want to go through a couple of elementary refactoring techniques. I don't want to do anything that is super advanced and complicated because I believe that there is a lot of value in doing the simple refactorings regularly, repeatedly, and doing them cleanly, and actually knowing what you do. And that is the main focus of the next 90 minutes. So we will extract a module, extract functions, extract data structures, maybe a class, and then talk about, if we have time, what else one might want to do to the code. I wrote a little space game. This is a rewrite of a game I wrote earlier that is also in one of my GitHub repositories. I wrote this for the purpose of this tutorial, so it's kind of a heavily condensed version of a game. Why did I choose a game? I wanted to do something with spaceships because I think it's fun to do. Second, writing games, I believe it's also fun to do. And finally, writing games exposes interesting programming problems very quickly because if the game doesn't work or is not fun to play because the code is not written right, you notice that very quickly. Not after months when corrupted data infests your database. That is also a situation where you might want to do refactoring, but that is not what fits in 90 minutes that easily. This is why it's a game. So the game is about you have different planets, you travel between them, there are some puzzles to solve or very short quiz questions, and then you either win or not. If, for the purpose of this tutorial, the code is here on this GitHub repository, it's also posted in the Discord channel, to actively work on the code, you need the code from this repository, at least the Python file with the game and the Python file with the tests. We will also need PyTest, this is why I put the pip install here, and here is the command to run the tests. I will do this here in a moment. And then, of course, you could play the game. In case you miss out something that I'm doing, the thing I'm going through here is a simplified edition of the course notes. In the README file, there is a longer version that is more for if you want to go through the entire tutorial by yourself later. It's very wordy. I think it's very challenging to read that while I'm talking. So this is why I made a condensed version that we are going to work through. Now, what is refactoring? What is it that we want to do? There is a textbook definition of refactoring. We want to improve the structure of the code without changing the functionality. That is important. Refactoring is not about making the program better. We want to make the structure or the quality of the code better. You don't want to add any new features while we are at it. So the refactoring might give us ideas what actually to build, But basically, we want to do a more sophisticated cleanup. How to do this? There's a very elementary procedure for refactoring that is run your tests, then change something in the code, and then run the tests again. And we will talk mostly about what is that something that we can change. So before we can start, we need to run the tests. So I have a terminal here ready, where I'm in this refactoring tutorial folder. And I'm going to run my test suite. So just typing pytest won't work in this case. I have to put in the name of the test module. And if everything is downloaded correctly and you have pytest installed, you should see a message like this. Does that work for anyone here in the room? Some of you tried. It seems to be working. Awesome. Good. And so that puts us in a position where we could start refactoring. Like, take-home message at this point, if you don't have any tests in place, then you can start restructuring your code. But I wouldn't dare to call it refactoring. It's very dangerous. If you do this on a big program, there is a chance that you really break things or introduce bugs that have not been there before. It's difficult enough already, especially if you work with code that you have not written on your own. But let's think about what we could improve here. Let's hear a few suggestions from both the people in the room and the people online. Does anyone have an idea or a suggestion? What is bad here in this code? And I'm going to repeat this for the online and the recording. Any suggestions? Bad things in the code here? Well, the text variable, or I would say constant, being there simply. There is a text variable. The first entire half of the file. This does not look like Python code. What else? there is everything is one function this there's just one big function in that program sorry which statements we we have a big big blob of nested if statements one more there is lots of repetitions and these are all typically things that you would want to get rid of in the markdown file on the repo there is a checklist that you can go through that lists the most common things that appear that you might want to pay attention to at this moment. So if you see this many levels of nesting in your code, this is something that you might want to refactor. If your function does not fit on the screen, then how can you read a function if you can't see everything at the same time? Then you have to spend a lot of energy remembering what you just scrolled out on the top. Theodore, do we have anything on Slido? OK, good. So let's start fixing things. Sorry, just one. Oh, we have one.

Speaker 2 [09:19]

Weird number like 79

Speaker 1 [09:19]

Weird number. Line 79, oh, that's a very, I don't remember what.

Speaker 2 [09:27]

Real numbers, I think there are some awkward in numbers someone

Speaker 1 [09:30]

There are hard-coded numbers. I think I added a few comments here, so the line numbers in my file have already shifted. But we do have, in fact, hard-coded numbers here and there in the code. Are there 79 here? I think this is one, yes. Yeah, yeah, yeah, yeah. Okay, so that might be taken into a different place. Thank you very much for those suggestions. Let's start working on the code. Because this file is, I have personally, I have a preference for small Python files in the context of a tutorial. This is already a big Python file. Let's make this smaller. And I want to get that huge, huge text variable out of the way because it's not really Python code. In a real-world program, you might want to move, that might also be HTML code or SQL code or something else that is not Python, and this should go elsewhere. We could put that into a database, into a file. In this case, let's put this into a separate module. And that is our very simple first refactoring. So I want to take, and you can do this right away, take this entire text variable, make it go away. I'm going to cut this out. Yeah, and where should we put it? Let's make a module out of it. So I'm pasting this into a new file, store it in the same folder, and I call it texten.pyen because it's already leaving the opportunity that this refactoring just has made it a bit easier if we decide at some point that the game should be available in multiple languages. We could have more than one of that kind that was kind of very, very, very hard to do if everything is there in the same place. Anything else that we should do? Now the sidebar of my editor just turned half yellow. Import. From text en import text. and I save that so is my is my refactoring done we we need now we need to run the test and see if we accidentally broke anything don't continue refactoring test at first even though this was a very simple refactoring we need to run the test to verify that everything that we assumed working before is still showing the same behavior and we've cut down the size of our file by by almost half so the rest of the tutorial is going to be to make these 100 lines here into a better structure up next is the in my opinion most important, most fundamental refactoring technique or procedure that I believe that there is, that is taking or extracting a function. And I've done this for quite some while, so I decided, hey, let's write a recipe for that. I put that here in the tutorial documentation, and I'm copying that recipe here into the code so that we can see it. So how to extract a function? First, you need to have some piece of code that you want to make a function out of. You need to give the function a name. You need to put code into that function and then basically adjust the function parameters and the return statements and, of course, make a function call. And the last point, again, is the most important. After you extracted a function, you would run the tests again. Now, how do you find a piece of code that you can make a function out of? What are signs that you can make a function out of a piece of code? Sorry? The planets might make nice functions. What else? Something that repeats. Yes, we will get to that. What else? Yes, I think this is the most fundamental and most frequent one. if you see something like this here is a section that is basically printing items or other people that you have on the spaceship and to have a better description of what that does I made a comment above that this is a function it just technically is not one but I had a function already in my head when I wrote this, so we might make a function out of it. So if you see a paragraph of code that you already see it belongs together, you might make a function out of it. So we can go through that recipe here. In this case, it's easy. You can take this comment and directly make it a function. Make it def. and then you take everything that is there before and this becomes our function. This is not in the right place yet, of course. Good. Now comes the challenging part. We need to figure out what arguments the function would have. In the most simple case, we are not thinking about any sophisticated stuff like how to restructure things. All I want to get at this point is to make that function work. So which variables should be put here into the... Credits, engines, and everything that occurs in the function and is not created there. These are Booleans. We might think about that. Having lots of Boolean arguments is maybe not the best design. But let's get back to that later. Then what does this function do? We could write a doc string. I'm going to skip that for now. You might want to check, do we need a return statement? In this function, all this function does is printing, so we don't need a return statement. So I can take this function and move it somewhere else. So let's put it here on top. And, of course, we need to have a function call. So function call is display inventory. And now I need to put in the arguments, of course, and now this is already this already suggests that these three Boolean variables that we used as function variables, that they might need further refactoring because now I have to look up that I don't mess up the order of the arguments. But let's see if this is working. Credits engines copilot. See here in the setup part of my main function, they appear in a different order. That is already a trap that I set up for other programmers, hint that this is not entirely obvious. But let's see if it's working. Tests should be passing at this point. Any questions? do this again if you want to you can try the same with the function that is here at the bottom here the select planet this is another function you can try that on your own everything after the if statement you can do that for the for the part where the planets are being visited like the entire middle part take that out. I'll do that slowly and wait if there are any questions occurring in the meantime. If you are running PyCharm, this might be a good moment to try out the refactoring functions because PyCharm supports that by an automated function where you mark a section of code, I think, what was it, alt-ctrl-m on my machine, and then you can type in the name of the new function and this is it. But you still need to be able to do this procedure routinely on your own. So let's refactor out the second function. Any questions? Theodore, Slido is clear, good. planet and this takes an argument destinations and it has a this time we have a return value select planet needs to return a planet and we can make a a function out of that. And I need to run the tests. Oh, shit. What did I forget? The return, thank you. Return. And this is a nice function because it does more or less one thing and it fits on the screen. I can understand what it does. any ideas how we can make this function even shorter and we could refactor out another function out of this I don't want to go that far today any Pythonic expressions that can we can use enumerate this counter variable is this is something that I learned while programming basic but Python has something better we can use enumerate and we want the enumeration to start at 1 and this this should still do the same thing and oh this has to be called position or I yep that is this is like a small scale refactoring that also helps a lot to include the readability of your code because less lines means less to read unless you make very long lines less lines also mean less bugs good thing did I manage to fix this yes that's the select planet function does anyone want to suggest a signature for the function for visiting planets, if I want to take this entire planet section with all the different planets, move them into one function at first, what would go into the definition line for that function? Let's wait for a moment. You have a suggestion? Yeah, I'm looking for everything in the game . Everything that we have here. So I can basically take the entire state of the game and put that into my visitPlanet function, because it really needs to have all the information from the game. I need planet engines. Oh, no, I should stick to the order that I used in the previous function. Otherwise, that would be really bad. Copilot game end. I don't know why one would want to visit a planet when the game is already ending. But let's leave that here. Maybe it's important after all. And then we visit a planet. And what's very convenient in Python that I can most of the time, unless I start using type annotations, I can copy all these elements here into the function call. Then let's make this a doc string. Now, I see that this visitPlanet function, it does two things. A question in the back. Yes, of course. Is that okay? so this visit planet function is doing two things it's asking no it's basically calculating where what are the next planets or star systems that the the player can go to so it's creating these destinations and then it's asking puzzle questions and these puzzle questions they might modify the state of the game. So somewhere here and there inside the game you would see that here we buy an engine, engines get set to true. What does that mean? Of course it means that we need to return the state after the function finishes. So that means that we get a couple of return values this time. So what we are going to see is that you pay a heavy price for refactoring this function because it's returning a lot of things. It's destinations and then our four boolean variables. And this is now starting to look really ugly. Yeah, I can use a backslash. If I have to use the backslash, this already is a hint that my code starts getting to become a bit convoluted. So this is really hard to fit here on the screen. So this is how my function call looks like. So I put five values in, I get five values back. We should be simplifying that in the next part. but let's make it work first. Do we need to return destinations? Sorry? Yes, we need to return destinations, because when I move this function out here, the next, the selectPlanet function is using that. If we leave the function call for selectPlanet in, then we need to have destinations coming back. Ah, because I didn't go to different sites. You could, of course, leave that part inside VisitPlanet. That's kind of a design decision that you make at that point. And there is no right or wrong. This is basically easy to change. I'll stick to this version because this is what I wrote in the tutorial documentation in case you want to go through this again later. Then I need to unindent this by one level. So I have arguments, I have a function call, I have return values. Does it end what we see? Big benefit, my travel function almost fits on the screen. We are almost there. I could trick around, but it's basically the only reason why it's not fitting on the screen is because of the font size. Do we have a question? No. Good. But does it work? Oh no, it's failing. You are on Earth. beautiful is better than ugly and then you get stopped, then the program gets stuck. It throws some kind of error message. When calling the visit planet function gets a non-type returned did I forget the return statement? I think I did. Yeah, now we have an interesting situation here. Yes, so I of course need a main return and for that I can copy I can copy this thing here from my function call because I did not change any of the variable names Yeah If I put that return statement here so that the return is not empty This should work on earth But it fails it fails on the black hole. So now the tests are still failing, but they fail in a different place because there is a blank return here. And that is a situation where you sometimes, when you do extract functions, you need to modify them a little bit. This is a comparatively simple situation where it's enough to, I think it would be enough to set the game end boolean here, but sometimes you need to do stuff like introducing new variables that reflect what is happening here. So you might need to, sometimes you might need to do something like crunched by black pole equals true. We don't need to introduce a new variable here because we already have a variable that does something like that it's right here it's the game and variable and we can use that instead and that is that is sometimes not easy to see usually these kind of issues are related to to control flow in the functions that you are extracting yeah like if you are if you have all kinds of continue, break statements, loop that change their behavior, or in particular, when you terminate a function earlier, then you might need to have extra variables that represent that. This is why in the documentation, I made a separate section for that that is labeled extract and modify. And in the solutions, there is an extra file that contains this part. So to take what out of the if-else? The game end. Oh, yeah, because you're absolutely right. The game end is redundant. We don't need to have that twice. It's better like that, isn't it? Let's see if it's working. Today is my lucky day. Good. So summarizing, we managed to make, to condense our travel function from 100 lines to something like 15. And that makes it a bit easier to understand what our program is doing. So basically our program is running through an event loop based on user input, visits planet after planet, and at some point the game finishes. So we have kind of separated some of our game logic here. And another nice benefit of that refactoring that we already did is that the new structure of our travel function exposes problems that have been in our code before, but they were hard to see. I think this is one of the strongest things about refactoring is that it makes weak parts of your code easy to see. And the weak part here is, of course, the booleans. I don't like booleans in functions, basically because Uncle Bob, who is one of the well-known programming teachers, and he's been teaching programming probably for as long as I live, he says, don't do this, booleans in function calls are a bad idea if you have many of them. But then he hasn't used Python data science libraries. But let's get these booleans out of the way. And we get with that to the next section that is about making a data structure. What data structure could we use to keep four booleans? Or assuming that at some point, the game designers might want to invent more planets, more items, more puzzles, and these Booleans might grow. How could we keep track of whether someone has an engine, a co-pilot, or the game has ended? A dictionary. So I could do something like this. I call this flags, because I was tempted to call this item, but the co-pilot is not an item. So I could do engines equals false. That would work. There's a simpler option. Sorry? The state data class. We could make it. You mean putting the engine as an attribute? Yeah, that's a valid way to do that. I have a strong preference for starting with the simpler data structures. No bit manipulation, please. Not in my tutorial. It's great. Yes? Maybe a set where we . We can use a set first. When we realize that the set is not sufficient, we might go for a class. I see that as a logical progression, that jumping from a Boolean to a class is probably two steps. And the set is a data structure that is like a shortcut for the dictionary. So I could say something like make an empty set and say there's nothing inside yet, and I get rid of these. So what would that allow me to do? I can say I can only use the flags set instead of all of these. And instead of passing in those arguments to visit planet, I only have to pass flags to visit planet. The best part is that I don't need them in the return value anymore because if you use a set and not a frozen set, it's a mutable data structure. We don't need to return it. So that means our main function just fits on the screen. Is that a good idea, though? Because I thought that it's a side effect of changing the variables. It's not obvious that you're changing the variables? It's a good point. It's an excellent point. So now we have a function that does two things. It's a very, very accurate comment. Thank you. Our visitPlanet function is doing two things. It returns destinations, and it modifies the flags. We should do something with that. But I want to point out that the problem existed already before. The function was already doing two things before. We just put ourselves in a position where this became easy to see. Let's make this work first, and then we get to this destination problem. Actually, that's going to be the next step. So how can we make this work? We now have to use the flags everywhere where the booleans occur. That's a little bit of a moment of tedious work. So I have flags here. I need to check if credits in flags, if engines in flags. Now I'm modifying the display inventory function, if co-pilot in flags. Now what is credits? This is the value that we are putting in. I was thinking whether I should make strings out of these, but I don't think that's a very good idea. I use a little shortcut here. Let's define all the flags here, because there's only four of them. If you want to make this really, really properly, let's make this as a to-do, use an enum. That's one way to do this, because the enum makes sure that only these four things can go into flags. Now we are using integers, which is okay. Question? No. Yeah. There's a question. Yes. . . .

Speaker 2 [39:01]

so it's about if we are going to discuss how to maintain downward compatibility in this tutorial

Speaker 1 [39:02]

. . . . . . . . . Oh, if we have to maintain download compatibility, huh. . . . . . . Yeah, I have a, no, I'm not discussing too much about that. . . . . So the download, you need to think about what you want to keep downloads compatible. That has a lot to do with how you write the tests. In this case, I wrote all my tests against the travel function. So I tested different situations with the travel function, allowing myself to refactor everything that's happening inside later. So I could now go and start writing tests against any of the functions that I just created, but then I would constrain myself quite a bit because I might not be done refactoring this yet. this might change further. Good. Let's see whether there are other places where we need to put in flags. I basically am going to go through everything from the top to the bottom. And when I see one of the booleans, I need to put in in flags behind it. Here we are adding the engine. What do we have to write here to add it to the flag? Flags.add engines. I think this is the tedious part of the tutorial, to clean up, to not miss out any of these places. I believe the proper operator here, if I check for the presence of a flag, is to use the not in operator. Then I fix the Sirius planet, flex.addCredits, I fix the Orion planet where you get the co-pilot, not in flags, flags.addCopilot. And finally, the black hole, are there any flags? Yes. And if engines in flags and co-pilot in flags, then flags.addGame end. I add a comment that this that I want to fix this side effect move side effect finally I'm adding the while condition editing the while condition and this if and see whether it's working. I've been trying this at home. You might take a moment to catch up.

Speaker 2 [43:05]

Theodore, yes, can you explain again why you put the range 4 in the beginning, please?

Speaker 1 [43:10]

the beginning please yes the range for in the beginning so I went when I when I wrote this tutorial I had I was considering three options to manage all the possible things that could go into into the flags one of them was using a set and put strings inside yeah if you put strings inside and you look up something, then Python does string comparison. Now, this game is not really a performance critical application, right? But I didn't want to do that anyway. It didn't feel quite right. I thought there would be a simpler solution. Then I thought a proper solution would be to use an enum, because that makes sure that the thing you put in is really supposed to be in the flags. But I thought, let's do something simpler here. I want to have integer numbers representing each of the items. I wanted to make sure that no two items have the same integer. And of course, I didn't want to write something like if three in flags, because that makes your code completely unreadable. so I wanted to write I basically wanted to write credits here instead of quote credits quote I think that was the main motivation

Speaker 2 [44:45]

Other question? Thank you. Is there a rule of thumb on what is considered one thing a function does before we split it in separate functions? Things can get big. Dot, dot, dot.

Speaker 1 [44:59]

I can quote, I have a personal opinion. I have an opinion that was carried to me through that from Uncle Bob. Uncle Bob says that a function should do exactly one thing. And then he was musing about what does it mean that a function makes one thing. That is when you cannot make two functions out of it. that is someone commented here to me on this conference that this is rather an extreme point of view and I think this is what Uncle Bob is known for in practice I don't go that far because I have other refactorings to do and I have other things to do besides refactoring So as a long-term goal, having very small atomic functions, I believe this is a very good idea, but you don't have to refactor everything to three-line functions right away. So what I'm usually happy with is when my functions fit on the screen, like even with a font size like this one here, And I can see what they do quickly without being familiar with the code. That means some other programmer should be able to work with the function. That means things that get on the way is like massive use of dunder functions. Lots of list comprehensions in a function means that your function does many things. If you have a very long list of arguments or return values, I think anything of five and bigger starts feeling painful for me. I hope that answers the question. there are a lot of small things to change in this code in case you get stuck somewhere and would like to see a working version of the code there is one in solution slash the subfolder with extract data structures where this is done correctly, in case you want to compare against. Good. But we have extracted one data structure. We have replaced our Boolean values by a set of integers and acknowledged that there are, in fact, a couple of different options. And this is very typical of refactoring that there is more than one way to do it. This is why people develop different programming styles. It starts when you refactor your code, or probably already earlier. Any questions? I'll run my tests again in a moment and then see whether we can find another data structure that we might want to refactor out let's run the tests they are still working good I have to two things to to edit here in this in this tutorial and before I open the floor to like more generic discussions one of them is I want to extract another data structure and I would like to extract a class. Let's start with the data structure and now we can start tackling this still big visitPlanet function. There is a strange pattern here that should make you suspicious when you read the code and that is you have a repetitive element here. It's not exactly a code duplication but it's repetitive. We could use an elif. Yes? But actually, I want to get rid of the ifs altogether. Hmm? yes so the the destinations here they strictly depend on the planet this is this is this is the thing that Python dictionaries were invented for and that's a very very common refactoring pattern if you have some codes that reads if something or same with the case statement you can if you have this long chain of ifs that all go against the same variable. There is a dictionary hiding here. It's not always a good idea to use a dictionary but very often it is. So what we can do is we can create a dictionary, I'll call it star map, that contains all the destinations. So I'm going to write earth, take the destinations out of here then let's put in place holders for all the planets and then cut paste what other planet do I had Centauri and the black hole and this makes the destinations disappear here from my from visit planet does sirius has destinations as well three of them and the black hole had one as well. Now, the only thing where destinations remain is here in the line with the return. What can I replace that with? star map and as a key I use the planet that's it now the function is still doing two different things but one of the things is doing all the puzzle stuff here and the other thing is in the very last line using the star map we've almost separated it these two things here and could now split the function into two. And this dictionary Python, it's really one of the most common refactorings in Python that I have seen. And this looks good. Now it's really easy to add another planet to your map of the universe because all you you need to do is edit here, and then maybe add a description, anything. But the ifs are still there, yeah? One thing that you might want to do, especially if you happen to use PyCharm, you could take these entire sections with the puzzles out. Yeah, I'll do one of them for now, because we are going to see then what I want to get at. I do the engine puzzle here. It takes the flags that is happening on Centauri, take that out, move it to a different place just so that the visit planet function becomes a bit shorter. We can move that code out of the way as well, including the initial if that checks for the flags. And if we now look at VisitPlanet, hey, we moved out the destinations, but it still looks kind of repetitive. What should we do with it? What is there that we can do? the we can make the print statement more generic yes how can we make the print statement more generic and the the key here is derived from the planet name we could do something like he equals planet dot upper plus this looks a bit hacky but it works and we can have a generic print for the planet description and now the Earth section is already empty because on Earth there is nothing going on. Nothing important, at least from the point of view of the universe. And I can remove all the lines that print descriptions. So my visit planet function becomes shorter and shorter. There is one more. I think that was a duplicate. The Orion description was printed twice, so the code is condensing further. The black hole had a description as well. And it got shorter. I need to run my tests I haven't done this in a while the description heck so we have this this huge dictionary that contains all the text elements and they are accessed by a generic descriptor and the description of the planets there always consists of the name of the planet in uppercase plus underscore description So I'm generating that from the planet to look it up directly. Yes, another question.

Speaker 2 [56:48]

What about introducing an event map with a key being each planet and the value of function to each planet logic?

Speaker 1 [57:03]

That's a fantastic suggestion. Thank you. Thank you to the person suggesting that on slack so the suggestion is we might have a data structure The Word event map is used and let's say I want to represent that There is this puzzle function on Centauri. I might represent that here in my event map and use, say, there is the engine puzzle here. Note that I'm not calling the function here. So I'm putting functions in a dictionary. That is a huge shortcut that is possible in Python that is not possible in Java that way. In Java, you would have to write a couple of classes to make this possible. In C, you could use some pointer operation. Maybe they have some modern things how this can be implemented. In Python, you can put your functions in a dictionary. This is a great pattern, by the way. How can I use this? I can say here, if planet in event map, I take event map planet. This is a function, and that uses the flags, And that is going to do the same thing as these two lines, and we get our If for Centauri out of the way, so we've converted this If logic into one more program element of data structure actually Yes, the guy in the back Can you can you speak up a bit? I could use a get. Yes. So the suggestion is that instead of using the if statement, I could use event map dot get. That's a very neat suggestion. So I can use planet. And in case the planet is not in the event map, I need to have some empty function because I cannot call none I put an empty function here I don't know whether that's that can be probably written more nicely but I got rid of one extra if that way that's usually a good thing yeah so let's quickly do this at the other puzzles to the event map we have another question yes

Speaker 2 [60:04]

question yes can you run through the code from top to bottom or top to bottom again I mean maybe not now maybe a little bit later yes

Speaker 1 [60:12]

Yes, I can. I'll start here at the moment. We have that star map that contains all the destinations for each planet. Then we start creating another dictionary where we want to have every planet that has a puzzle. So, on Sirius there is a quiz, on Orion you hire a co-pilot, and in the black hole you would jump into the black hole. These are the four puzzles that the game consists of, so we need to make functions of the three, And the code is going to become a lot more readable in a moment. So I need to create a quiz function from that code on Sirius that is here, quiz flags. These puzzle functions, they modify flags. They do not need any return values. So the only output channel of these puzzle functions, like this quiz here, is that they modify flags. I'll do the same for the black hole function. But how did I call it? Jump into. That also takes flags. And I think that will remove our side effect here. And I think there is one more here. Def hire co-pilot flags. These small functions, I still don't like very much this nested if-else structure that is here in the function. And I was thinking really hard about it while writing the game for this tutorial. I think I had two more puzzles initially. And getting that somehow generalized was really, really hard. And in the end, I decided not to do anything with it. But by putting these into these small functions, we have at least contained that part of complex logic. There is one specific place in our program that is responsible for it, and it's cleanly labeled. So we can live with that for now. So we now have, from top to the bottom, we create our possible flags. We have a display inventory function. We have a selectPlanet function. Then we have one, two, three, four puzzle functions that all return nothing and all have flags as an argument. So they have the same signature because we want to use them as part of the event map data structure. Then we define the map of the planets. And then we define the event map. And hey, where did our visitPlanet function go? It's gone almost. We delegated all the parts that happen here to different parts of the program. So all VisitPlanet does now is it prints the description, runs a puzzle if there is one, and then returns the next destinations. That's it. We've managed to get rid of, or no, We've portionized the chaos. And travel is also rather short. I see two. So more or less, the state of this is in the solution folder in extract data structures in a working condition. let's see let's see if i have a working working code as well i have i'm still lucky and i can take another question from the online

Speaker 2 [64:55]

What's your opinion on one type hints to docstring for every functions?

Speaker 1 [65:02]

whether I have an opinion on having docstrings for every function. I think it's a good idea, and refactoring is actually a good time to put in docstrings for functions. I am leaving them out because there's already a lot going on on the screen in this tutorial. I can understand that there are situations where the team doesn't really need, if you are working with experienced people who know the code base well, then the doc strings are maybe not necessary for them. But what do you do when a junior person joins the team or a new person is on boarded? For those, I believe that dock strings are quite crucial.

Speaker 2 [66:04]

Second question, and what about type hints? What's your opinion?

Speaker 1 [66:07]

What's your opinion on that? What type?

Speaker 2 [66:09]

Type int.

Speaker 1 [66:09]

Type. Type hints. I like type hints a lot. I mean, type hints are, from my perspective at the moment, I treat them a little bit like machine-readable documentation unless you plug in a static type checker into your continuous integration, which I think could be a good idea. I see that I've worked with statically typed languages and having them catches a lot of bugs early. I think that is the motivation with using types. But I don't have practical experience with how well that works with Python code. I sometimes have the impression it makes code a bit harder to read. but this could be one goal of refactoring actually fix this but I would feel more comfortable backing that up by MyPy or some other static type checker yeah, one more

Speaker 2 [67:15]

What are the options to replace lambda x?

Speaker 1 [67:21]

What is this doing?

Speaker 2 [67:23]

What are the options to replace it?

Speaker 1 [67:25]

Yeah, I could make a function do nothing. That's the other option. I think this is easier to read. Thank you for the question. Now, we have some time left. Let's take a look at another design weakness. Now look at this, what we created. The star map idea was awesome, and it made our code simpler. The event map idea was awesome, also made our code simpler and easier to read and easier to change. But now we have two dictionaries that look the same. We could have done the same with the descriptions. Then we would have three dictionaries that look the same. What is there hiding when that happens in your code? We could make a nested dictionary out of it. We could make a nested dictionary out of it. Now that's the time where we could introduce a class. Let's do that, see how it looks like. When you see that and say, hey, I'm going to have more planets, you can think about making a planet class. Let's make a planet class. What attributes does the planet class have? When you write classes in my opinion what you're really doing is data modeling. So think of your attributes first. What attributes would you give a planet? Sorry, stars. What do we mean by stars? Moons. We don't have any moons in the game yet. Name. Let's start with a name. I mean you could you could introduce that might get add a nice element to the game so but Starting from the stuff that we have here, so we have a name What else? We have a description for the description, I don't have to put in the description as a as an Input parameter because I can use this whoop there goes my description now it's encapsulated in a class if I want to have something more intelligent here later I can change that yes sorry again replace the planet with oh yeah so sure thank you I would like to have two more attributes here yes destinations and they would go here up here and the puzzle thing that it could happen that there is no puzzle good so my now and I can now use the information here in my star map and make planet objects out of it so I contain kind of the same information I'll add the puzzles here in a moment I know there shouldn't be a curly bracket in the beginning, we'll take care of that in a moment but we will create let's create a list of planets first then all the planets, some of the planets they do need to have a puzzle and I can take these from the event map. So I'm bringing together these two data structures into one. So Centauri has the engine puzzle. Now note that with this structure it became really easy to put some puzzle that you already have on a different planet. It's a nice compositional pattern that we have here. The copilot hiring goes here and the jumping is with the black hole and on earth there's still nothing going on. Good. And I want to have in the in the end I want to have a dictionary planets and I'm going to do p.name for p in this. So I'm going to use a dict comprehension here to create a dictionary of planet objects. Sorry? We only have a set of planets yet. Oh, you are right. like this is sorry is this better thank you so we define the we define the attributes of the planet there are four of them then we have the we have regenerate planet objects we might to adjust the methods there at the bottom is there a question on the from the online

Speaker 2 [73:51]

So I had it, so thank you.

Speaker 1 [73:51]

So thank you.

Speaker 2 [73:53]

Should we rather make puzzle non-optional in the class init and explicitly use the do nothing when no puzzle is needed?

Speaker 1 [74:01]

Oh, we could we could use do nothing. That's a very that's an excellent suggestion I think that do nothing was much better than the none Thanks debugging gets so much easier when you have more than one person looking at the code okay I would like to have a method here who do we have another question yeah

Speaker 2 [74:35]

Yeah, maybe yeah

Speaker 1 [74:36]

Let's take the question first. No question. What can you do with a planet? I want to have a method here. You visit them. Visit. We already have a function called visitPlanet and we can carry that, make a method out of this. Oh, you're so great. You are so great there. Is it that comma? That was the missing comma. Thank you. Of course, you don't want to have a method visit planet inside a class called planet here. It's just visit. It's more of a linguistical thing. I think here was the description printing. And here we can just say print self.description. Now we don't need to take anything from the event map because that event map does not exist anymore. The event map was kind of a transient state in our refactoring that helped us to see how our class might look like. Yeah, it was a good idea that helped us get there. If you might decide that when you have these these dictionaries that you stop there. I've done enough refactoring for now And keep that but now we have a class So if we do self dot we call self dot puzzle This again became very simple and we return Self dot this the I don't think we need to return that. We can use planet.destinations where that function is called. So now we went down from 100 lines to 2. So let's see how this code would be used. We need to change the travel functions. So, first thing, what would we need to change here? We use a planet object here to start with. The flags did not really change. Then we say, oopsie, then we say planet.visit. with the flags and now that doesn't return anything anymore instead we can use planet.destinations here as an attribute directly and I think we have to make sure that what we get here out of select planet is really a planet object. So we might want to say planet equals planets. Ah, that doesn't look nice. Maybe like this. Do my tests pass? Oh no, do nothing is not defined. I think this needs to go somewhere on top planet is not defined I think there's a missing us

Speaker 2 [78:46]

I'm missing your S.

Speaker 1 [78:48]

planets visit takes one positional argument that looks like a missing self self our flags yourself flags without the tests I would be completely lost at this point don't know how about you okay question yes

Speaker 2 [79:17]

what's your opinion on named VS positional arguments or one example will be the planet class initializing the planet class

Speaker 1 [79:29]

Sorry again, I didn't get the first part of the question.

Speaker 2 [79:34]

What is your opinion on named vs. positional arguments?

Speaker 1 [79:43]

names as names versus traditional arguments I don't think I have an opinion so what what I've what I've seen that these the keyword arguments they they are very useful if you build like a large library like like let's a good example mudplotlib does that a lot because there's lots of functions internally where the same chain of optional arguments gets passed around through half the library. When you write your own, or when I write my own stuff, I tend not to try to imitate that unless I'm writing something really big because I want to have things more explicit and see how the things are named. Good. Other questions? So the code is working. The planet class here is working. The travel function is working. Everything else has been delegated to functions. There is one point that is not super pretty yet. In the interest of time and leaving a few minutes for discussion, I would like not to fix that here now, but you see there's still some redundancy here. We have the same condition in our main function twice. The order of lines can be changed a little bit to make this go away and it's probably a good idea. So if we check game end, we probably should check it only once. Which is a good reminder that what we have here now is not the end of refactoring, it's more a state that we can work with more easily than we can work with before. Questions? We have time for questions. I think I'm done with the code. If there's any section of the code that you'd like to see, I'm happy to scroll to it. Do you normally use the automation libraries like Rope, resectoring libraries to automate stuff? No, I don't. I personally believe there is a lot of value in being able to do this yourself, things like extracting a function when you have a big chunk of code and chop it down into smaller pieces like PyCharm supports it. I think it's a great idea, but you still need to be able to do it on your own. I want to know what's going on in my code. So my general approach, so first what did I do here is I basically hijacked the print and the input function, because that is how the player interacts with it. The test needs to be able to type stuff in and see what's in the prints. That is what is done here. PyTest provides very, very nice shortcuts for both. this monkey patch fixture and the capsus fixture that we see here. So I had to look this up in the PyTest documentation, but once I found it, it was super easy to do. Your other part of the question, the order in which I proceed, I wrote a prototype first to figure out how the whole thing looks like. So I got a basic version of the game running. Then I wrote the tests. And then I changed a lot of things in the game for over a couple of days. The tests more or less stayed the same. So all the prototype writing tests, then changing stuff. And I think the only thing that I changed in the test file was the test data here where there is the actual solution to the game, or this is the point where you finish in the black hole and don't get out. This is the version where you get out. And these are keywords that should appear in the output. So it's not exhaustive testing. You don't need to test everything possible. It's more like a scaffolding that helps you to walk around the construction site. Yes? Do you add tests when you refactor? That's a great time to do that, yes. For instance, the test, the death by black hole test, this was added during my own refactoring, when I did a trial run of this whole thing. Yes. Because what happens quite frequently is that the refactoring might actually uncover bugs or weaknesses of your code that you did not see before. Theodore.

Speaker 2 [85:30]

question from the slide oh where where is the text brackets come

Speaker 1 [85:30]

Question. The text bracket is coming from here. This is a Python dictionary where we have all the... This is the thing that we moved out of the way as the very first thing in this tutorial. And having that in a separate module, this would make it very easy to have... This could be a JSON file. So you could have one JSON file for English, one for German, one for some other favorite language. Next question. Can you spend a minute explaining the principles you applied in solution number seven? Oh, wow. Yeah, I can. There is an elephant in the room. Yeah, I'm very, very happy about that question. The elephant in the room is that there is a very, very big design weakness in this version of the game. The design weakness is that our input-output functions, namely print and input, they are mixed everywhere throughout the code. What that means is we do not have a clean separation of business logic, that means the actual game, and user interface. That means one could not make a web app out of this very easily. This would be more or less a rewrite. In solution number 7, I tried to decouple that using the same tests. And what I did was, let's go to the very bottom. I used a couple of design patterns from the design pattern textbook by the Gang of Four from 1994. These design patterns are very, very useful when you build more complex object-oriented architectures. The book itself is not. I recommend the more modern O'Reilly headfirst design patterns, which is a great book if you want to learn about how to compose classes. So the whole purpose of this was to have one thing for print and input. So there are two functions here. I think this is the only print in the program, or maybe not the only print. Maybe there's two of them. But the print is isolated from the rest, and the input is isolated from the rest. And everything else that you find here in the file is to make that possible. So what that means is I treat the planet selection and answering a puzzle as the same thing from the program's point of view. And that required to write a few classes to make that abstraction possible.

Speaker 2 [88:40]

I think we are running out of time. So thank you very much, Christian. I think we got one last question from the audience to scroll through everything, but I suspect that you have the solution.

Speaker 1 [88:52]

have the solution? I have a solution. I think there are seven different solutions on the GitHub. But I think I'm going to post right away, after I've talked to everybody who wants to talk afterwards, I push that one from the tutorial to GitHub as well. Because there are small differences every time. I refactored this about seven times to prepare for the session, to find my own bugs while I'm talking to you. And every one of them was different. And this one that we did together was different again. And this is how it does. There is no one, not one single thing. As long as the tests pass, you should be fine so. Please have tests, and then have fun and success refactoring your code. Thank you. So yeah.