Refactoring in Python: Design Patterns and Approaches

Speaker 1 [00:04]

Thank you for the great introduction. So, people, thank you for welcoming me here. I'm going to be speaking about refactoring. I'm going to be speaking about what exactly is refactoring, how to do it well, what's the point of it, why even do this thing, and why not rewrite everything. Programmers usually have a habit of rather wanting to just throw away old code and do it their own way. Sometimes that's right, mostly it isn't. So, first things first, some personal introduction. My name is Tin. I'm a software engineer that enjoys coding, enjoys making these big systems, enjoys architecture of software, of getting people to make things greater than one person, of making teams work on large scale. I've been working on several projects, some of them open source, most of them not. And I've been doing this thing for a bit. I'm a lead engineer at Kiwi.com. I lead a team. We work on our main product, booking. We allow you to book flights. The company itself sells flights. We have a motto that is make travel better. You can check us out later if you want. I'm not going to advertise much more. I enjoy reading about fantasy. I'm a tea enthusiast, a board gamer, and overall a geek slash nerd. You can check out my homepage if you want. It has a bit corny name, and I'll leave that for later. So we're going to be looking at how to read from old code, how to see what your colleagues or your previous you or your drunk you have been writing right there. We're going to check which tools we can use to improve it, see how to do these incremental changes, see why not to throw away everything. I enjoy one principle that Wikipedia uses. It's Chesterton's fence. It means that there can be a fence over a straight road. And while that may confuse everyone, there's probably a reason why it was put there. So it includes that you shouldn't change things or remove things until you understand why they are there. Don't remove that fence. It may be that the road ends in a chasm a kilometer later. So yeah, we're going to check out how to modernize things, not reinvent them. And we're going to use something nicknamed bubble of testability that we'll try to introduce in our code base that's not tested. Of course, it's not tested. so yeah i'm gonna cross this in three sections starting with the easy wins moving to some patterns and anti-patterns things that people do when they have their hands free in the end we're gonna talk some philosophy and see how to improve this in a smaller company startup or your place of work what can you suggest to people to make them see the thing the same way you do so yeah Easy wins. Let's start. Those are the easy things you can do to improve your code base. There are plugins you can use, libraries you can import, utilities you can run every now and then. They don't solve the root cause. They don't fix your code base immediately and make it wonderful and pretty, but they're a nice step. They are there to jumpstart the whole process of getting to write something you like. So yeah, this boils down to automated code quality, to use tools that are going to be making your life easier, that are making your work cool again. You'll make decisions in your team or yourself that will save you so much time because they will be automated. You're not going to be coming back to them now and again. You'll decide them once and you're going to keep that decision going on. And that saves just so much time. Everybody likes to argue minor things, but it's not productive. It's not productive when you have to have a discussion about indentation level. Nobody actually likes having those discussions if they have productive work to do later. So yeah, moment. So let's start with the classics. PyLint and MyPy. How many of you have used one of these two? Raise your hands, please. Good, good. Majority of the whole. How many of you use MyPy specifically, typing in Python? Much less people. But yeah, typing in Python is somewhat of a controversy for some people. But I like it. I like it in larger teams. I like it when you have multiple people who have to read each other's code, typing helps. And the good thing about MyPy is that it doesn't force you to type everything. You get to type only the parts that are really crucial, that are good for you to use. If you have some wrapping code, if you have some old code that there's no chance you could type ever, you just leave it untyped. You try to figure out how it works around it. And that lets people go there per function, slowly, implement it step by step. It doesn't have to be this one day that you decide, okay, now we're typing. And that's the pattern we'll try to reach here. We'll try to reach things that can be done slowly, one by one, function by function, class by class. If you have that slow approach to a problem, you'll eventually solve it. If you try to make that one big rewrite, everything will crumble. You will create losses. Nobody will ever let you refactor anything again. That's not cool. PyLint, as most of you have heard of it, it's a linter. It's an established industry practice. I use it everywhere I code Python. And it's very configurable at places. That's why we also use Black. How many of you have used Black. Could I see or show hands? That's great. That's great. Today, or not today, but this conference, we'll have a developer of Black speaking. I look forward for that. And yeah, I love the tool. I think it's a great advancement for Python coding in general for the industry. It keeps code style consistent. It's simple to run. It's simple to keep running. Invoke it, just black. And there's no arguments about things like where to fold the line, where to break, where to not break, how to indent. It's all automatic. The great thing about it is that the code after behaves the same. It uses the AST to check the code to see that its output is always the same, so you have no risk of breaking things, which is just wonderful. Black every day. So, yeah, we also use pre-commit. You can just plug into Git's pre-commit hook, or you can run a tool that's listed here, pre-commit.com. It's a great thing to get a sync of your developers and your automated code inspection system, CI. Basically, pre-commit forces every developer who has it installed to run all the checks before Git allows them to commit. This is wonderful, as it keeps the developers clean. It keeps everybody from pushing things to CI that will fail. It keeps you using the tools and the checks locally, so it makes everybody behave, pretty much. You plug all these tools that we've mentioned into pre-commit, and it'll make everybody behave code-wise, code-cleaning-wise. It's synced. You keep it synced with the server. on your server. You run CI. There's probably going to be a talk or two about that. I'm not going to go into CI details. But you run it via CI, and you have a wonderful environment where everybody cleans their code before they push it to the Git. And that's great. So yeah, this is some contrived example of how somebody may write code, break lines, whatnot. not. This is after several runs of these tools. There's some comments because some of these don't let you have functions without comments. There's typing. It cleans up code that's commented because commented code doesn't belong in a code base. It belongs in the Git history. Generally, it's much more of a pleasure to work with. And most of this is automatic, so you don't have to worry about it on a code line basis. You run it, something breaks, you fix it. It's like testing. You don't have to think about it actively. You just follow the checklist until everything passes. So yeah, tools make discussions that are trivial not happen. It makes us think more about actual problems, about solving those problems, and less about things like line breaks, things we can fix easily and things we shouldn't think about, things that are very, very preference-based. It's an easy bump in code quality, but it's just a bump. It doesn't solve your core problems. It doesn't make your code easier to work with. Not the majority of it, some things, sure, but it doesn't solve your core issues. For that, you have to go into the patterns you use in your code, into the methods that your developers use to develop code, or you yourself. Codes that's hard to use, code that surprises you, that's the hard code. That's the code, the functions you call, and then you're astonished. You're not sure what the hell happened here. Who thought this was a good idea? I wanted to format a dictionary into a string, and I got a database hit. How did this happen? What did I do? That's the principle of least astonishment. You want your code to behave like you expect it to, code-based-wide. You don't want to run a function that will do some surprising things. Nobody likes to be surprised in their day-to-day life. And when you have legacy, it's often astonishing. It's often something that will surprise you, and you will not know what happened until you debug it for a couple of hours. And many of you have probably heard the phrase historical reasons. Why do we do it this way? It has been done like that since the start of the company. Don't ask too much. How many of you have had something like this happen? Near everyone. So yeah, historical reasons hurt. Things like this astonish us. It's painful. So yeah, you start looking into these things and you start seeing some patterns, some code smells, so to say. You have things that seem to be obviously wrong, but they're like that, and you're not sure why. Neglect, inconsistency, some redundancy there, some things are repeated, somebody copy-pasted something they shouldn't have. Things like that can be called code smells. And they happen because of a plethora of reasons, from code scouting to deadlines to just implement it now, we'll refactor it later, things like that. nobody ever does it later. So yeah, these things happen to everyone and it's best to cut them down slowly, one by one. There are some levels of this. You have some easy stuff that somebody just copy-pasting because they were stressed or not thinking right. There's some medium stuff that are mistakes that were made at the start of the project and they were hard to change. Then you have the hard stuff, hard smells that are very easy to notice, but nobody can figure out how to remove them without a full rewrite. That's the main reason for let's rewrite everything. We didn't use an ORM. We have raw SQL everywhere. And now it's very hard to manage. Let's rewrite everything. Things like that. So, yeah, these are some examples from our code base directly. Things like just currency checks that are all one under one. Why not? Some order processing that just takes a huge dict of data and returns a huge dict of data that nobody knows how it will behave. This function had 2,000 lines of just editing one dictionary. Your imagination can go wild. then the ORM thing where we need to have some way of parsing objects and not just using dictionaries everywhere those were problems we went through but they were very slow they were a very tenacious thing to do so yeah, detecting these and knowing about them is a very important part of getting your code quality on the next level so yeah, what we used here and what helped a lot was SonarCube. It's a tool. I think some parts of it are open source. It's a static analysis tool that goes through your code and does similar things to our other tools, but just a bit bigger, a bit better. It analyzes some bugs, some code smells. It finds some known security oversights, checks your test coverage, and plots these things. So you can see if your efforts are paying off. It's plotted. It's on the graph. So you see, it's either going up or down. It's hard to oversee that. So yeah, here's some examples of how it works. It can be plugged in into, for example, GitLab to comment directly onto your merge request. So you have a reviewer that's automatic and very thorough. That's great. Also, every programmer's nightmare. You also have a list of previous defects, previous bugs, that it has an explanation for. This is a great feature, actually. You can see here, some of those are just detailed with examples, with full reasoning, with links for more reading. So if you have a junior or two, or you yourself are not an expert, this is great. You can see why is this wrong? Why am I doing something and why should I change my way? It's great to teach people things. It's great to teach people why is code written a certain way. Try it out. So, yeah, find anti-patterns in your code. There are some anti-patterns you should recognize. And as opposed to programming patterns that are used everywhere, anti-patterns are usually unique to code bases. It's somebody's smart idea that they came upon organically and they invented it. And then after some time, after growing, after the company blowing up a hundred times, this becomes a bad idea. They realize why they shouldn't have invented it. The whole not invented here thing. So yeah, anti-patterns appear and they are very specific. I'm going to go through some that we've had. So yeah, we had these magical methods that I've already mentioned. Things that didn't have an input and an output. It's a dict, it returns a dict, have fun. It usually was an implemented side effect. You would pass it a dict that had some value, let's say a booking number, and it would access the database, get it, do some stuff, return it, and return you the data in the same dict. It would just look several levels deep, so if you didn't know exactly what you were doing, it's a nightmare. You don't know what type is it, you don't know how it behaves, you have to do some archaeology through the code to figure out how it's going to work. It's a problem. It's often prepared by just doing real object-oriented programming, trying to get some typing in, trying to figure out what's happening there, and making it explicit. We also had this unusual case. I haven't noticed it in many places, but we had somebody think that decorators are a good idea. and they were really into it. I've seen functions with over eight decorators at the same time. And if you think that's a good idea, it isn't. Basically, decorators, as a rule of thumb, shouldn't modify function signatures. They shouldn't add new parameters. I've seen functions like out be called with zero arguments. And then when I open them, they can accept up to 15, five of which are mandatory and it was called without anything but sometimes I wanted to quit my job so yeah be explicit when writing code don't do things like this that would make code golfers proud and don't try to replace good old function or method calls with some magic nobody likes that so yeah separate these things try to get rid of them and try to implement some patterns to separate it. Try to get some interfaces, not the Java-like ones, but just functions to wrap around your code. Try to get them there, try to insert them so your new code, your new code base, your shiny objects and classes can shine. Set them up, make a facade around them and run that through your colleagues so everybody knows how things should work. So yeah, what we used as some kind of an interface were just wrapping these old and crusty pieces of code with functions that had a nice signature, that had a very nice way of working with. And that signature then we reused over all the partial implementations. So yeah, we would invoke the database several times because each function decided it needs to get its own objects. But we would do it over a common interface that we could later rewrite and have it all load in at once. As an example, a function takes a user out of the database. We would rewrite it to use an object, user object, even though it doesn't need it. Already in the code, several times it calls into the database to pull that object. but we started the first step was to rewrite it to take an object instead of an id it was redundancy but it let everybody know okay this is how it should be done and people will kind of assimilate to it eventually so yeah you cannot do it as a one-time thing you need to wrap your code in these facades in these patterns that may not be functional at start but it lets you delete piece by piece the unnecessary code side effects there sometimes are unavoidable especially it's a very old code base you cannot just rewrite it to have functional programming happen now but they need to be obvious they need to be very explicit very like this function will edit the tables in the database don't use it if you don't need to so yeah have that manage it all in one place put it all in a single module we called our old module we called it monolith so the we import from monolith with very very uh explicit warning that this shouldn't be done unless you absolutely need to do it and everybody knows if it includes the monolith it's probably not very exemplar code so yeah abstract it uh do this both ways do this also for the uh old code and the new code like you need to implement something that doesn't fit your new paradigm you need to have it explicit okay we're gonna load these objects but we're gonna do it here so you can see that you shouldn't do this and it helps it helps to put these warnings everywhere because uh after we started explicitly writing all the warnings hey you shouldn't do this unless you're certain people have started improving in their code those graphs that i mentioned started going up because they saw something and they didn't copy it because it had a warning don't copy me new programmers usually just start by copy-pasting everything. So yeah, it's almost never pretty after growth, after you get your code base to grow up, to blow up, but you cannot really throw it away, not easily. So improve it gradually. Try to get a bubble of this code to be clean and to grow. And by having that perception that, hey, this is how we should do things, people will assimilate to it. people will start doing it more often. Yeah, talking about how people should behave better is great, but unless you put it down somehow, unless you put some rules so everybody follows them, it's not that great. People don't do things that are not enforced. You may want to have clean code, but unless you push people to do it, they're going to make a hack after second, after third, and it's going to be hard. so yeah uh try not to get these things to be an overall change of direction approach it slowly because people want stability in their work they don't want to start using both type old typing uh linting automatic formatting all the things in one week it would burn out anyone it would frustrate people who just want to do their job but add one by one over a month or two it works great. People get time to enjoy each of the steps and to figure out why you're doing it. So start code reviewing if you're not. Who here doesn't have a code review at work? Oh, that's a lot of people. Get somebody to check it, even if it's just somebody who works in a different company. It's cool to have somebody look over your code, figure out what you're doing wrong and just tell you this is a straight out bad idea. In our company, we have a responsibility split that a quarter of all that went wrong is on the reviewer. So basically, if you review something, you assume a part of responsibility. If your code that you allowed into the code base started breaking things, you'll be asked questions as well. It's not only the guy who wrote it. We require these tools to pass all the CI because then reviews are simple and about logic. You don't have to argue about variable names in a code review. And what's great, it reduces the bus factor. Somebody else will absolutely know what you're writing code about. So you can go on vacation without stressing out. It's a great thing. Keep it blameless. Blameless is number one. responsible yes but don't have some kind of a blame culture oh your code broke three times last week you're the worst person here don't do that impersonal so you shouldn't find patterns about the person you should find them about their code and you shouldn't figure out hey you write these functions wrong way you're a horrible person no he's not he's just confused and try to do reviews in three steps start with the overall scope like is the thing solving our problem go into the system scope is it solving them right does it fit our culture and then try to search for the small things in code like okay dictionary comprehension would have worked here and stuff like that nobody likes to have to change small lines of code and then in the end figure out oh oh, this is all wrong, so you did all that for nothing. Try to educate your employees or colleagues. Make sure that devs understand why is something happening. All changes, all rules have to be documented. And do that all the time. Because if you have a policy that's not explained, that's not decided, then people will just feel that you're unfair that you're giving them your preferences and making them rules that's not great for anyone every change you make document it, teach people why is this the better way and they will accept it they will keep with the changes and they will improve because they will know that what you're doing you're doing for a good reason otherwise it may help you understand that hey maybe your logic isn't that great maybe you shouldn't do this That's also great. There's no easily winning this whole thing. The small wins, easy wins are a step, but it's a step. It's all about steps. And tools cannot replace engineering. I've had this problem in the culture where every problem we've had, every big outage, every code depth incident was like, hmm, is there an application that can fix this? Is there a library we can import that will make our code great? There isn't. write code to be replaced if you add things new things always think about how easily you could replace them tomorrow or in a year because you will be it evolves technology evolves there's going to be a new library there's going to be a better ORM you want to know how to replace it without having to pass through the whole gauntlet of pain again less dependencies is always better easy reuse means that you can easily replace it as well if something is very portable that means that it's very expendable as well explain people especially management i have one more slide on that explain people how codept actually hurts it hurts productivity it hurts the bottom line after all eventually things crash things break and main thing is it's not the developer who's to blame. It's the overall matter that they are forced, or you are forced, to develop more things more fast, more better. K-men speak intended. So yeah, management needs some convincing every now and then. You may have a boss, a product manager, or somebody who says that, hey, if we go back and rewrite code, we're not making money. Yes, but we'll prevent ourselves from losing it later. Blame big mistakes on code and code depth, not on a simple junior developer who came and didn't know that this will explode after he makes that change. He didn't know because of a reason. And make sure it's not false flags. Maybe sometimes your code is good enough. It's not good to push for perfectionism if you're not doing software for a living. So yeah, low quality code is a symptom of a greater cause of a greater problem go for the cause step by step try to like isolate the rotting parts that make everybody have a headache and then change them eventually to get a great product this consistency of change by change every time in a better direction is better than having a hackathon where we will make everything great again because after that hackathon, things will fall apart again. Enforcing documentation, enforcing things to be consistent is much more important. So, read your code, figure out why it's done, realize that whoever wrote it, wrote it with a reason. Listen to it, see those reasons, modernize them, improve them. Don't think that you should throw it away and re-implement it. Grab the easy boosts along the way, because they're easy. Rewrite the failures in bubbles, try to isolate them into classes or special functions, modules in Python, and try to maintain that quality going forward. Try to be very explicit about why your vision is like it is. Thank you very much for your attention. Thank you very much, Sin, for this talk. Alas, we do not have time for questions today, but you can probably find them at the booths all over the week. I'll be all around, so feel free to reach out. Again, thank you very much. We'll continue in five minutes.