Refactoring Conway's Game of Life | ArjanCodes Code Roast

it's time for the first code roast of 2024 Cod [Music] roast this code has been submitted by Lup it's a game of life implementation when I say game of life I don't mean growing up getting married kids work a lot retire and then die I mean a computer program called The Game of Life it's also called life and it's a cellular cellular cellular cellular how do you even say that it's a cellular automat created by a British mathematician called John Conway in the ' 70s life is a zero player game that sounds both profound and depressing you just provide an initial configuration and then you watch how it evolves I'm going to start with a review of the code and then I'll do a full refactor now if you want to learn how to review code yourself and detect problems faster check out my free workshop on code diagnosis you can access at IR code diagnosis it's it's a 30-minute Workshop that teaches you how to detect problems in your code fast by looking at existing codes from a few common libraries so ir. code diagnosis to get access for free the link is also in the description of the video now let's dive into the game of life let's first take a look at what happens when we try to run this game of life so this is what it looks like it uses matte plot lip to show the state of the game and as you can see there is a sort of weird creature thing happening here so let's take a look at the actual code so the main file here is called Game of Life and this is what it looks like so it starts with a couple of different classes there's a birth rule lonely death rule stay alive Rule and overpopulate Rule so these are the four basic rules of the game of Life game and each class has a static method to apply that rule to a cell and this also provides the neighbor information because that's what you need in order to uh apply the rule and as you can see each rule is slightly different and returns a value or it returns none then we have grid class that has an initializer rows columns and there is a grid uh stored in a list of lists and then there are a couple of helper methods like checking if a certain row and column is within the bonds of the grid whether a certain cell is alive whether the neighbors are alive and a string representation which is used by the console visualization then there's a class game that has a grid it has rules and it has an update method that updates the simulation every step and that's a nested for Loop triple nested for Loop actually and that does the updating and then we have the main function there's a configuration number of rows and columns how many generations uh the rules that should be applied the sleep time so that influences how fast the simulation is going and how we're going to Output this and then we create the game we set an initial state to the grid and then depending on the output type we either visualize it with this function or we use a simple for Loop and just print the grid to the console so that's the main file then there are two other files that are important one is the visualizer so this is a single visualized function that gets the game number of generations and the sleep time and then uses map plot lip to visualize the game and finally that's actually nice we have a test file that has unit test for the various aspects of the game so it tests the rules it tests the grid and it tests the game as well and these are organized into classes using unit test and this is what it looks like so to start my review with the test so one of course it's good that you have test many programs that I've seen in earlier codos didn't have any test whatsoever so the fact that you have them is really good however putting all the tests into a single file is not such a good idea because it's a really large file it's hard to keep track of what things you tested and what things you didn't test yet so I would split this up over multiple files what I would also do is instead of using unit test I would use py test py test allows you to Define tests in functions and not in classes and that makes your testing code way easier to read so next the visualize function so one thing that I notice is that there are no type annotations here so I think it's good that you split out this code from the main file because this is a specific type of visualization but type annotations are missing so we have no idea what game is or what generations and sleep time is sleep time we can kind of guess that it's uh a numeric value but Generations is it the Boolean is an INT we simply don't know what it is we have to look into the code to figure out oh actually we're using it to create a range so apparently it's an integer value but uh there's no way to know by looking at the header so that information is missing in the main file so you can see that actually still most of the code is in a single file and that's also problem itic because now it's hard to understand what is actually the main code that's being run and we have to scroll past grids and the game classes in order to get to the main function also here type annotations are often missing what's the return value of the update method what does alive neighbors return what does is alive return Etc so all of this is missing and the problem with the missing type annotations that they actually hide in Precision for example the is aive method here you would think it returns an integer but actually this is a Boolean value so in some cases this returns a Boolean value and in some cases this returns an integer value now you could map zero to false so that sort of makes sense but it's not what the method actually returns and that can lead to weird bugs if you're not careful about those things and conceptually speaking I would expect is alive to return a Boolean and not an integer so in short adding these type annotations is important because it helps helps us understand the code better and not make these kind of imprecise Errors then there are a few other issues so we have this grid class which makes sense but if we look at how it's actually being used in the main file so we create a game object and game has grid and then we're directly setting grid values here this is what's called a law of Demeter violation this means that the main function needs to have detailed knowledge of the implementation of the game class in order to set things in the grid so here we need to know that it has a grid attribute that the grid attribute itself has another grid attribute and that that happens to be a list of lists a two-dimensional list and then we can set values so this is a bad idea because if we ever decide to change the implementation of the grid maybe change it to something that's a bit more performant then now we also have to change this code here which seemingly has nothing to do with the actual grid implementation and that's why should avoid these types of law of to meter violation here we have a good example of something that could be more like a strategy pattern where we have different Behavior depending on a particular type of output what I don't like about this is that there is Imports within the code itself I'd like Imports to be at the top of the file so I can more clearly see what the dependencies are now I understand you maybe want to make this more efficient by not importing something if you don't use it but I would not do that unless it seriously hampers performance because now it's hard to see if we look at the top of the file we think oh this actually doesn't import anything but that's actually not true because there is an import right here at the end of the file so I wouldn't do that always put your import simply at the top you see that pilent also agrees with this another thing is about the config itself so now config is a dictionary that's inside the main function I wouldn't do this because now the configuration setting is hard to find you have to scroll all the way to line 84 in order to find the configuration what I would do instead is Define the config values as constants in the top of the file that way later on if you for example want to move some of these things to environment variables so you can more easily change them uh without having to change the code then you can simply replace them there by the import from the environment variable and you don't have to do that here and also by putting it in a dictionary it makes it kind of hard to use because now we have to uh get a number of rows by going into the config dictionary and if we make a typo here python is not going to detect us because it's a dictionary key and then uh we're going to get a weird bug that we're going to have to look into so it's best to avoid this type of dictionary usage because you can't rely on the IDE anymore to help you with typos now finally a minor thing is that we have a requirements.txt I would actually suggest to use a by project file which is what you would typically use with tool like poetry and there also seem to be quite a few things in requirements.txt that are not being used at all I think only matte plot lip is actually been used and all the other things are not used that also they shouldn't be in requirements.txt so that concludes my review of this code now let's get started with refactoring this code and improving things a bit so the first thing that I've done as sort of preparation for the refactoring is adding a p project file and that only contains the dependencies that we actually need which is mat plot lip and I'm going to start by rewriting the tests using py test and that means that I don't have to Define these classes anymore moving all of these test to functions is a bit of work but it's going to make it a lot easier to see what is going on now I've already done some preparation here because it just takes a long time to make this translation it's not so interesting for you to watch so what I did is I created a folder called test where I added for each of the types of test I added the py test test so these are just simple functions this is just a copy of what is in the original test file but I just created functions out of them with a bit of help by chat GPT by the way so here we have the test for the birth rules and I've also created test for the lonely death rule the over overpopulate Rule and the stale life rule so these are all in different files so that they're easy to track and we can easily see what is being tested exactly I've also translated the test for the grid so in this case I'm using a fixture from P test so this creates a setup for us so that's the same as what we had before in this file the test file so with the unit test package to do this you need to create a setup method in a class here I've created a p test picture and then you can pass this as an argument to each of the testing functions so we have testing the initialization which tests that the grid has indeed three rows and columns then we have bound tests we have is a live test and a couple of other tests as well so I've simply all translated them from the unit test file and same for the game test so also here we have now simple functions that test specific states of the game again the code is just more or less directly copied from the original test and then what I can do is run P test and now you can see we have 27 tests that are passed so this gives us our safety net that we can work from and now what I can do is remove this file because we don't need that anymore since the tests are now in a separate folder now the next thing that I'd like to do is split out the code a bit from this huge Game of Life file so I'll keep the main function for now but I want to move at least the game class the grid class and the rules to separate files I'm going to start with the rules so I'm going to create a file called rules. pii and then I'm going to take these classes birth rule Lely death rule stay alive Rule and overpopulate Rule and I'm going to move them over there like so and now that means in the game of live class I need to import them so from Rules import and then we have these four rules and also need to update these Imports in the various tests that I've written so this is going to be rules instead of Game of Life and same for these tests there we go let's run by test again to make sure we didn't make any mistake and the test still passed next what I'd like like to do just to simplify things a bit is rename gameof life. pii to main. pii that way we know that's the main file that's what we need to run when we want to launch the application and now I'm going to move out the other classes as well so let's start with the grid so I'm going to create a grid. pi file and then I'm going to take the grid class and then move it over there so there we go and then of course in the main file I'm going to need to import that like so now I just need to go through the different test and also fix the Imports there you see that pylon gives me some errors with Imports it's actually just the IDE that's complaining this is not the python interpreter because if I run the py test again you see that it will work just fine it's a bit frustrating that this is happening it's probably has something to do with pass if you have an idea of how to best fix these kind of things let me know in the comments so now back to the main file we still have the game class so let me also move that to another file called game. pii and we see it needs the grid so let's import that like so and in the main file we're going to need to import it there we go and actually now the grid import is no longer needed here like so and then in our test of course we also have to make sure that we import game from game let's run the test again to make sure this is all still working correctly so now that we've done this the main file is a bit more manageable let me also move this import to the top so there that also fixes that problem run P test again yep that's still working so until now these have been pretty superficial refactors I've mainly moved around code a bit and just Rewritten the test to be P test tests so now we've done this we can start making other types of changes and I want to start with the rules because the rules they currently classes and in order to do that let's see how rules are actually being used so that's in the game class let's see what's happening there exactly so you see that for each of the rules we call the apply function I think using classes for the rules is slightly over complicating things because in essence a rule is a function so why not turn them into functions instead so the first thing that I'm going to do is Define here A type that specifies what a rule should actually look like and I'm going to use the new python 3.12 type annotations for that so we have a type Rule and this is going to be a callable which I import from typing and the rule takes two integers and it returns an integer or returns none that's also what you see here so we apply a cell and the live neighbors and then it returns into your or returns none and since we now assume rule is a function we don't have to do dot apply anymore but let's first fix the type annotation here so rules is going to be a list of rule and that's optionally none so now self. rules is also a list of rules and if you want to be very precise then you can even indicate that here in the type annotation of the rules arguments rows and calls they're both integers so let's also add those type annotations here and the initializer is not going to R return anything so that's going to be non return type like so then we have the update method which is also going to have a non return type and then here since rule is a function we can simply do this we don't need to call Dot apply let's also untangle these nested for Loops a bit so this whole part that we have here is actually about applying the rule so we could create a separate internal method for that and let's call that apply rules to cell and this should get a cell and the alive neighbors and we're going to let it return an INT I'll talk about that in a minute so this is going to be part of that method and actually this should still be part of the update function like so so before you use new cell to keep track of whether there was a result from the rule but since this is now a method we can do it in a slightly smarter way so this part assigns the new cell to the new grid so the way that we can set it up is that we can let new cell be applying the rules to be the result of the apply rules to cell method and then here we don't need this variable but then if the result is not none then we can simply return the result like so and in all other case if there is no result we simply return the cell which means that we can now simplify the right hand side here by simply removing that since that's already been taken care of in the apply rules to cell method like so so this is nice because it has simplified the update method a lot let's run our tests again so now we see there are some issues because we haven't changed the rules yet to functions well we already updated the game to have function rules so this is something we need to fix first so let's go to the rules class and then let's change this so here the birth rule let's start there instead of having this class with a static method we can simply change this to birth rule like so and let's also change the type annotations while we're at it and this is going to return in or nonone this is going to be the lonely death rule again expects two integers and we're going to return into or non then we have to stay alive and finally we have the overpopulate rule like so let's fix the test so here we're going to import birth Rule and instead of applying it we can simply call the function let's do the same for the other rules then we're also going to need to change it in the game test so that's birth Rule and the other rules as well and then here we don't have the classes but we have the four rule functions and I think that should be mostly it so let's try this again so we see there's a problem with a missing right bracket which is of course something that we always need to do let's try that again and our test pass again good we're back in the safe zone so we've made now pretty big changes to the rules so rules are now way simpler that's just simple functions and that makes way more sense to me in the main file we also need to import the new rule functions and while read it let's run the game again to make sure that this still works as expected and it still does next what I want to do is clean up the grid class a bit first step is that we're going to add some type annotations to make clear what this is actually all about and this is going to be a non type and at the same time I'd like to solve that law of the meter problem where we have to dive into the implementation details of the grid by setting this kind of initial state so what I'm going to do is add an extra argument here called initial called init State and that's going to be a list of list of integers or n and by default that's going to be none and then what I'm going to do is that if there is an init state I'm going to set the grid to that in it State else I'm going to initialize the grid as usual let's also add type annotations here so rows and columns they're integers and this is going to return a Boolean value then I also want to fix is alive so I'm going to set row to an integer and column to an integer and I want this to return a Boolean value so I'm not going to let it return zero but it's going to be false and let's run py test again to make sure that that doesn't break anything fortunately not so it's always being used as Boolean in the rest of the code next we have alive neighbors so I'm also going to supply the type annotations here and this is going to return an INT as a value I am going to change the implementation here slightly because this has a sum of is alive of course but is alive is actually a Boolean so what I'm going to do instead is I'm going to return the list but then I'm going to do count true so I'm counting all the instances of true I feel this is a better approach it's more correct with the way that the types are and count is going to return an integer as a result let's run the test again it still works as expected another thing that I'd like to do is solve a la of theer problem here and that that game needs to know implementation details of how GD is organized by the rows and the columns that's because it needs to run this nested for Loop now you could decide to move the whole code over to grid but I'm going to show you something else that you can also do which is that we could turn grid into an interval and then what that will allow us to do is to iterate over all of the values in the grid both the rows and the columns so in order to do that it's quite simple so we return an iterator for each of the rows and columns in the grid and then we yield the value in the grid and we can make this even a bit nicer by also returning the row and the column so we know where we are so once we have this we can now replace this whole nested for Loop by four cell in cell. Grid and then we don't need this line anymore and then we can de invent this and then of course we also need the row column and the cell here and now this becomes way simpler so that's really nice and then a final thing that we could solve here is that instead of creating a list of lists and then assigning that directly to the grid implementation we could actually simply create a new grid with the same dimensions and then what we can do here is simply assign that to self. Grid at the end the only thing we need to do now is solve this thing because now of course we can directly access the grid here like so now we could do something like this again but again that would be a violation of the law of the meter so what I'm going to do instead is add to grid a set cell method that's going to allow us to set a value at a particular row and column and now in the game class we can do set cell and then we Supply the new cell there we go let's run the test again still works but since we now added a method set cell let's also add a test for that so I'm going to add that here so very simple I'm just going to set a cell to a certain value and then I'm going to assert that this actually has the right value so we should have one more test now and that test also passed so this already makes things a lot better the next thing that I want to address is this part so we're dealing here with different types of visualizers so I'd like to organize this a bit differently what I'm going to do is create a folder called visualizers and then I'm going to take this visualizer which is the one using matplot lip and I'm going to rename that to plots. pii and I'm going to call this visualize plot so that's easy to understand what this is doing npy is not needed here so I'll just remove the import also I'd like to fix the type notations here so Generations that's an INT sleep time is a float there we go and this is not going to return anything as a result also here you see a law of the meter violation that or accessing the grid part of the game and the lower level list of list that is part of the grid again so that's not great either this we can solve by simply adding some properties to grid and game so what we can do here in the grid class is that we add a property called raw and that's simply going to return the um the raw grid itself so this is going to be a list of list of integers so this is going to return the raw grid and then in game we should do something similar so let's add a raw grid which is going to return the result of this raw property call like so and now we can do in plot is that we do game do raw grid and same here so now visualized plot no longer needs implementation details of the game class so how do we deal with the type annotation for game so we could import it from game simple solution is to import it from game another thing you can do is Define a protocol class and the only thing that we need for the protocol class that it needs an update method because that's what we're calling here and we need a raw grid property so let's add a type annotation here and now we also have the syntax highlighting here which is nice generation is not actually being used it's simply used for repeating so I'm changing that to an underscore and fig is also not being used so there we go we have our plot visualizer let's also import it in the main file and then use that instead there we go finally what I'd like to do is move this part to a visualized console so I'm going to create a console. pii which is going to look exactly the same game generations and the sleep time and then I'm simply going to put this for Loop into that and we're going to need to import time mat plot that is not needed here and this is not the config value but this is the actual sleep time there we go and then in the main file from visualizers do console we're going to import this function and then we're going to call that here there we go the time import now is also no longer needed in the main file let's run the test again there we go still works okay so I'm starting to be pretty happy now with ref Factory the final thing that I'm going to do is move this config dictionary out of the code and going to turn that into a couple of constants at the top so we're going to have rows and columns and I will simply use that here also I'd like to have the rules defined here let me remove them here then we have the generations and we have the sleep time and the output type which is either console or visualizer I'm going to remove that here and then change the values here there we go that already looks a lot simpler the final thing that I'd like to fix is this initial State because here we still have that annoying love the meter violation where we're trying to set the game grid values and what I'm going to do for that is change how setup in the game class so here currently we create the grid as part of the initializer and that makes it hard to replace the grid by already a prefilled value so instead of supplying the rows and the columns I'm simply going to supply the grid so this is nice because it actually simplifies the game class bit bit and then in the main file what I can do is I can now create a grid so I'll need to reimport that and then we can supply that to the game class and now what I can do is call grid. set cell to set the actual values and let's create the game after we've created the grid since that makes logically a bit more sense now if we run the test again this is going to be problematic because now we need to change some of the game test because there we initialized it differently so so let's go back to those tests and fix the problem so what we need to do here is import a [Applause] grid so then we create a grid of 3x3 we pass it to the game and we provide the initial State like so we don't need to do this and here we can do exactly the same thing so we create the grid and we pass it to the game let's run the test there we go now the test pass again so lots of refactoring and changes in the codea I think we've really simplified the way that it works we've improved the testing we've solved the law of Dem meter violations that were happening throughout the code we've added many type annotations so that it's easier to change the code in the future I hope you enjoyed this roast of The Game of Life thanks again to Lup for submitting this project as a roast you can find it before and after version of the code in the get repost story I've put the link to the repost story in description of the video If you enjoyed this roast you might also enjoy this one thanks so much for watching and see you soon

Channel: ArjanCodes

Views: 27,030

Keywords: conway's game of life, conways game of life, conways game of life python, conways game of life code roast, game of life code roast, refactoring, game of life style, code roast, code roast python, refactoring python code, refactoring code, refactoring python, refactoring code python, game of life, conway game of life, code refactoring, python code review, python code roast, cellular automaton, john conway, code roast arjan, code refactoring python, python refactor code

Id: NOcwXmXr0ho

Length: 31min 48sec (1908 seconds)

Published: Fri Jan 19 2024