Even More Code Smells - Purge These 7 Python Putrid Peccadilloes Now!

in the past i did several videos covering cold smells in python if you haven't watched any of those videos i'll put a link to them in the description of this one some of you told me you didn't like me use the word code smell so here's a warning please skip the next 10 seconds of this video cold smell cold smell cold smell cold smell good smell gold smell gold smell 7. that's how many goat smells i'm going to cover in this video and there's a smelly little bonus right at the end while i'm working on the coat today i'll be using tab 9 who's also sponsoring this video tab 9 is an ai assistant that provides smart code completion in your ide it supports over 30 languages including python and 15 ides including vs code and pycharm tab 9 offers both a local model and the cloud model you can choose to run tap line locally only and your code never leaves your machine this also means you can use it to work offline while ensuring maximum security and privacy tap 9 recently launched tab 9 for teams which will learn your teams projects preferences and patterns suggesting even better code completions for you and your team members you can get tab 9 basic as a free extension to your ide of choice if you're a student you can get tap 9 pro free for more information go to tab 9.com students or if you're not a student you can use coupon code ariane to get a discount off the pro plan the links are in the description of this video today's example is a point of sale system that handles orders and processes payments there's a few files in this example we have a main file we have a pos point of sale package that contains a few other files an underscore init file it also contains an order file payment file and a system file so let's just quickly go through them and see what they do so first let's start with the order file this contains an order class and another class an enum actually that represents an order status so there's a couple of different possibilities order can be open paid canceled delivered returned maybe in a production system you'll have more options here the order class is a data class has a couple of fields these fields are related to the customer information so name address city etc then we have a few members for representing the items in the order so there is a list of items which contains the names of the items and we also have a list of quantities so for each item we know how many we have in the order and we have the price for each item in the order prices are expressed like integers in this example this is actually quite common in systems like this and most of the times this is expressed in the smallest possible unit of the currency so if this is dollars the price is going to be expressed in cents all of the values in the order class are actually initialized with the default value so most cases there's the empty string or the value zero there is a status which is initialized to an enum value because it's of type order status and we have an id that's also initialized to the empty string and then these things because they're lists we can't just simply assign the empty list in a data class because that's a list is a mutable thing and that's not allowed so we need to use this structure here calling the field function and telling the data class what the default factory is for creating the default value in this case that's the list initializer then we have a couple of methods in the order class there is a create line item method that gets a name quantity in price and that adds it to the various lists in the order and we have a set status method that changes the status of the order and this is going to be called by the payment system later on when the order is being paid so if you look at the payment system it has a custom exception class that's raised if there is a connection error with the payment service there is a protocol class called order repository so the payment processor relies on a system that provides it with orders and that computes the total price of an order for it so it can handle the payment for that the payment processor itself is a regular class it has an init method that gets the order repository and then we have a method for connecting to the service for the moment this doesn't do anything but if you'd have a real payment server then here you would set up a connection it sets a connected instance variable to true so we know it's connected and when we process the payment there is a check in the beginning so for not connected then we're going to raise this custom error that we had here the payment service connection error then in order to process the payment so it gets an order id so it retrieves the order from the order repository it uses that same repository to compute the total price and then it processes the payment of that price then the point of sale system itself this is another class with an initializer it creates a payment processor it has a dictionary of orders that it maintains then we have a couple of methods one to set up the payment processor so that calls this connect to service method we have a method for registering an order and for finding an order so that actually maps back to if you look at the payment system we had this order repository we had a couple of methods here so find order is here and you see that the bos system also implements that method and it also implements compute order total price so that means it can be used together with the payment system process order then calls process payment once it's been processed it sets the order status to paid and then it prints shipping order to customer if you look at the main file it basically uses these classes to do some basic things so we create the pos system we set up the payment processor we create the order create a few line items we register and process the order and when i run this code then this is what we get a code smell is a hint that there might be something wrong with your program it can be a pretty minor thing to fix or it can point to a bigger design problem so it's different from a bug in that the program might work perfectly fine even if it has cold smells but there might be problems when you try to maintain or extend the code in the future i'm going to cover seven of these code smells now they're not all python specific so you can apply them to any other code base in other programming languages as well so now let's clean up the code so we have all these different instance variables here and there's one particular problem which is look at these three instance variables they're all separate lists used for storing item names item quantities and item prices and this leads to a problem because now you have to keep track of three different arrays you have to make sure that they're always the same length otherwise doing things like computing the total price etc is going to break so it makes way more sense to actually combine these three things into an object and then maintain a list of objects and then that object has an item name a quantity and the price so that's a common problem you see in a lot of code where you're using data structures that are actually too elaborate or that do not really match what you're going to use the system for this shows how important it is that when you design software that you create the appropriate data structures so in this case the solution is that we create a separate class to contain the item name the quantity and the price and let's call that a line item so i'm going to create a new file here inside the ps folder and let's call that line item so for line items we're also going to use a data class this is a class line item and what does it have well it has an item which is a string it has a quantity which is an ins and it has a price which is also an int we're going to do a bit more with line items later on but let's keep it really simple to solve this particular problem and then let's go back to the order class so instead of these three separate lists let's just create a single list of items so i'm just going to delete this and i'm going to insert items which is going to be a list of line items there we go and we're going to use the default factory like we did before so that's also a list and line item we need to import let's also update the create line item method here so instead of passing a name quantity and price and then creating the line item in here we could just pass it an actual line item the advantage of doing that is that this method then no longer needs to know implementation details of what a line item is it simply adds it to the list so that's a line item and the rest we can delete and this is going to be an item and these two we can also delete so now it simply creates a line item or it appends the line item to the list of items the second gold smell is using misleading names it's actually related to what we just changed here here for example this method is called create line item but is it really creating something no it's not it's simply adding something to the list of items and there are more cases where you have to think carefully about how you're naming things like create something get something delete something and think what are you really creating getting and deleting things or whether you're actually doing something else and being precise about what you're doing can also help in establishing who is responsible for maintaining the object that you're creating or that you're getting so in this case let's simply rename this method to addline item since this is actually what it's doing now obviously because we just changed the signature of this function to accept a line item instead of a name quantity and price we should also change this in the main.pi file because obviously this doesn't work anymore so what addline item expects is a line item and i'm going to automatically import that and we're going to use it to create these different line items that we add to the order so now in the main file instead of passing these three values directly as arguments to the addline item method we create a line item and then add that instead the third code smell also has to do with the order class you see that even though we changed these three lists to now have a single list of line items order still has lots and lots of instance variables and when a class contains too many of these instance variables this is often a sign that it has simply too many responsibilities in this case there are actually too many responsibilities because not only is the order responsible for keeping track of the items that are in it and the status of the order but also for all of the details about the customer so it has a customer id name address postal code city and email and that doesn't make a lot of sense order should be a bit more focused on only the stuff that's related to the order and the problem is once you start expanding the system and making it more complex perhaps you want to do other things with customers as well and now this is not possible because it's all directly part of the order class so that all points to us having to separate order stuff from customer stuff so in order to do this let's just create a separate customer class so i'm going to add another file here called customer let's also use a data class to represent the customer so i'm going to automatically import that and create a customer class and this is going to have a couple of fields that's going to be an id yeah that was an integer i was not entirely sure if we used an integer or string then we have a name which is a string we have an address we have a postal code so you see tab 9 is actually quite helpful here and filling in the details for me so we have city and we have an email address so that's the customer class and then let's first change the order to actually use the customer class instead of all these separate fields here so i'm going to have a customer and that's going to be of type customer so let's delete all these lines here and now order is much shorter it has a customer it maintains items it has a status and it has an id which is really much better in the main file i was going to need to update a couple of things because now i'm passing all this detailed information about the customer directly to the order initializer so instead of this we're going to need to create a separate customer and then attach it to the order so let's remove this like so and then i'm going to create a customer here and we're going to pass all this information to the customer and customer we need to import it and let's see obviously it's expects an int there we go so now we have all the customer information and then for the order i can replace this by passing the customer directly so now we have the customer and we pass it to the order and which is nice now because we can use the customer object in different parts of the system because we now separated it from the order when you encounter a method that gets a single object as a parameter and that only does something with that object that's also called the verb subject smell where the verb is the method and the subject is the object the positively worded version of this is the ask don't tell principle don't ask an object for all kinds of implementation details and then perform the computations yourself but simply ask the object to do the computation for you an example of this is compute order total price so obviously it's broken now because we changed things in the order but you see that this gets an order as a single argument and then it just gets data from the order computes things about it and then returns the result so because it needs this data from the order it's actually much better to move the total price computation out of the point of sale system and into the order class where it makes much more sense i'm going to delete the method here that's going to break something else but we'll fix that later so i'll remove this here so we can delete all these lines and i'm going to save that file and then i'll go to the order class and then we're going to add the total price computation right here let's also turn that into a property that's one thing and the second thing is we have a line item that can actually also compute its own total price so let's separate the total price computation over the line item class and the order class so i'm going to start here because here it's a pretty straightforward way of computing the total price simply using a property and let's call that property total price and that's going to return an int obviously and that's going to return the quantity times the price and why do i get an arrow here oh this should obviously be an arrow and the column should be here let's go to the order class and then use the total price property of line item to compute the total price of the order so i'm also going to define the property here that we're going to call total price as well and that's also going to return an end and now one thing you could do is use a for loop like we did in the original compute total price method to compute the total price of the order so let's create a total variable that's zero initially and then we're going through the items and then we call the total price property of the line item and then in the end we return the total so this computes the total price for us there's actually a shorter way to do this without using a for loop slightly more functional approach so i'm going to change this into comments and i'll show you what i mean so instead of using a for loop you can also use the sum which is actually really straightforward to do that instead so we're going to compute the sum of the line item say we have a line item total price for each line item in self.items and that basically takes the sum of all of the total prices and returns that as a value of this property so now because we removed the compute total price from the system we've run into another problem in that the payment processor expects to have a method that computes the total price for it when you call a method but don't provide all the data that it needs that method is going to need to look around to get the data in different places often leading to the method having to know implementation details of things that it shouldn't know this is called back battling and it's something that you should avoid a related principle is the law of demeter or the principle of least knowledge and that means that you should try in your design to make sure that separate units in your code only needs limited knowledge of the other units a side effect of not applying the law of demeter is that you often get these change of object calls so you have self.object1.object2.org etc which means that the method that does that has to know all kinds of implementation details about these different objects that it's calling and you can kind of see that happening here where it needs to know if you want to process a payment that you get an order id you have to find the order then you have to compute the total price via the point of sale system so now that means that the payment processor needs to know a lot of details about how the point of sale system is set up and that's not necessary because in the end if you look at what the payment processor actually needs to process the payment is it needs some kind of reference well that's the order id in this case and it needs to know the total price that the customer has to pay that's the only thing that it needs to know so instead of letting you do all this work of retrieving the order computing the total price etc itself why don't we simply provide that data directly to the process payment method so it could simply use that and then do the job that it needs to do and that's going to really simplify the payment process because then here we can replace this parameter with something else so we can pass a reference which is a string and we pass a price which is an int we don't care about the order here anymore we don't care about the total price anymore but we simply write the price here so that's the price and here's the reference there we go and this is basically all the process payment method needs to know and now the nice thing is because we're no longer relying on the point of sale system here we don't need it anymore so i can simply remove it here i can also remove it as a parameter to the initializer there we go and that actually means that the payment processor is now really simple and we actually also don't need this order repository protocol anymore so i'll just remove this as well and that means we can clean up these imports as well so you see this actually reduces the the coupling between the different elements of the point of sale system stripe payment processor is now a completely disconnected thing from the rest of the point of sale system and then we have another problem if we want to process the order then of course we need to fix this and that's not actually really simple because the process payment method needs a reference so that's already here that's the order id and we're also going to pass it the total price and for that we can simply use the total price property and we're done and then here when we create the payment processor we don't need to pass the point of sale system as a parameter anymore and now let's run the code and just verify that everything works as it should so that seems to work correctly code smell number six is having hardwired sequences with a fixed order and if you forget one of the steps then you're going to break the system currently we need to make sure that when we create the point of sale system we also set up the payment processor correctly if we don't do that we're going to run into a problem that happens here but you also see that when we look at the payment processor as an initializer and it has a connect to service method and in the point of sale system where we have to add this extra method to set up the payment processor with the url so it can connect to the service so generally it's better to do this automatically so you don't have this dependency now in this simple case you could actually call the connect to service method in the initializer but often the reason that this is split is because connecting to a surface which is probably somewhere on the internet takes time and you need to wait for that so that's an asynchronous method call and unfortunately initializes of classes cannot be asynchronous the way to solve it in this particular case is to add a static method because we don't have an instance yet that creates the instance for it a static method like this could actually be asynchronous and then you could have other asynchronous operations that you have to wait for so let's create a static method called create static method and then let's call the method create and that's going to get the parameter that it needs in order to set up the connection so that's the url it's going to return a stripe payment processor and then what it's going to do is create an object it's going to connect to the service passing it the url and then it's going to return the object you see that we get a type error here it says that stripe payment processor is not defined the reason for this is that we're defining this in the definition of the class and then the types are not yet annotated so the type system doesn't know about it yet this is a problem in the current version of python but in future verses they're going to actually solve this and the way to get around it in the current version is to actually import the annotations from future and you see when we import this then it actually is able to resolve this particular type reference here of course you could also leave out the type hint but i'm a big fan of using type in so i prefer to do this and probably in a future version of python we can just remove this particular import again because it will have this behavior built in so now we have a create method and then what we can do is go to the point of sale system and then instead of doing the with the work here and having this separate method we can actually create the stripe payment processor only once now the problem is that the payment processor is still being initialized here it's been created by the point of sale system and that's the seventh gold medal you should avoid having objects create other objects explicitly unless it's really necessary if we let the point of sale system initialize the payment processor explicitly here it also means this now depends directly on the stripe payment processor if you want to replace it with another payment processor then this is not going to work because it directly depends on stripe so instead of doing it in the initializer let's pass it as an argument and use dependency injection instead and that means we don't need this method anymore because we're going to do that somewhere else if you look at what it actually needs from the payment processor it just calls a single method called process payment so we can use protocol class to define that interface of what it expects let's say we have a class payment processor which is going to be a protocol that will automatically import and the payment processor is going to have a single method called process payment and that will have a reference which is a string and it's going to have a price which is an int it's not going to return anything simply going to process the payment and since it's protocol class we don't provide any implementation and the standard way of indicating this is using these three dots and then here we add the payment processor as a parameter and we're going to store it in the object and now you see we can remove yet another dependency here because we don't need the stripe payment processor anymore and in the main file instead of doing this the first step is create the payment processor and that's a stripe payment process but here we're going to call the create method and then we're going to pass the url that it's going to need and the payment processor is passed as a parameter to the point of sale system let's run this again to verify that this still works and it does so now the nice thing is we're explicitly creating the objects here the point of sale system no longer does that on its own so that means it's also easier to test here's a bonus code smell if you create an object like let's say customer or maybe even a line item it's not really clear what all these arguments mean i mean of course you can kind of guess that this is the first name and this is the street and we have the postal code but instead of relying on the developer looking at the actual values which often you won't even know because they're read from a database or something it makes a lot more sense to actually use keyword arguments more explicitly so you know what kind of arguments you're setting and the other advantage of that of using keyword arguments is that you can actually change the order without the system breaking if i would change the order of the postal code and the city then my keyboard and ssd drive and use and usb cable would not arrive at my home so it would not be very happy so instead of directly passing these arguments like this it's better to actually use keyword arguments to clarify what they actually mean so this is an id and you also see you get the error that if you define one keyword argument you need to do the rest as well this is the name of the customer this is the the address of the customer we have the postal code we have the city and the email address and since i'm using the black auto formatter it also puts it nicely on separate lines which is much more readable than it was before so i have the customer and using keyword arguments now really clarifies it and you can even do this for the line item so the item is a keyword quantity is 1 and the price is 50 and even in this more simple case it helps because we might not always know what is the price and what is the quantity here it's pretty clear but let's say i wanted to buy 500 600 usb cables then i wasn't sure if i was ordering 500 cables that cost six dollars or 600 cables that cost five dollars and now at least it's very clear what it all means so let's run this one more time just to make sure that all of this still works correctly and it does i hope you enjoyed this code smell video if you did give the like consider subscribing to my channel if you want to watch more of my content thanks again to the sponsor of this video tab 9 check them out via the link i put in the description of this video thanks for watching take care and see you next time

Channel: ArjanCodes

Views: 19,836

Keywords: code smells, code smell, clean code, python refactoring, python code smells, code smell python, python refactoring exercises, vscode python refactoring, software development, code smell test, beautiful python refactoring, code refactoring, clean code python, code smells refactoring, python refactoring tutorial, code smell detector, software development tutorial, code smells python, code refactoring vs code, Software development course

Id: Kl3_Gmn4Ujg

Length: 29min 43sec (1783 seconds)

Published: Fri Nov 05 2021