Python Code Review: Debugging and Refactoring "Conway's Game of Life" + Automated Tests

Key Takeaways

hey laib um first of all I hope I pronounced your name correctly uh laib that's a really cool name um I took a look at your code I just cloned the repo right now and it's like literally s of the first time I'm looking at this uh I just checked it out and I've got the the test file and your your grid. Pi file open um first of all I want to say it's it's really awesome that you're writing tests and then you're you know splitting this up into separate modules and uh it actually seems kind of clean I was I was really impressed by that so good job with that um you can see here that I've got it open in my my Sublime Text Editor and um I hope you can see that in the video but it's highlighting some some stuff here so these are warnings from a linter a code linter tool that I use it's called flake 8 and I I just wanted to show you um what what that does because I think could be a really helpful tool for you to you know avoid some of the common mistakes and just to get your formatting consistent I mean honestly with the formating if you're just working on a on a pet project for yourself then it doesn't matter as much but um I think it's good to build a habit and just kind of you know stick to some style guide and uh there's some really good tools for that available so I wanted to show you this one um it's called flake 8 and you can just install it through pip so I've actually already got it set up um and it's it's really easy to run so even if you don't inte this with your editor you can just go uh flake8 and then just give it all your python files and you can see here that it actually finds a couple of things right in in your grid. piy and in your test life. piy most of them are formating related for example here in this case there would be um a missing uh new line between these two functions or methods here you know that stuff isn't too terrible I I personally I still believe it makes sense to clean it up but you know it's not going to affect um sort of whether the code works or not like in same with this stuff here you know missing a space if you want to go with that go all the way with the formatting stuff um that that's all okay the cool thing however with flake 8 is that it also does some uh static analysis on your code so it can detect you know syntax errors and uh it can also find um things where you're not following the the the python best practices so for example you know in this case we're comparing um we're checking for truthiness with uh this equals equals true and typically what you do in uh in Python is you would either use uh the is operator um because that's going to make sure that uh you're you're checking for an instance of true you know not just a specific uh instance of true but you're you're kind you you're looking for something that is true itself or some subass or you know some other instance of true so usually you want to check for truthiness like that and actually you could even simplify this and just say if self is alive and then uh the the if statement here will kind of check that for truthiness right so I mean it's it's just the little things it's not going to affect the um the way your code works but uh stuff like that I find really helpful in you know in finding place where you can simplify the code so I just want to go through this like you know skim through this and then see see what we can do here so okay so I think this is interesting where we're having the global here um probably the first thing that stands out for me here is this comparison so we're saying okay if self live Grid live grid and then we're checking that so that makes sense here then you want to return true or false so you can actually shorten this and you could just say return and then what that does you know it's it's basically going to evaluate this statement here or this expression and then uh if it's equal to one then it's going to return true and um if it's not equal to one then it's going to return false so you you you know you can just get rid of some of these lines and then uh what I found interesting here about the the life grid stuff I don't really understand why we would do it like that so I'm seeing you've got this create grid here and I saw this over there where you're you're creating an instance of your grid class so that makes sense and then you're calling create grid on it which um assigns the grid to life grid so my thinking here would be whether whether that's the best way to Do It um because really you're you know here with create grid you're just basically giving this guy a new name here right just giving it another reference so when we look at this test here for example when you change you modify IP that would also affect um the live grid here in this class and so I'm that could be you know that could be what we want but it could also be a little bit dangerous if we actually just assign it and don't copy it over so I would maybe think about that design Choice um okay so you know what let's let's just go do a pass on like the formatting stuff here so we can get that out of the way right saying okay want space in here you know and that stuff doesn't really matter all that much it's also going to complain about the line length being too long stuff like that we don't really have to worry about that too too much um just want to make sure we're actually pep eight clean and I I really like what you're doing here with the um with uh your your print statements I think this is this a pretty good way to debug you know a lot of people say oh you shouldn't use um shouldn't use wait what's going on here here you shouldn't use print statements to do your debugging you always want to use a debugger but um honestly I don't believe in that you should use whatever gives you the best result so okay so it's I don't know why it's complaining about these what we're going to do is just clean up these oh yeah okay probably about missing space or something too many blank lines yeah so you know this this seems maybe seems a little bit like retentive if I'm doing this but I really like following pep 8 because it makes it a lot easier I find uh just to you know mentally parse the code and and understand what it's doing so with this guy here you know you could think about formating it slightly differently um because then it it becomes clear that this is uh kind of two-dimensional structure um I think is nice because it may makes makes a little bit more obvious what's going on so I think these two are the same um so I'm just going to select all of them and then do a oops oh that's weird okay I thought that worked okay so let's just do that then it'll be easier for me to understand kind of what's going on here um okay I think that one's the same too and here let's also reformat that guy yeah so now we're getting rid of all the the linter errors here and just kind of normalizing the way our code looks okay so now it's it's complaining about a couple more things so here for example um again we're comparing to true or false with uh equals equals we want the is here um it's fine for for the numbers um so with these messages what you could actually do is you could move them into the assert right so if you do a comma and then you could say um left Edge neighbor missing or something like that and then if if that assert fails you'd actually get that as a print out on your console so um you know you don't have to do it but could be a neat trick to just kind of uh make that a little bit easier for for you because otherwise you have to go back manually and then kind of check check these so so this is a bit weird that they're uh that they're just strings right like if it might be easier if you're just or you know what I would do would be something like this where I put just a regular comment instead of a string CU also the comments are going to get stripped and then the string is is going to be processed by the python interpreter so you know it's not going to make a performance difference really but um in my mind this this should be just a regular common okay so this is definitely something uh you could look into with the the uh Boolean comparison here um but other than that I mean here you could also just do you know assert is alive and you could you could actually drop that right so instead of doing this you could say Okay assert not is alive that's that's probably actually what I would do um unless you want to absolutely make sure that it's returning a Boolean but um honestly like with these I would probably just say check that they're truthy or not truthy and then it just makes that a little bit easier and I feel like it makes the tests a little bit easier to read um because you're you're stripping down some of that uh that extra verbiage here okay so this one we want to be true okay I hope this still this is still going to work at the end when we're done uh okay okay so now you know now all the linter stuff should be clean so let's let's run flake8 again okay line too long it complaining about Oh weird I don't get that in my editor yeah so I mean you know technically that line would be a little bit too long um we could simplify that a bit let's actually leave it like that that's okay and then what's happening here oh yeah okay so it doesn't like this print space stuff so really this is just a regular function call and um you don't necessarily need that space I mean you can put it if you want but um you know it wouldn't really be the recommendation okay let's actually try and like cut this line down a bit um so we're just oring all of that together so what we could do here is this probably um split it up here like this uh because I feel like that's that splits it up pretty logically I'm not mistaken oh okay yeah so we need to indent it like that right and now okay so this actually flate flake8 clean uh whoop nope that was too much so P test and then test life okay let's see what happens it's going to bump that up a bit okay so okay so we got test create grid all of these worked test num Neighbors so is this actually a bug that I introduced I'm not sure so where's this happening or the cell in the middle okay so let's just make sure I didn't I didn't screw this up um trying to test again yeah okay so I definitely definitely broke this here for you so let's undo that just jumping back and forth uh between between versions in in so what I edited in what was in my git uh yeah okay so it's probably copy and paste right so accidentally when I when I copy and pasted that formatting change when I broke it okay so now we've got this cleaned up um got this cleaned up um I'm just going to make a commit here just to take a snapshot of this cleaned up version right and then we can dive in into making some change changes um okay so commit that okay right we're going to commit that and okay so now we run this we're at this point where test apply rules blows up and um we're getting that error here so now actually before we go into that um I have a suspicion when we look at this apply rules function so what we're doing here so we're creating a new grid B then we're calling create grid right and when you look at create grid what this does is it really just assigns the live grid variable so at this point um at this point a live grid and B live grid are pointing to the same list right so we're not creating a copy which means if I change a. live grid we're also going to change b. live grid and so my suspicion would be that um you know as we're iterating over this thing that uh we're creating new cells and killing cells in B however what that actually does is because they're the same thing it's also modifying a which means that your your neighbor counts are are not going to work right so what I would probably do is um I would try in terms of debugging this let's not do this in terms of debugging this I would try to actually make a proper copy of your life grid with the copy uh module so let's give this a shot and just see where where we're getting with that so I'm going to import the What's Happening Here import the copy module and then here instead of just assigning that we're going to say copy deep copy IP that's going to create a new copy of that list and now if we run this it actually passes so the problem here was you know like I said said uh they were both pointing to the same list and as you were iterating and making changes to B you were actually affecting the live Grid in a because they were one and the same thing you know there were not two different copies they were one and the same thing so this was really screwing up your algorithm which was otherwise correct it was just this this piece here that was missing that you needed to make a copy of the life grid so so this fixes that one problem right in the apply rules so now all of your tests are are passing so now I would probably make another commit [Music] um and then what there's one more thing we can take a look at and that is um so it's a I'm I'm not exactly sure what you're doing with the with this here Global Life grid so probably what my gut feeling would be on this stuff um probably get rid of that and I would introduce a proper Constructor here right um I probably do yeah probably do this right so we're instead of calling create grid explicitly I would just say okay we're introducing here that're introducing this thunder init uh method and then I would just say self life grid basically this do this and then actually get rid of create grid here we don't need that um we basically don't need these globals because you know this is just a class level class level um attribute now like this is just an attribute on on um on that grid class here so we don't need the global here so I'm I'm not sure why that was in there but we were not going to need that for now um yeah this I think this is pretty neat actually what we're doing here where you're just you know giving the offsets and then um and then adding up the count this is pretty cool man this is this is nice yeah well done you know there's probably a way to simplify that um with the sum function and some crazy like generator whatever but this is this is pretty cool yeah I think this is a good function um okay so here you're just setting that to zero and one uh potentially you could think about making these constants right having having like a special value for dead cell and Alive cell or something like that so that they're not like these magic values but um you know I think it's fine for for a small uh small piece of code um yeah and then okay so what I introduced here was the Constructor right so I got rid of the create grid um so what you can what that means now we can just instead of calling create grid we we just be calling grid and I want to update the tests here as well um because really what that does is it makes it a lot easier to work with this class I think or I think you know I think this the other way to do it would work too but um it's really powerful if you can just oh yeah that's my timer it's really powerful if you can just um work with uh with the grid directly right like the the whole point of this of creating a class in my mind is that that you can just create a new instance of that and you pass everything it needs to this Constructor so really what what this allows us to do is we can say um we can say my grid equals a new grid and then pass it something and it's going to create a copy of that and that's going to create a new uh a new grid class for us and then everything you need you actually pass to the Constructor right and this is this is really similar to the way you would um what's a good example this is you know if you're if you're thinking about something that's built into python um if you're just doing list for example well maybe maybe list is not a good example but um if you're creating a new object you really want to give it everything it needs for it to create itself and so you don't have to remember calling like a special create grid method and and stuff like that so so I think this would be the you know the more pythonic way to do it um okay so where was I getting distracted here okay so basically what I'm doing now is um refactoring all of that stuff to not use the create grid uh method anymore because we won't need it and that you know that makes the code a bit shorter as well okay and now because we have your awesome tests here just make sure it still works okay so we're still you know tests are still uh running clean which is which is great and um probably what I do here yeah okay so I think this is a bit um I don't like this line because it can really lead you know to all of these weird issues where we're assign um we're updating the live grid kind of by swapping it out which you know that's okay but then um it's uh it can get a little bit dangerous here so you know I'm wondering okay so at the moment this apply rules function it Returns the grid itself right it it returns a.if grid so what you could think about doing first of all is [Music] um making this guy here a method on the class so this is probably what what I would do um so you just say self and then what this would do is it would create a copy of itself create a copy of itself and then instead of okay so so let's let's call this um it's called a state okay let's call this current grid right let's call this guy current grid and then uh we could say Okay current grid and then we're checking everything on the current grid and we're updating ourselves right and then I think it becomes more clear what Our intention is it's going to get rid of that so we're updating ourselves and we don't return anything because we're we're mod modifying our own uh life grid here um and so I would say okay if you're running this I would just say okay so if if you know want to use that code now so I would just say um a apply rule tools and then I check what this does yeah so in that way you know um we we've we no longer need to do this reassignment because we're take we're creating a copy of the current state and then based on the current state we're updating ourselves in the supply rules method here and um I think that makes the code a little bit easier right because you can just go a do apply rules and you know it's it's going to compute the next state and kind of update itself and um that's probably how I'd do it all right so this is a bit of a long and rambly video but I I hope it helps you out man I I think you you did really well with this stuff I was really impressed by how you actually split it up into separate test code and um it was you know you were so close it was almost working was just this one thing with the copy and you know what this was tripping me up at as well a couple years ago I I was banging my head against the wall trying to figure this out because it's such an easy mistake to make in Python so um yeah you were really really close and I think this looks good so what I'm going to do now is [Music] um I am I'm going to probably going to create a poll request and send you this video and then yeah you know maybe you can learn something from that from it if not that's also cool and I hope that helped you out all right I got to go back to work