Тёмный

7 Python Code Smells to AVOID at All Costs 

ArjanCodes
Подписаться 250 тыс.
Просмотров 373 тыс.
50% 1

Опубликовано:

 

1 окт 2024

Поделиться:

Ссылка:

Скачать:

Готовим ссылку...

Добавить в:

Мой плейлист
Посмотреть позже
Комментарии : 618   
@ArjanCodes
@ArjanCodes 3 года назад
SOLID design principles help avoiding some of these code smells. Watch this video to cleanse yourself ;) : ru-vid.com/video/%D0%B2%D0%B8%D0%B4%D0%B5%D0%BE-pTB30aXS77U.html.
@timstoev5607
@timstoev5607 3 года назад
good points, but most of that is refactoring related. In reality when you need to push it out, the entire code-base can and will get bugged up. Perfectly planned and executed projects exist only in the books and tutorials, mostly because there are no clients on the receiving end, or deployment requirements, or exotic randomness specific to the project. Reality is smelly, dirty and scary, and you, the dev, are up against it on your own. Against your team, which is fighting its own battles(hopefully project related- toxic environments are pretty bad). Against the system architect who doesn't give a shit for your 5K lines of code until it ends requiring interface redesign or comes up as a performance bottleneck during profiling. Against the boss who lives in a timeless fairytale of semi-random deadlines, just as a way to avoid the fiscal nightmare. Against the client who expects everything, requires nothing, but piles demands constantly. You are like Rambo- if you need to to kill 100 enemies with 5 bullets you need to make it work, like now(which would be great, if it was real- you needed to do it yesterday, which means you have to get that time travel stuff sorted out ASAP, from the cell, without the guards seeing you, and not chicken that's from a different movie cheater), you'll figure out the paperwork later. An yes you have to deal with the future you whining that the code you are writing now sucks, but consider that- if your future self likes the code you wrote now, you haven't learned anything while wring it, so probably programming is not your forte- maybe try QA instead On the actual points you make: Code duplication is bad, but messing with already tested and released code is worse. Even if it is such simple fix, you will be surprised how strange a released and tested code behaves once you start 'optimizing' it just before production. Underspecifying variables is bad, but if it passes code-review you are stuck with it as it gets too expensive to fix the mess as it tends to grow. The same is true with variable names, AND NO "REPLACE ALL" IS A BUG IN THE EDITOR. DO NOT USE IT UNLESS SUPERVISED BY A SENIOR SUPERVISED BY A SENIOR SUPERVISED BY A SENIOR... (SUPERVISED BY A SENIOR) Boolean flags are nasty but features on larger projects tend to come out of the blue, so learn to live with the dead weight. Just know that it is a dead weight and not a goto fix. Ignoring exceptions is a mixed bad. In general you should never do it. in reality however there are certain parts of the code that can throw, but are accessed in such fashion that the exception does not need to bubble(threading, communication, certain UIs). Then there are the order of operations problems and asynchronous callbacks(combined, because performance and shit) where you can get pretty crazy outputs, so exceptions are not really something you can rely on to normalize the code flow, but you still need to account for because the interpreter fires them. Adding loggers there is also dangerous(trying to trace a thread that throws and logs the exception can cause the entire machine to freeze), so sometimes you should simply let it go. The trick is to know where you can get away with it, but unfortunately that comes with experience. The best advice there is, if you are not sure, write comments. Make them long, don't be shy to explain yourself, you will find out that while trying to explain the smelly part in the comment you will actually find a way to fix it, and if not you will leave enough information to fix the WTF problem, that inevitably comes during reviews. isInstance(there was one more actually, just can't get the name off the top of my head) is required nuisance. I hate it in general, it should not exist, but because of the way python handles OOP it is a helpful hack. As the development progresses you will find yourself going for it, since redesign of the entire architecture is way too expensive. The other two are a bit more complicated: Using built in python functions is ok, as long as you are sticking to the python ecosystem. Modern projects however tend to stretch the stack, which inevitably will introduce other languages and frameworks. Those come with other people(hopefully) that can handle them. It is a nice fantasy that those will remain separated during the dev-cycle. In reality the teams(or you) will have to adapt parts of the stack that are outside of the particular tech domain to get features running. This is why you want to stick to programming fundamentals as much as possible, Yes if you want to cast a list to a dictionary you will use the zip function(or whatever its name was) and not write one yourself, but iterating an array does not need to be obfuscated just because python allows you to do it. You are not gaining that much performance, the amount of writing time is negligible, but the readability of the code is priceless. Do not forget that an experienced engineer, can start reading python in minutes. The language is not that complicated, so really give the guy(or yourself) a chance to do work on the project and not the code-base. The same is true for most of the modern languages. Please do not go exotic because the language supports some neat convention. Unless it gives you a real boost in performance(in which case adding a comment that points it out is really, like really neat, way better than the language feature itself actually), keep the things simple and straightforward. On the custom exceptions, you have a point, but honestly there is much that can go bad there, so I wouldn't recommend it for such video. Most of the people that watch it are probably lacking enough experience to make the call when and where to use custom exception. The policy for exception handling alone is a pretty deep topic, that is depends on the application, and/or the particular layer that you are in. So yeah, I do agree with the empty exception handlers, but lets leave custom exceptions. An overly-inspired junior being handed a relatively simple task, with the power of a simple language like python can do a lot of crazy shit, if s/he goes on and decides to showcase his/her knowledge of the language fundamentals, while forgetting that it is the programming fundamentals that matter, the language is just a tool that does the job. The latter is something that I would love to see promoted more often. I really need programmers than can solve programs, not translators from natural language to a set of computer languages. Possessing knowledge for the language is indeed important* but most modern languages are so similar from programmer point of view that the extra knowledge you throw in ends up bugging the project in general. *if you are doing C or C++ things are different, there knowing the language is required, but even more you need to know the machine you are building for, not to mention the libraries you are using. If you can cover all three inside out, you might handle the project relatively well, if you are disciplined enough that is
@brane2379
@brane2379 3 года назад
Who cares about Python code smells. Whole thing is c**p. Python is not even a computer language - no computer ever has actually executed python program. Instead, program is written as a series of one line jokes that cores laugh at while reading it on their free time at local bar, while laughing maniacally at each one and competing at trying to guess what shit comes in the next line. PC term for this is "interpreted language". On top of that, it comes with its own friggin ecosystem that whole distros are supposed to build araond and adapt to. And users are expect to wonder what little change in next Python microversion has shattered their duct-taped program, without any care about the task they are trying to do. I've yet to see ONE great program, written in Python, that is really worth using. Python was meant to e a duct-tape for in-house use, not as crucial component that one intends to sell.
@punkisinthedetails1470
@punkisinthedetails1470 2 года назад
Cleanse your :
@dariuszspiewak5624
@dariuszspiewak5624 2 года назад
@@brane2379 Some people say something because they have something to say. Others say something because they have to say something. Guess which group you're in...
@littleconan7929
@littleconan7929 2 года назад
the "try execept" thing always bothers me. I understand the idea, but sometime, when you are depending of an other complicated code, that might generate any Exception... well you do not know the kind of exceptions you are supposed to kind, nor if you forgot one exception type. So you execute this code and pray for nothing wrong to happen. And yes this might be the result of a poorly designed code, but you are not always able nor authorized to check/modify this code. Is it realy a good thing when for instance processing a file to have. except IoError (yes but an IOError could occur during the processing, not only during the file reading) Except OsError (the same) except ValueError except KeyError .... ... No, you just want to catach any error. The way you treat each of them is not different. And letting those error propagate at this stage will only create a stupid message error of the user such as "Invalid character at position 98"
@andrew_ray
@andrew_ray 3 года назад
Don't store monetary amounts in a float! Rounding errors and money don't mix. You should use a decimal type or integer (with the latter getting divided by 100 for display).
@cerulity32k
@cerulity32k 3 года назад
EXACTLY!
@FasutonemuMyoji
@FasutonemuMyoji 2 года назад
if I get really nervous aout it, I append _in_pennies to the name of a integer DB column or variable just incase someone else refuses to pay attention. 😂
@cerulity32k
@cerulity32k 2 года назад
Doesn’t a decimal, or quadruple, still have rounding errors, but way less significant?
@andrew_ray
@andrew_ray 2 года назад
@@cerulity32k A proper decimal type shouldn't have rounding errors since they arise from the conversion between decimal and binary. When you tell a float (or double or quad) to store 0.2, it stores something close to, but not exactly 0.2. Double and quad types will get you closer to 0.2, but still won't be exact because 0.2 cannot be represented in binary using finite digits. If you multiply that by a large number, or add together a bunch of numbers with similar errors, they compound and you get further and further from the right value and you end up buying 1000 widgets at 20 cents a piece for a total of $199.99. This isn't a problem for measured values because the rounding error is swallowed by limitations on precision, but money comes only in exact multiples of $0.01, so monetary values are exact. When you tell a decimal type to store 0.2, it stores exactly 0.2 (or 2 * 10 ^ -1), so when you buy 1000 widgets at 20 cents each, the total cost is correctly $200.00. Decimal types may still be subject to loss of precision (e.g. because your number has more decimal places than fit in an integer or long), but are immune to rounding errors at the cost of being more computationally expensive to use. If you also need to protect against loss of precision also, there are types like Java's BigDecimal, which allows arbitrary precision (subject to memory limitations) in exchange for being even less efficient.
@seantilson8728
@seantilson8728 2 года назад
@@andrew_ray thanks for this!
@yeagerdd
@yeagerdd 3 года назад
First time here. I gotta say: I'm in love with the way you explained why these changes had to be made. They're not just out of "because I say so" great job mate.
@ArjanCodes
@ArjanCodes 3 года назад
Thank you so much, Diego, glad you enjoyed the video.
@k283
@k283 2 года назад
​@@ArjanCodes while you applied some correct refactoring here and there, you took a simple script and overengineered it by defining garbage enums and completely unnecessary (in this case) custom exceptions. This is also a smelly code, because it actually make the module harder to read. Keep it simple, unless there is a good reason to add your exceptions, or enums, or to use ABCs, etc.
@yt-sh
@yt-sh 3 года назад
0:00 Intro 1:27 Explaining the example 3:12 Code smell #1: imprecise types 5:52 Code smell #2: duplicate code 7:31 Code smell #3: not using available built-in functions 8:53 Code smell #4: vague identifiers 10:05 Code smell #5: using isinstance to separate behavior 13:40 Code smell #6: using boolean flags to make a method do 2 different things 15:58 Code smell #7: catching and then ignoring exceptions 17:29 Code smell #8 (BONUS): not using custom exceptions 21:30 Final thoughts
@ighsight
@ighsight 3 года назад
SMH I finally learned what enum does. This quickly turned from a code fragrance video into a level up the code video for me.
@ArjanCodes
@ArjanCodes 3 года назад
Glad it was helpful!
@CharleswoodSpudzyofficial
@CharleswoodSpudzyofficial 3 года назад
Same. Mind blown
@chri-k
@chri-k 3 года назад
And I learned that Python has enums. Didn’t know that somehow.
@Corporal-Clegg
@Corporal-Clegg 3 года назад
This man explains stuff so calmly and I love his accent.
@ArjanCodes
@ArjanCodes 3 года назад
Thank you - I've been working on my accent a long time - and give it the right amount of "cheese".
@Corporal-Clegg
@Corporal-Clegg 3 года назад
@@ArjanCodes are you Dutch?
@ArjanCodes
@ArjanCodes 3 года назад
Yep! As Dutch as they come.
@berndwarnders
@berndwarnders 3 года назад
@@ArjanCodes Wow, working/living in the Netherlands for 10 years, I would have not guessed that. Learning Python at the moment in my 'free time' and I am amazed by the beauty and potential of it - as demonstrated by you (required to use C# at work ;( ).
@manonthedollar
@manonthedollar 3 года назад
Everyone likes their own smell.
@SenselessUsername
@SenselessUsername 3 года назад
And code smell is just code flavour.
@gondoravalon7540
@gondoravalon7540 2 года назад
George Carlin DID ask "You ever notice how your own farts ... smell okay?" - well, maybe not the kind of smell we're talking about, haha.
@DilettanteProjects
@DilettanteProjects 4 месяца назад
If you let smelly code ripen enough, it eventually becomes _Code Umami™_
@proud22beme
@proud22beme 3 года назад
thanks for doing refactor walk-throughs of implementing concepts, it really does a nice job of reinforcing how it should be after refactor, a lot of tuts just stop at "do this and it will be better" and you going the extra mile is really nice!
@ArjanCodes
@ArjanCodes 3 года назад
Thank you - glad you liked it!
@Sciencedoneright
@Sciencedoneright 3 года назад
I like how you're like Good code => nice smell Bad code => bad smell Lmao
@notinterested8452
@notinterested8452 3 года назад
He's a smelly guy...
@Sciencedoneright
@Sciencedoneright 3 года назад
@@notinterested8452 lol
@quantumjolt8663
@quantumjolt8663 3 года назад
Great video as always! I think another thing to mention is maybe to think through what responsibilites each class has and/or should have, for example moving the pay() method to the Employee class as you did in the video, maybe move that responsibility back to the Company class with maybe an abstract payment handler that handles it, as it seems kind of weird for an Employee to have the responsibility to pay themselves. Nevertheless, thumbs up and great work!
@ArjanCodes
@ArjanCodes 3 года назад
Absolutely and good point. Actually, in my composition vs inheritance video, I’m doing exactly that, also with an employee example: ru-vid.com/video/%D0%B2%D0%B8%D0%B4%D0%B5%D0%BE-0mcP8ZpUR38.html.
@Mateusz143
@Mateusz143 2 года назад
came here to comment exactly the same 😅
@coltonmccormack8978
@coltonmccormack8978 Год назад
I think it's important to know when one should or should not use list comprehensions. Just because something can be done as a nifty one-liner doesn't mean it's always the ideal solution. I see too many mid-level developers try and get fancy cramming logic into them to reduce characters at the expense of readability, which is just another code smell. If the two methods perform the same and one is significantly harder to read by the next developer, I'd argue that's worse. Your example does not fall into this category, but when you see people start chaining ternary logic operations in a comprehension it can get ugly quick. List comprehensions are usually a bit faster than implementing via a for loop though due to the time appending each item rather than initializing the full list with the comprehension results. Great video.
@sebastiansoto3595
@sebastiansoto3595 3 года назад
I found you from reddit post and I'm glad I did, this video was superb. Amazing work!
@namanguntiwar1012
@namanguntiwar1012 3 года назад
Same
@ArjanCodes
@ArjanCodes 3 года назад
Thank you, glad you liked the video!
@ahmadmostafa46
@ahmadmostafa46 3 года назад
Well ... Maybe in a year it won't be that much anymore 🤣
@marwensallem1397
@marwensallem1397 3 года назад
Hello Arjan, nice video as usual 🙌 I have a couple of questions : - This is the second time you create an exception class and you put the message as a parameter to the class. I am wondering, why don't we just create the message using the other parameters instead ? - Second question is more like a request. I am always wondering what is the best to organise the files in python ? is it like java where we create 1 file per class ? It would be useful if you can make a video talking about this subject specifically if there are some principles to follow in general. Thanks again ! see you next Friday ☺️
@ArjanCodes
@ArjanCodes 3 года назад
Defining the message in the exception class itself is definitely a good option as well. I kind of did it automatically at the point where I’m raising the exception, as a habit. For your second question, I would indeed start with creating one file per class/concept. I’ll think about doing a video about code organization. That’s a good topic, thanks for the suggestion.
@nochan99
@nochan99 3 года назад
Residual code smell: all functions rely on side effects (print statements) instead of actually manipulating a state. In other words; there is no separation of data and view.
@filippopisello
@filippopisello 3 года назад
This video is excellent, both in terms of editing and content. Pointing something out as a constructive feedback. I found the thumbnail to be misleading. The display of the uncaught exception error almost made me avoid the video as I thought "oh, once again the same old mistakes that are pointed out in ever video". Instead, your video had no trivial content! You could choose something more original for the preview! Anyway, thanks for the content, great job! Subscribed!
@codeman99-dev
@codeman99-dev 3 года назад
It also contained incorrect information. First, `except Exception` will not catch a SyntaxError. Second, many many companies consider using built-in error types good practice. Why? You're reducing the amount of code you need to maintain. You'll also hear this from python core devs as well. Such as Raymond Hettinger and Jack Diederich.
@bevintx5440
@bevintx5440 Год назад
Thanks for this enjoyable Python code refactoring video. Just FYI, “code smell” is much older than that refactoring book (1999]. The first time that I encountered “code smell” was in 1978 in a code comment, “This code stinks; hold your nose while you read it!” I’m sure it is even older than that.
@thomasburette9129
@thomasburette9129 3 года назад
Did you notice that the refactoring at 5:52 "Code smell #2: duplicate code" is the exact reverse operation of the refactoring at 13:40 "Code smell #6: using boolean flags to make a method do 2 different things" ? In the first case we combine multiple functions into a single one by adding a parameter. In the second case we remove the parameter and split the function in two. In one case it's better to combine to avoid duplication and in another it's better to split to avoid complicated code. Goes to show programming is not about blindly memorizing language features and rules but there is a sense of 'taste' involved.
@ArjanCodes
@ArjanCodes 3 года назад
Well spotted! Note that the underlying principle in both of these is actually the same: single responsibility. In the first case, we improve the definition of the responsibility, leading to less duplication and easier ways to extend (finding employees is now possible for any role). In the second case we split the method in two to separate the two responsibilities that were in the single method.
@cristobaljvp
@cristobaljvp 3 года назад
About #5, I've been using isinstance to raise TypeErrors with "wrong" type of inputs, I mean something like: if not isinstance(variable, list): raise TypeError("You must pass a list") is that not a good practice?
@ArjanCodes
@ArjanCodes 3 года назад
That’s one of the few places where isinstance is useful, in particular if you’re getting the values from a place that you can’t check (such as from a JSON file that a user supplies at runtime). In the cases where you do know where the data is coming from at all times, I think it’s better to put these kinds of checks in unit tests.
@Insanestab
@Insanestab 3 года назад
In those cases where duck typing is insufficient and you actually need check the type, I would recommend using pydantic rather that calling isinstance yourself. It'll look a lot cleaner and you won't need to write as much code.
@SophieJMore
@SophieJMore 3 года назад
I think that's good use of isinstance. But, instead of checking for a list, I'd instead recommend to check for a broader base type (unless you specifically need a list, which happens rarely). So, if you intend on using the variable in a for loop, check for Iterable; if you need to get elements by index or do slices, check for Sequence; if you need to change some values, check for MutableSequence etc. This way you're not preventing the user from passing a type that would actually work correctly with your function if it wasn't blocked by an isinstance check.
@temmayB
@temmayB 3 года назад
16:40 The KeyboardInterrupt exception is not inherited from class Exception, but from BaseException (according to Python 3,7 docs). Such it is not caught here. But it would catch a bdb.BdbQuit, which is triggered when the Python application is to be aborted while being debugged with the Python debugger.
@jackwii1472
@jackwii1472 2 года назад
Deep lore
@Sciencedoneright
@Sciencedoneright 3 года назад
The custom Exceptions bit was actually really helpful for me! Thank you Arjan!
@ArjanCodes
@ArjanCodes 3 года назад
Glad to hear that!
@grrsa
@grrsa 3 года назад
The algorithm brought me here. Love the instruction and examples. Great work!
@ArjanCodes
@ArjanCodes 3 года назад
That’s really kind, thank you - happy that you like the video!
@OlegBedriy
@OlegBedriy 3 года назад
I feel personally roasted, yet this is the code review I needed.
@thatone5350
@thatone5350 3 года назад
I was putting away a large refactoring job for days, but after getting this in my feed I got more confidence in myself seeing these incredibly well explained refactor examples. Managed to finish it in a few hours. Really enjoying your content.
@ArjanCodes
@ArjanCodes 3 года назад
Thanks, glad you like the content!
@JeremiKress
@JeremiKress 3 года назад
I'm still new to Python programming and I just learned a ton in these 20 minutes!
@ArjanCodes
@ArjanCodes 3 года назад
Happy to hear that the video was helpful!
@NotTouchable
@NotTouchable 3 года назад
Would you consider making a tutorial for creating a CI/CD Pipeline in Python? (Ideally with only open-source dependencies) I think it would be really interesting and empowering!
@ArjanCodes
@ArjanCodes 3 года назад
Thank you for the suggestion! I'll make a note of this.
@arkocal1611
@arkocal1611 2 года назад
To extend on isinstance, as some have asked in the comments. One use case that I consider legit is when you are writing generic API functions. Since you cannot overload functions in Python, isinstance might be the cleanest solution. Consider numpy's sin function (I am not sure how it is implemented though). If it takes a float, it returns a float, if it takes a python list, it returns a numpy list. Especially if you are working with basic types, and you cannot make use of inheritance because you do not control the code using your library, this approach might be necessary. Another case is assert statements, or testing, when you want to make sure an object actually is something that you expect it to be. You might not need this kind of tests if your code is thoroughly type annotated though. Finally, if you are writing super generic magic that you need if you are developing a testing library, a framework etc. (which most people don't), or something that processes generic code objects (think pypy) it might be necessary.
@ViralKiller
@ViralKiller 2 года назад
try except pass is what made me the man I am today
@ShanilPanara
@ShanilPanara 3 года назад
Honestly, the best python RU-vid channel out there for non-beginners! Incredible! Thank you for everything, I'm learning so much from you
@amir3515
@amir3515 3 года назад
I feel like if I'm not doing everything in these videos I'm still a beginner 😭
@liesdamnlies3372
@liesdamnlies3372 3 года назад
For #4, I’d argue even better would be hourly_rate_ and then the currency code, e.g. hourly_rate_CAD . That way it’s much more specific, the name is shorter, and it sets-up clarity when you continue development and potentially add other currencies. …should also be using the decimal module for handling money, not floats, echoing the “use builtins” bit from before. Edit: Good job with the thumbnail. I was furious already. XD
@Vedurin
@Vedurin Год назад
Why not go the right way and implement currencies dynamically ? If you already think about having different currencies then they shouldn't be part of any variable naming.
@MariusBelea
@MariusBelea 3 года назад
Thanks for the nice explanations, seeing how these principles get applied onto code examples is a lot more informative and easier to remember than reading about them. Regarding the bonus smell, I'd suggest more emphasis on the usage (you did mention it in the video), when proposing custom exceptions. It's actually good to reuse built-in types as much as possible. Here, custom exceptions were needed, because of the metadata and that there's code somewhere else handling that error.
@EW-mb1ih
@EW-mb1ih 2 года назад
I've read the comments to see if someone wrote something about the custom exceptions part and its utility. In my opinion (and I am not an expert), the ValueError exception associated with its string description is sufficient and making a custom exception seems a bit overkill.
@codeman99-dev
@codeman99-dev 3 года назад
16:01 What? You can't catch a SyntaxError with `except Exception`.
@ArjanCodes
@ArjanCodes 3 года назад
Well... more precisely, a NameError. Try the following thing (literally): try: something except Exception: pass
@sachinjogi1995
@sachinjogi1995 3 года назад
Great examples. Can you please start a series on complete back-end Dev from scratch where you go through all concepts like handling loggers, exceptions, config management following clean architecture
@ArjanCodes
@ArjanCodes 3 года назад
Thank you for the suggestion!
@prudhvirajchowhan
@prudhvirajchowhan 3 года назад
@@ArjanCodes please do this series
@constantchanger
@constantchanger 3 года назад
Font smell: Using a font for coding that doesn't include any separation between adjacent underscores thereby obfuscating how many were typed :-)
@ArjanCodes
@ArjanCodes 3 года назад
Ha, yeah that's true actually. This is the default font used by VS Code, you can probably change it somewhere in the settings.
@constantchanger
@constantchanger 3 года назад
Yeah it must allow that somewhere. Can you imagine an editor that didn't allow changing the font? May as well upgrade to notepad.exe if that's the case.
@SzaboB33
@SzaboB33 3 года назад
This video felt like a senior developer 1 on 1 reviewing my code, it was really helpful, thank you :)
@jemand771
@jemand771 3 года назад
i love how you sped up the parts where you type. usually these videos where people write code in real time get boring quickly because you have to wait for them to type something out. not you however, you're making this really entertaining to watch :D
@ArjanCodes
@ArjanCodes 3 года назад
Sped up? No man, those parts are actually slowed down 😉.
@zapazap
@zapazap 3 года назад
@@ArjanCodes I thought it was simply a good editor autocompletion!
@telnobynoyator_6183
@telnobynoyator_6183 3 года назад
You should rename this video to "7 Code Smells" because those apply to other languages than python !
@ArjanCodes
@ArjanCodes 3 года назад
You're right - they are applicable to other languages as well. I left in the Python keyword on purpose. I learned some people really hate Python so I prefer to make clear that I'm showing these using a Python example, to avoid disappointment.
@telnobynoyator_6183
@telnobynoyator_6183 3 года назад
@@ArjanCodes I guess some people also really love python and can find your video more easily this way too
@KyleDB150
@KyleDB150 9 месяцев назад
13:31 I have one issue with this: The company.employee[0].pay() method call now implies that employee[0] is the one DOING the paying, not getting paid. Say that employee was a manager, is he getting paid, paying someone, paying a bill? I dont think renaming it would help, cause that still implies the employee is taking the action. This seems like a new smell to me, what about you? I guess I'd keep a wrapper with the call: company.pay_employee(company.employee[0]) Which just runs that employee's pay method.
@乾淨核能
@乾淨核能 3 года назад
May I ask what's the purpose of doing this? var_name: data_type = 12 I tried num: float = 2 type(num) is still int, not float why? thank you
@dariuszspiewak5624
@dariuszspiewak5624 2 года назад
Arjan, when you create the custom exception class at ru-vid.com/video/%D0%B2%D0%B8%D0%B4%D0%B5%D0%BE-LrtnLEkOwFE.html, you do something like this: self.message = message super().__init__(message) Is it not enough to just make a call to super()? Why would you need self.message = message on top of that? Can you please explain? Thank you.
@HPD1171
@HPD1171 8 месяцев назад
the take holiday methods were another code stench for me. payout_a_holiday should take an int for requested_days which would be much more useful as right now that method is only useful if an employee wants to request EXACTLY 5 days. also take_a_holiday would just call self.payout_a_holiday(1) with the parameter for 1 day which removes the duplicated logic you have in both of those functions.
@hansdietrich1496
@hansdietrich1496 Год назад
About 16:30: "except Exception" doesn't catch KeyboardInterrupt. KeyboardInterrupt on purpose is only a subclass of BaseException. Of course you're right with all the other criticism about that smell here.
@NickPodratz
@NickPodratz 10 месяцев назад
Your solution to code smell #5 violates the Law of Demeter (LoD), so arguably keeping the Company's pay_employee method to call the Employee's pay method is cleaner.
@tobene
@tobene Год назад
I disagree with #8. There is nothing wrong with using built-in errors as long as the error type Matches the actual issue. For example get_month_name(month :int) should raise a ValueError if you use an invalid Parameter Value (eg. 13 or 0). In your example the problem was that ValueError was used in a case where there was no invalid value
@technodiver9035
@technodiver9035 2 года назад
Hi, Arjan, I really enjoy your channel.. maybe you'll answer a question -> I've been trying to get that color formatting for over a week. Mine formats but not like yours. Mine doesn't change color of constants/globals, when I type cast it stays white. Too much of my coding text is white where I want it color formatted. I watched your video on it but I haven't seen a difference. Some of the extensions you use I can't find. Is it because I use vscodium instead of vscode??
@ravenecho2410
@ravenecho2410 4 месяца назад
Im silly bug i like filter instead of like list comprehension. I currently havent found an apply for like `map, filter, generator` types it would be really cool to have a ""reduce"" which is like hey, for everything make the previous computations concrete do this and discard. I always end up doing like a for loop, but it feels conceptually as the biggest gap in the _functional_ mental context, it causes me to break that mode of thought
@EvenTheDogAgrees
@EvenTheDogAgrees 3 года назад
hourly_rate_dollars: NO! Hourly rates are understood to be in the default currency. If you work with multiple currencies, e.g. if it's a multinational, then you want the currency in a separate attribute. You don't want to write "hourly_rate_dollars" and then expand to the EU and find yourself having to introduce an "hourly_rate_euros" alongside it.
@essamal-mansouri2689
@essamal-mansouri2689 3 года назад
I really like how your keyboard sounds but I tried MX Brown and I absolutely hated the typing experience. Are Gaterons significantly different?
@Ayymoss
@Ayymoss 2 года назад
I stopped watching about 10% through because your microphone picks up all of your mouth noises and I do not want to hear that. Please fix in future videos...
@bungh0LeO
@bungh0LeO 3 года назад
I’ve been working with Python for almost a year now, but many working with pandas, so I don’t have much exposure to the use object oriented programming side of Python. How would this script be used in the real world? Is the script being called from an external program? If so, what kind?
@jeelpatel6506
@jeelpatel6506 2 года назад
Amazing. Need a video for building a better API with Python and maybe FAST API
@SiarheiAkhramenia
@SiarheiAkhramenia Год назад
Talking about responsibilities - when you move the 'pay' method to the Employee class, then you should probably also rename it to smth like 'receive_payment', because this is still the responsibility of the company to PAY, and it's still the responsibility of the employee to GET PAID. When you add the 'pay' method to the Employee class it basically means that the employee is supposed to pay to someone, which isn't the case of your example.
@foreverl01
@foreverl01 Год назад
That last method has something else that smells terrible "Don't forget to check your emails". Reaaaally!?? on holidays?? 😂😂😂
@Upendrahanda
@Upendrahanda Год назад
Hi @ArjanCodes. Loved this video. I am curious though, I've witnessed use of choices parameter in Django model fields in order to choose from limited values. Can the concept of Enum be used in those cases as well? If so, should we drop the use of choices at all?
@drJnmrH
@drJnmrH 3 года назад
There's still a thin smell in the code. The inheritance should be used in another manner here: employee payment type should be used as a strategy design pattern. Thus there will be only one type for the employee class (Employee), which accepts a strategy for payment (a PaymentPolicy derived class instance). Why it's better? Imagine a payment type is changed for the existing employee. What should be done in the code, which has Employee class inheritance? And compare it with the case when you use strategy for the payment policy type. Nice video though, thank you.
@EvenTheDogAgrees
@EvenTheDogAgrees 3 года назад
Hmm, that refactor of the find_* methods is not how I would've done it. I would've kept those, since they're probably used throughout the project (assuming a realistic project, and not just two to three A4's worth of toy code). I would've written the find_employee, and rewritten the other methods to call it with the proper argument and return its result.
@deemon710
@deemon710 2 года назад
@6:50 What key combination did you use to replace all instances of "managers" with "employees" within the function??
@kareltavernier6642
@kareltavernier6642 6 месяцев назад
I develop a geometric library. It has a number of geometric object types, such as Point, Line, circular Arc, etc. 0:02 each defined by its dedicated class. I have a polymorphic distance function that takes two geometric objects as parameters, and returns the distance between them. This distance function contains a lot of isinstance calls to cater for each combination. A lot of isinstances is a code smell. However, I do not see how to avoid these isinstances. I also do not see a problem, the code is quite limpid. (Note that the calculation for each case is typically a single formula, so the function is not overly long.) Is there a better way?
@nicoz6540
@nicoz6540 Год назад
Brilliant, in terms of exceptions, what do you think of the "look before you leap" vs "ask for forgiveness" philosophies?
@polyliker8065
@polyliker8065 2 года назад
Thanks! This was way more useful than the lesson I had on codesmells when studying (they used the same book). These concrete examples and simple explanations of why it's smelly really help with digesting this 'dry' matter. And it isn't just useful with Python but with Java and other OO languages as well.
@ArjanCodes
@ArjanCodes 2 года назад
Thank you, glad it was helpful!
@Septem_150
@Septem_150 3 года назад
How would you avoid isinstance when using strongly typed python code, except by disabling mypy warnings?
@clonador.206
@clonador.206 Год назад
stopped watching when you declared money variables as float. no credibility. biggest code smell was your own. Dont learn from this wannabe youtuber coders guys... search for well known good coders on the internet
@legitjimmyjaylight8409
@legitjimmyjaylight8409 Год назад
The find employees of type functions are almost identical! Violation of DRY.
@soupnoodles
@soupnoodles 2 года назад
16:53 Hey, just wanted to point out that exceptions can't catch SyntaxErrors, those get caught during the parsing phase I think you meant NameError
@Hamsters_Rage
@Hamsters_Rage 3 года назад
are employee classes still a dataclass with pay method implemented? how real pay method are going to be implemented (like subtracting money from company account) if they are located in employee class? can you still use auto() in enums if you are going to save enum value into any kind of persistent storage?
@ErikS-
@ErikS- Год назад
17:07 - Damn... there goes my (incorrectly) paid holiday...
@goldfishgallant1432
@goldfishgallant1432 3 года назад
Not making use of functions that python offers such as list comprehensions is a big smell. Next tip, never fucking use is instance ever or your code is bad.
@MarcoGuardigli00
@MarcoGuardigli00 2 года назад
Very good content. Thank You!
@ArjanCodes
@ArjanCodes 2 года назад
Thanks so much, glad it was helpful!
@BryanChance
@BryanChance 3 года назад
Code may start out clear and clean. Wait 2 or 3 years and surprise!! Sometimes a clear and clean code makes it too difficult to modify. Then again, mabye it wasn't clear and clean.
@AIRLZ6
@AIRLZ6 6 месяцев назад
At the end you say the we could have used a dataclass for the custom error. But I see that using super() on a dataclass is a bit tricky. Is it actually better to use a dataclass, in this situation with inheritance? (sorry if this was covered in a different video on you channel , i'll give it a search)😁 Cheers!
@asdfgh6906
@asdfgh6906 2 года назад
Is it possible to implement roles for employees without creating another class - so that you can assign roles with "...role=manager...". Having to reference another class when assigning roles seems like an overkill
@nickeldan
@nickeldan 2 года назад
I agree that, in this case, the use of isinstance was bad. However, in one of my projects, I have an ABC which is further refined by a subclass (i.e., the subclass is also abstract). The subclass provides functionality not present in the base class. So, I have a function which takes an instance of the base class. It does stuff and then, if the instance is also an instance of the subclass, it invokes that extra functionality as well.
@Soul-Burn
@Soul-Burn Год назад
Having complex logic functions in a dataclass is code smell. Dataclasses are meant to be data. Any functions in dataclasses would usually be helper functions over the data in the object, rather than business logic is this code. It could either be standard class, or move the logic to another class/function to handle vacations.
@alimohamedabdelrehim6582
@alimohamedabdelrehim6582 Год назад
What is the shortcut that you used in 6:51? :) Thank you for the video as well really amazing!
@remustata
@remustata 2 года назад
"I'm looking for troublemakers, have you seen any?"
@JohanDahlin
@JohanDahlin 3 года назад
They're pretty good, but I would not consider the list comprehension a code smell. Particularly nested list comprehensions are hard to understand and extend.
@0x44_
@0x44_ 3 года назад
I love watching this stuff only to not have the power to make certain changes because of Senior Developer decisions.
@apmcd47
@apmcd47 Месяц назад
When is a code smell not a code smell? For instance if you have a block of if ... else-if clauses, should you end with an else clause even if that clause is empty?
@toooldtobejunior
@toooldtobejunior 2 года назад
one code smell was introduced during this session. the 'pay' method does not belong to the 'employee' class. max of what 'employee' can do is to know how much needs to be paid. then, I would call a separate module to actually make a payment.
@Gashdal
@Gashdal 3 года назад
am i the only one who absolutely hates the term 'code smell'? its just gross to me, and i hate it. i cannot stress that enough. i don't ever want to read the words code smell ever again. why is that what we decided on? it could have been anything else, but you just had to associate it with shit that stinks. i do not want to picture that. fuck. end of diary entry also completely unrelated you know this guys european when the employees in his example get 25 days of paid vacation. america is a hellscape
@bonbonpony
@bonbonpony 3 года назад
10:20 So what are the legitimate uses of `isinstance`? 14:14 What's "cohesion"?
@marcosoliveira1538
@marcosoliveira1538 2 года назад
I think I am strange, because I think these videos really entertaining. Great Content!
@emilyscloset2648
@emilyscloset2648 4 месяца назад
Not sure how you avoid if isinstance if you get passed an object you don't have control over
@Meleeman011
@Meleeman011 2 года назад
Imprecise types, lol the code is more readible when checking for specific strings, it takes more time for me to figure out what int maps to what role and is annoying af
@deatho0ne587
@deatho0ne587 Год назад
I understand number three, but I am almost never a fan of things like this. If you have a new developer. Can they look at a function and understand what it does in a couple of minutes? If not is it worth the time to change it? Cause they are going to have to maintain it in the future. Maybe in the future that loop needs to do something different, yes it would most likely mean other code changes. It is more of thinking about the future of the code.
@Gameplayer55055
@Gameplayer55055 3 года назад
19:58 how can i deal with same names? I have code that uses tens of values: self.value=value Can it be prettified?
@THEMithrandir09
@THEMithrandir09 3 года назад
Well Python isn't dart :( You can mitigate this somewhat using dataclasses, or instead go for a kwargs dict and just store that, though that might make catching bad inputs harder and code less readable. Another way could be to split your class and compose it, but that's basically adding boilerplate to hide a class that's probably too big. It's the init function, sometimes it's just a long boring list.
@Gameplayer55055
@Gameplayer55055 3 года назад
@@THEMithrandir09 thanks for your reply. I was worrying about it, personally in some big parameters i use dataclasses
@jammydodger9288
@jammydodger9288 3 года назад
@@Gameplayer55055 I don't know if it's quite what you want but you could use something along the lines of: for k in [*kwargs,]: setattr(self,k,kwargs[k])
@Gameplayer55055
@Gameplayer55055 3 года назад
@@jammydodger9288 thanks
@petersilva037
@petersilva037 3 года назад
@@jammydodger9288 OK... for the very simple case... but more fun might be to use @propery and @x.setter ... which allows a lot clearer range checking and such... www.geeksforgeeks.org/getter-and-setter-in-python/
@egorsencha2428
@egorsencha2428 3 года назад
I absolutely love your videos, randomly found your channel and it is exactly what I was looking for. Exellent content.
@ppybmjc
@ppybmjc 2 года назад
"Maybe in a year, it won't be that much any more..." I hope you went ahead and shorted BTC on that hunch! 😂
@SeamusHarper1234
@SeamusHarper1234 3 года назад
I like this video, because it's a good resource to improve after taking it all the "learn python in 30 minutes" videos =) Instant sub.
@ArjanCodes
@ArjanCodes 3 года назад
Glad it was helpful!
@eidiazcas
@eidiazcas 2 года назад
Inheritance by itself should be a code smell, and classes should be simply structs and interfaces, the only case I see for classes/inheritance is performance (when millions of instances are needed), other than that, closures and simple interfaces should be the way
@shreedaghatpande1878
@shreedaghatpande1878 3 года назад
Great job !! This a incredible reference for interns or freshers. Thanks for all the efforts.
@annakquinn7084
@annakquinn7084 Год назад
Quesrion: Can I hack my life and say “It still works”? 🤔
@danielazulay4936
@danielazulay4936 2 года назад
Pay attentions with list comprehension, your app might grow and more logic will be added, so an if else might be better here
@QuintinMassey
@QuintinMassey 10 месяцев назад
4:35 I get the use of Enums, but what if I was using ConfigParser to read in a string from a file, what’s the best way to take that string and make sure it’s one of those Enum values or Enum names?
@proud22beme
@proud22beme 3 года назад
if you dont mind me asking. could you do a video going over the itertools/functools builtin modules? itertools is a module i always forget what does what in, since its a relatively rare usecase, but when you can use it, it often saves a lot of time. having a overview video would be a huge help with it
@ArjanCodes
@ArjanCodes 3 года назад
Thanks for the suggestion, I’ll put that on the list!
@stefanopalmieri9201
@stefanopalmieri9201 3 года назад
Code smell 1 seems more like a domain modeling issue and not a true code smell.
@russleen403
@russleen403 3 года назад
Well, you're Dutch, so you must know python I guess. Jokes aside, great video, you totally caught me with at least 3 of these!
@ArjanCodes
@ArjanCodes 3 года назад
We teach list comprehensions at primary school here! Just kidding, glad you liked the video ;).
@patrick6116
@patrick6116 2 года назад
I didn’t really understand the abstract method when he explained the pay function. Why does one do that?
@rodrigogutierrezarana1078
@rodrigogutierrezarana1078 2 года назад
just laught like for a minute for smelling a good python code
@alexd7466
@alexd7466 Год назад
the solution for Try..except is incorrect though. It should use 'with contextlib.suppress(AttributeError):'
Далее
Purge These 7 Code Smells From Your Python Code
29:43
Composition Is Better Than Inheritance in Python
23:29
Просмотров 260 тыс.
МОЮ ТАЧКУ РАЗБИЛИ...!
39:06
Просмотров 452 тыс.
Дикий Бармалей разозлил всех!
01:00
Python's 5 Worst Features
19:44
Просмотров 108 тыс.
15 Python Libraries You Should Know About
14:54
Просмотров 391 тыс.
25 nooby Python habits you need to ditch
9:12
Просмотров 1,8 млн
Avoid These BAD Practices in Python OOP
24:42
Просмотров 56 тыс.
Dependency Injection | Prime Reacts
28:34
Просмотров 333 тыс.
5 Useful F-String Tricks In Python
10:02
Просмотров 310 тыс.
МОЮ ТАЧКУ РАЗБИЛИ...!
39:06
Просмотров 452 тыс.