My Favorite Code "Anti-Patterns" (Break These)

We hear a lot about "clean" code principles and anti-patterns to avoid... but sometimes those rules are either misunderstood or just bad coding principles altogether.
Prepping for your frontend interviews? Use code "conner" for a discount on my product FrontendExpert:
www.frontendexpert.io/conner
🎬 TikTok: / connerardman
💼 LinkedIn: / connerardman
💻 Video/Coding Gear (affiliate): www.amazon.com/shop/connerardman
Timestamps
0:00 Principle #1
2:46 Principle #2
4:33 Principle #3
7:11 Principle #4
8:58 Principle #5
10:55 Principle #6
13:07 Principle #7
15:03 Principle #8
Business/brands 👉 [e-mail]

[@dave7244]

The one return statement is the dumbest rule ever. You should try to exit a function as soon as possible. It massively simplifies the logic generally and flattens out if statements making them easy to read. It also ensures all your guard statements are at the top. I really dislike the one return statement rule and there is literally no good reason for it.

[@martintuskevicius8084]

Yeah this is a bad rule. You should generally return as soon as you can. Making the function run longer and having the developer read more lines just to avoid a return statement is silly.

[@ilonachan]

The one argument I can see for it is that if there's some kind of cleanup logic required to always be used at the end, you'd have to repeat that at every return branch, and stuff like that is easy to forget over time. Having a single consistent exit path makes sth like that easier. Depending on the language there are likely strategies to deal with that tho, like putting the cleanup logic in a local function inside your function scope and call that passing through the return value, or something. Still, if you forget to `return cleanup(myValue)` once that can spell trouble after all. ...hmm it'd be kinda cool for this if functions had finally clauses... I'm not sure how try-catch-finally interacts with early returns in JS, for example. If returning from the try block still ensured the finally block is always run before returning, that'd even be the correct idiom for putting cleanup logic! Final edit: OH SHIT THAT ACTUALLY WORKS!!! This is HUGE! Sure wrapping everything in a "try" block seems a bit odd, but oh my god is this just the PERFECT place to put cleanup logic! Even works when the early return is an exception! (tho you might have to tinker a bit to make early returns exempt from the cleanup for some reason)

[@axe8241]

@@ilonachan the same as c# try catch finally, comming from old GOTO cleanup back in the days

[@fullmontis]

​@@axe8241I still feel like this is the one case where goto is the objectively cleaner and more readable answer. Try catch finally always obfuscates flow for me

[@nightfox6738]

It can be good if you're working on safety critical systems that it needs to be easy to define and test all code paths so you're sure some piece of code that hasn't been tested doesn't cause an accident that leads to loss of life. It's a lot easier to verify stuff like that when your functions only have one entry and one exit. For non safety-critical applications though it's really not a good rule though.

[@vytah]

The "single return" rule is a misunderstanding of the original "single exit" rule that is completely irrelevant for modern languages. It means that the function should have one place it returns TO. In modern languages, it's always the place where it was called, but in Fortran you could define a function that could return to a different place. Since it had all the same issues as goto, Dijkstra was strongly against it, similarly how he was in favour of "single entry" - a function should have only one place where it can be entered. More modern languages, like C, took Dijkstra's advice, and disallow functions with multiple entries or multiple exits, so they do not have those issues. Although exceptions sometimes feel like those multiple exits of yore.

[@michaelutech4786]

Same name, different meanings. Single return is in itself also a principle. Eiffel is one of the languages that incorporated that principle by not having a return statement. Meyer wrote a lot about why he believes this is a rewarding topic to think about and I believe this effort justifies more than "he didn't know what he's talking about". Of course, Eiffel is dead. The question remains: Should it be and are we better off because of it. We are looking at JavaScript in this video. We are talking about code quality here. Well crafted code with early exits or guard statements is my preferred way. But it's hard to teach a linter what well crafted code is. It's easy to enforce a single return. Single return is not leading to the most easily understandable code, but it prevents a lot of madness and capricious code. That's a good thing. Just not the best outcome.

[@FinnGamble]

I’ve always felt that throw is mostly just goto in disguise.

[@cQunc]

Python has generators, which can be used like coroutines. The first time you enter one, you start at the beginning, but instead of "return", you end at a "yield". The next time you enter, you resume at the yield. You can even use send() to give additional data to the generator, inserted at the yield expression. So in a way, multiple entrances are still a thing too.

[@Fafr]

So then I'm confused, do people that follow the rule never think of what they follow?I think it would be obvious that something's up with the rule, but there they are still going with it

[@vytah]

@@Fafr People wrote an entire operating system using systems Hungarian notation. Never underestimate cargo cults.

[@RAINE____]

No five minute intro? No sneaky sponsor? Barely even a goodbye? This guy understands getting to the point.

[@igorskyflyer]

I, very proudly, break the 1st principle, every single time. 😌

[@vitcermak7737]

I never even heard about single exit point principle. Since I've learned about guard clause pattern, I've used else if and else maybe once or twice.

[@stsam63]

on top of number 6 mathematically, you can do the multiplication once (a*tax + c*tax + b*tax) == (a + b + c) * tax, which would be a great refactor, always faster to do less

[@FandangoJepZ]

For compiled / jitted languages, that will get optimized fortunately

[@IlariVallivaara]

Also cleaner, I think: return taxRate * items.reduce((total, item) => total + item.price)

[@FandangoJepZ]

@@IlariVallivaara JavaScript user detected!! You can just use .sum() in any sensible language

[@SimonBuchanNz]

​@@FandangoJepZyou might be surprised! Float operations are notoriously hard to optimize, as theoretically equivalent transformations can completely change the behavior in edge cases. Look into "-ffast-math" for the gory details.

[@dash8497]

@@IlariVallivaara soydev detected

[@som.shekhar]

An interesting observation: For the test coverage point (#4), if you look closely, you'd realize that even if you remove the last test that ensures that the white spaces are trimmed, the code coverage would still be 100% and this is because the `input.trim()` line runs for the other tests as well even though it does nothing for them. So, the last test is definitely an important one, it ensures that the trimming is actually happening, but the coverage report doesn't force you to have that test.

[@colin7270]

What if you pass null instead?

[@som.shekhar]

​@@colin7270 The point that I was trying to make and the case that you're referring to, are two different things, let me explain. So, there are these two things: 1. You should ideally be writing tests to cover all the important things that your code does, one of which, in this case is the trimming of the input, and this isn't related to the test coverage percent. You could sometimes cover all the important functionalities w/o reaching the 100% mark and sometimes you might need to go beyond the 100% mark to achieve the same. And this is what I was referring to, we can remove the last test and the coverage would still be 100% but we'd not be testing all the important pieces of our code, which might become problematic as somebody might remove the `trim` piece and the tests would still pass. 2. Now. the second point is you should ideally test your code with different kinds of input, like `null` OR an uppercase `HELP`, and this would ensure two things: 2.a. First, that your function properly tackles unexpected input, like `null`, this could mean silently ignoring the input OR intentionally throwing an error etc. 2.b. Second, that your function doesn't miss out on anything that you should handle but you aren't, things like an uppercase `HELP`. And this is what Conner addressed in the video. Hope that made some sense!!

[@thacf23]

The thing with #6 is that the first example can be understood by people, who don't even know JS. A schoolkid, who knows only Pascal and maths will know what is happening, but then we add reduce and arrow function, which require you to be familiar with them

[@Brad_Script]

single return statements only make sense in languages where you have to manage memory before quitting a function. In javascript, this concept doesn't exist so it's better to have multiple return statements. The rare cases where you have to do some sort of clean up in javascript (example: closing a file), you can put it all in the "finally" block inside a try/catch/finally, ever thing in the finally block will execute before any return statement.

[@ConnerArdman]

That's an interesting point. Admittedly I haven't done any C/C++ or similar development in years, but I could see it being a good practice when you have significant cleanup to do.

[@stefanalecu9532]

​@@ConnerArdman You can definitely have multiple return statements, even within C and C++ (it's only the former where it's a pain in the ass with resource management, C++ has RAII which works pretty much like using in C#, so that's a non-issue.

[@Internetzspacezshipz]

I’m not sure if you can really make a case for that. I mainly say this because 95% of the time when making a function you’ll just use stack memory anyway, then when you do want to make heap objects you should probably wrap them with something like shared_ptr or unique_ptr, or even just have a clear line of ownership by instantiating and deleting only in a single place each (eg in constructor you can use new once, then in destructor use delete). Maybe in real time performance code it might be necessary to avoid using smart pointers and such… but then you’re dealing with a much more restrictive environment where you should probably have an absolute ton of testing to make sure nothing ever goes wrong anyway. Also this is specific to C++ since that’s the language I spend most of my time using 😅

[@Brad_Script]

@@stefanalecu9532 yeah C++ has destructors and smart pointers which can do the clean up for you once you exit a scope, in C everything has to be done manually.

[@MortonMcCastle]

Lately, I've been practicing C by making a small Tic Tac Toe game. I made a TakeInput() function that would take a user input and convert it to a location on the board. If the location is valid, it calls the next function, UpdateBoard(). But if the location is invalid, it calls itself, making it a recursive function. I had a bug. If I was testing invalid locations, when I would finish a game, it would barf out a duplicate of the final output for each invalid input I used. This bug was fixed by adding "return;" after each recursive TakeInput(); call.

[@chefaku]

4:45 the lack of documentation in zig for being in the 0.xx has taught me how useful are comments, and how much you can understand looking at the source code

[@athulgeorge4873]

The single return statement is the stupidest thing ever. One of my uni assignments enforced this rule without ever telling us why it’s there. It made the codebase way less readable and more inefficient.

[@Internetzspacezshipz]

I agree with a lot of things in this video, but there is one that I will not agree with, and that is the commenting one. No matter how simple YOU think your code is, there is always going to be someone else who goes in later and looks through it and might misunderstand it. There’s a very big bias here because the person writing the code knows everything they are thinking during writing it, so it’ll seem “obvious” to them. There have been many instances of someone at my job (even myself!) going into some code and thinking “wow that’s obviously going to lead to a bug if we do that”. Then when it is changed, suddenly there is now a REAL bug that happens all because there was one tiny edgecase or something like that wasn’t accounted for by the person doing the edit afterward. Another thing is making it extremely clear what functions do, what class variables control and where interfaces are implemented and how they are used. I say this because I CANNOT COUNT the number of times I have gone “oh this function doesn’t have a clear description, let me see how it works”, then find it does something entirely different from what I expect. Literally one of the first (major blocking) bugs I found at my current company was caused by exactly this. It literally froze our whole programming team for a week as we all looked at it. Then finally I looked into a function that everyone ASSUMED was doing one thing, but it turns out it was doing something entirely different from what was expected, all because the name of the function was vague and there was no commented description! This is 1000x worse when you don’t have access to the underlying source code as well, since you might not even be able to figure that sort of bug without good comments and source code. Excuse the massive rant, I’ve just seen this go wrong too many times.

[@tmbarral664]

For the tax & discount, you may have said that we do not used const values like 0.1 but we will have 2 constants const TAX = 0.1 and const DISCOUNT = 0.1. And then we use the right const making our code DRY ;)

[@ImperiumLibertas]

This is correct. Named static consts can greatly improve readability. It makes code read like English where you may say price * DISCOUNT * TAX for example. Or even better use a declarative approach instead.

[@atiedebee1020]

I fail to see how named constants are DRY

[@ImperiumLibertas]

@@atiedebee1020 named constants can be declared globally if dry is your concern. My use for named constant is readability. Something that says 300 means nothing but FIVE_MINUTES all of a sudden you understand without additional context. I think what the OP was talking about is instead of repeating the value several places you can use a named constant and have that value in once place. It will become a problem if the value youre relying on doesn't actually represent the named variable youre relying on which may result in needing the value to be something different in one place than another. Try to ensure you're named constants are distinct unique to your domain and specific problem.

[@atiedebee1020]

@ImperiumLibertas I don't particularly care for DRY. The original comment read to me as if named constants are a DRY concept, which isn't the case from what I've seen.

[@ego-lay_atman-bay]

I am against the idea of you should never have long functions, because sometimes long functions are good. I feel like it just makes sense to put all related code inside one function, rather than breaking it down into 50 smaller functions that you will never use elsewhere. That's the thing, if you're not gonna use that bit of code in a different function, don't bother making it it's own function, unless it makes sense to have it as it's own function.

[@michaelutech4786]

I don't like it, but I agree. The problem is what happens if you say that to the wrong person.

[@wardibald]

If you name your smaller functions properly, you'll far more easily understand what your larger function is doing. You'd read some descriptive lines instead of a massive piece of code spaghetti, where you would no longer have memorized the first parts once you wrestle through to the end. Also if your function can be described as 50 smaller functions (I know - exaggerated - but you get the point), it's a golden hammer function that can be split up into components with their own responsibility. Which encourages re-use and may very well cause some of those functions-you-will-never-use-elsewhere to become useful elsewhere.

[@michaelutech4786]

@@wardibald I understood this comment to refer to exceptional functions that may be better writting monolithically, not as a principle for how to organize functions generally. I personally have two and a half approaches to this. I generally design function they way you describe it, as a vocabulary that I then use to describe what my program does. This works well when this vocabulary is closely related to the problem domain, that is when a small function is understandable without or with little context. It works less well, when in order to understand the purpose of such a "vocabulary" function you need to lookup the context to understand what it's doing. That's where big functions sometimes (rarely) tend to be easier to understand because you see the code in the context. You see this in complex computational code that is specific for a problem domain. You also see that when doing infrastructure stuff. The "half" above is that I still dislike monolithic code (huge methods). What I like to do there is to lean on literate programming, where I start out describing what I do in natural language and progressively add code. This often works well because such code is often hard to understand, it's hard to name functions you would want to otherwise extract and such code also tends to change less frequently. With literate programming you then have other means to partition the code (sections, lists, etc.) that allow for folding and otherwise structuring the code to make it easier digestible. My workflow looks like this. I start in scratchpad mode implementing functionality. Then I keep extracting functions (vocabulary). If it's hard to name a function, I get suspicious and continue leaving it inline. If naming keeps being difficult, I start documenting the algorithm in prose. If that does not lead to consistent naming, I keep it in that literate form that ends up generating big functions. Then I add a linter exception and am satisfied. All that's based on my experience that if I can't name it properly, it's either inatelycomplex and needs documentation or I didn't understand it.

[@WaterZer0]

If your giant function needs other functions that are only used it in, make a local function! I have no idea about JS though so maybe you can't do that.

[@williamforsyth6667]

The problem is not with the specific principles, but with any absolute principles. Projects, languages, environments are different. Just an example: single return is almost mandatory in environments logging is the only option to observe execution. It is also useful if cleaning up allocated resources has to be done manually.

[@michaelutech4786]

One of the few informed and competent comments. Great! Single return also helps defend against bad developers who produce capricious code, to some extend. The price is overall less readable code. It's easy to lint.

[@DWebOscar]

14:06 Principle 7: Absurdly small functions actually help troubleshooting in compiled languages. Speaking specifically of c# and .net: where a call stack in an application compiled in Debug mode will give you (mostly accurate) line numbers of the error. But a call stack in an application compiled in Release mode will only show the most recently called member, whether that is a method or a constructor or whatever. So if your functions are small, you can easily pinpoint where a problem happens without the aid of line numbers. If you’re deploying the application properly, you’ll only ever get the help of line numbers locally. So when everything hits the fan, it’s nice to be able to see the clues you’ve left yourself for exactly that purpose.

[@Extremaduur]

Linting, branches not passing on Jenkins if the test coverage is not 100%, branches not passing if your static code analyzer complains. Sure, there can be legitimate code issues, but coding can also become a straitjacket. It can be very frustrating if you have your code working within a couple of hours, but it takes a day or (gasp) a few days to get all your RxJs tests working with 100% coverage. And in the mean time your manager is on your back.

[@michaelutech4786]

The question here however is how important correctness is. The next question is what the manager is managing. Are they managing the product or just their budget and timeline? What is your personal goal? Delivering what you are asked to, what you want to produce or just get a paycheck? If you don't need a branch, can you just delete it? What would happen? To the test: nothing. What are you testing then? If you don't feel comfortable deleting each line of code that is not tested, you should not feel comfortable not testing that line. Alternatively, if you think this is too extreme, why are you wasting time testing at all? All that boils down to "what is good enough" and who knows.

[@ego-lay_atman-bay]

My problem with tests, is I tend to write programs or libraries that deal with data on the disk, rather than running with no input. How do I test something that requires external files? Do I include the files in the tests? What happens if I'm writing a program to read files that I can't legally distribute (such as a library to read and modify files from a video game)?

[@Tekay37]

You would create test doubles for the tests.

[@taidokax7562]

"Single return" - rule highly depends on the coding style, programming language, and codebase you are working with. For example, in Scala return keyword is optional and the last statement is always returned. The general rule in Scala is to avoid the return keyword. Due to the features of Scala, the avoidance is easy and there are no drawbacks. In fact, using early return in Scala probably means that your code is more messier than if you use Scala right and avoid return.

[@Barteks2x]

For comments my general rule is "explain WHY, not WHAT" - everyone who knows the language can see what the code does, as long as it's not too badly written. But it's often not at all obvious what the intention is or why it's done. And with comments/documentation on methods/functions - typically as someone using some libraries, it's annoyingly common to see the documentation restate what the name says, and then I have some sort of special case and I have no idea what the intended behavior is. In case of the "calculating area of a rectangle" - it would be a good idea to actually document that the function is supposed to give an error if the area is zero, as it's not at all obvious from the name alone. A good general example of a type of comment I write - when fixing a bug and it's not immediately obvious from reading the code that it should be that way, I add a comment explaining why this specific thing that fixes a bug is done.

[@FurryDanOriginal]

Just discovered you recently and I love your channel! You're making great takes and there's few content creators I passionately agree with so often.

[@brianviktor8212]

Principle 1 sounds silly, sacrificing readability just to have a single return statement. Principle 2 well, just use calculateTax and calculateDiscount because logically they are different. In the future either one can change independently. Principle 3 "return width * length;" in the body. Write 1 line of comment (not n + 2) above the method to explain it if needed. If it's trivial or the function name explains it, no explanation. Principle 5 is a distinction without difference. But in general, doing algorithms better than worse is preferable, and in my case, if the code is ran a lot, I always strive to do that, especially if it means decreasing computation by 50% or by orders of magnitude. With experience this occurs automatically. Principle 6: It depends. The example is too trivial to matter, but as complexity increases, splitting it up can be a good practice. I prefer to keep code coherent as long as it logically makes sense. Principle 7: Same as 6. Principle 8: Splitting up classes makes it more tedious to use. As of a logger you want one single class that does everything. If you believe you need a logger that does something else than what loggers normally do, create a new class specifically for that. Generally I prefer to keep logic coherent and avoid splitting up classes into these weird abstract "Creator", "Formatter", etc patterns. That helps nobody. If there is a need, do it, but not by default.

[@OmegaMusicYT]

In principle #1 you can just add "& foundUser is null" (or whatever the syntax for JS is) to the loop condition. That way it will stop iterating once the value is found without having to use a break statement. A uni professor I had use to be against break statements too, so I had to do it that way

[@RexiRexx87]

Hey guys. I’m going to the army pretty soon, in about 100 days. By the time I get out I’m 5 years I want to get a bachelors in computer programming and/or cyber security. But before I ship, I want to familiarize myself with the topics and start pouring somewhat of a foundation. Any books, topics, sites, programs, general information, etc I should know to get started? Anything helps, thank you

[@PythonPlusPlus]

I don’t think any real programmer says to have only 1 return statement. That’s the dumbest idea ever. There are no benefits that come with having 1 return statement. For principle #6 you haven’t really done a refactor at all in the last bit. You just renamed the reduce function.

[@roccociccone597]

Yeah it leads to very deep nesting of code which I hate.

[@LikeFunnyMovies]

Great explanations, agree with you on almost all points!

[@michaelutech4786]

While listening to this video, I think that one pattern is never mentioned: Think. We keep looking for simple principles to guide our reactions to complex problems. DRY is a good example. If the requirement is "do X" and X depends on multiple pieces of code doing X independently, this is nearly always bad. The principle behind DRY is that there need to be a clear tracibility between requirement and implementation, so that a change in requirements can lead to a controlled change in the implementation and that it's clear that a change of an implementation may have an effect on a requirement and which one. DRY is the reaction of stupid copy & paste jobs. DRY IS BAD is a reaction to overgeneralizations. The real principle is "DO IT WELL". That principle is so general that it doesn't really help, because it's not a simple rule. But it's good because it emphathizes that if "IT" is complex, you need to think about it. Why do people think that just because you try to solve a problem with a code editor, it's easy? If a problem is simple enough that a rule like DRY or DO TESTS is a solution, there was no problem in the first place. You cannot make complex problems simple by applying simple rules. We know that from math. We can prove that. It's true in politics, it's true in software engineering. It's not breaking Anti-Patterns that will solve the issue, just as much as following Patterns is no solution. You actually have to learn to think it through, make mistakes, suffer the consequences, recognize mistakes and do better. Our mistakes have patterns. Sometimes avoiding them helps, some problems are best solved with methods that fail for other problems. If your mistakes can be corrected by telling you "DRY" or "RY", then you work on problems the next AI can probably solve better than you.

[@Californ1a]

I'd make an exception to #3 for jsdoc comments, so even outside typescript you can see the function info on hover without needing to manually look at the actual function definition and jump between files.

[@philadams9254]

3:26 - you missed a subtle but important thing here: the first 2 functions are easier to read and understand at the location where they are called because their names describe the behavior better. The more generic function makes things more... generic so more cryptic to understand

[@joshadams8761]

One of my commonest suggestions during PR reviews is to remove comments not adding value.

[@daaa57150]

Single return point is stupid. Just return early, get rid of the edge cases and then do the main stuff which won't be nested inside 2 or 3 if statements. A lot of what you explain after is the KISS principle, which I always praise over anything else. Yeah patterns are cool and useful, all the principles have benefits but the best thing to do is to Keep It Simple Stupid.

[@tmbarral664]

about the "only one return", you said the key word here : readability. In the example you showed, the function is pretty simple and short. It is perfectly readable with the 2 returns. We saw them pretty quick. The "rule" about "only one return" should said only one if you have a long complicated function where it becomes difficult to see at first glance all the returns. As always in programming, there is no such thing as imperative rules. We have to adapt to every situations. Not to mention the people you're working with. They may prefer this or that. And we have to listen to this. btw, just discovered your channel. Nice ;)

[@NebihTV]

Yeah man I have the same problem sometimes. People tend to add sooooo many comments for obvious stuff, its so annoying

[@Veretax]

See I have come around on the return one. I used to think return should be at the end, but then you realize that early return for certain conditions actually is a performance boot. Ever wonder why a program is slower when it throws exceptions? (Late returns folks)

[@SimonBuchanNz]

That's... not why exceptions are slower. Exceptions are the earliest of early returns, they don't even run the return code. Exceptions are slow for a few reasons, but they mostly boil down to it's not meant to be run all the time so slow things like capturing stack traces and runtime type checking of the error type is done to make it easier to debug. Languages that use exception like things as part of the normal program behavior like Python have perfectly performant (or at least no worse!) exceptions.

[@Veretax]

@@SimonBuchanNz it matters if the exception handling is all the way at the end you know just after or just before where the actual return would be but you're right if exceptions are written correctly they should return as soon as the exception happens but I have found that bugs that cause exceptions also tend to cause things to run slower

[@SimonBuchanNz]

@@Veretax are you perhaps referring to error case handling, not language exceptions?

[@WaterZer0]

Sorry if I'm misunderstanding here since I don't currently do any JS, but in #6 those functions were only a few lines. I don't think it makes any sense to be calling other functions with something that's fewer than 5 lines unless you're passing a function up from a an object contained in this object. Having a bunch of single line functions you call only for this function is unnecessary as you describe. I hate to think what someone is afraid of a 5 line function would do in a 500 or 1000 line function; I don't think it would be comprehensible.

[@LearningTwentyFive]

Agree with all these points, but i do think there's a certain nuance you gain by coding for a while and trying to understand these rules and then learning enough about when and how to break them

[@niks660097]

Return statements are the only way to handle edge cases and invalid inputs, so one return statement rule won't work in most cases.

[@Jaryt]

I feel like rather than complex logic, comments are more useful when something isn't obvious (as you said). Basically, if someone else is reading your code, ask yourself if they could understand _why_ you made the decision for the logic. If it's not immediately obvious, or should be common knowledge across the team how your product works, then leave a comment about that choice. Your future self will likely thank you.

[@joelv4495]

Exactly. The variable name should perfectly describe the "what?". If I cannot trivially infer the "why?" when scanning, that's when a comment is useful.

[@MeriaDuck]

Regarding principle 5, I think you are right. The spread operator version is more clear and specifies what you _want_ and not how to do that. And if the spread operator ever gets optimized by the developers of node, you benefit.

[@JohnP55]

Number 1, the whole point is to use break, no one preaching having only 1 return statement prefers iterating over every loop It’s still an anti-pattern but it’s good to get it right.

[@kamelrnb]

Does anyone know what font is that?

[@TheTigerus]

4:33 Maybe calculate10Percent function solves DRY, but it violates SRP and open-close rules.

[@couch9416]

Not sure what SRP and open-close rules say, but with this you would just have the problem that as soon as some value changes you need to refactor all the places where the function is called

[@SimonBuchanNz]

I've yet to hear the same definition twice for SRP and Open Closed, but they are roughly: - "Single Responsibility Principle", something like "should only have a single reason to change" or "does one thing" - "Open for extension, closed for modification", extremely confusingly stated and I've heard various wild interpretations, but basically seems to be some version of "preserve compatability" - eg add new functionality rather than modifying existing. Not sure how it applies here.

[@PieJee1]

I am not even sure the spread operator solution with Math.max is slower as Math.max is being executed in machine code and the other one is being interpreted. In a compiled language the more concise one is faster, but i doubt it does in javascript.

[@69k_gold]

The best code ever written is one with the most lax implementation of clean code principles. These principles should be applied lazily, that is only when the alternative is objectively worse or causes a degraded performance.

[@johngibson4874]

I think comments at the boundaries (function definitions, class definitions make sense) especially when dealing with code that is non trivial. Even your example with calculating area could have a comment. What happens if width and height are less than 0? What are the units? Can you multiply values that aren't the same unit (for example meters and centimeters?)

[@tmbarral664]

For the 6 and 7, I can hear my colleagues shouting : "YAGNI" ;)

[@joelv4495]

@5:15 there IS a way to declare the return type in JavaScript, using the @returns annotation.

[@ConnerArdman]

This isn’t really a feature of JavaScript, it’s just a comment. Sure you can put whatever you want in comments, but nothing enforces it or ensures it stays up to date. There’s some JSDoc plugins that kind of do that, but at that point you’re basically using TypeScript (or have at least departed from vanilla JS by assigning meaning to things outside the language spec).

[@anonymous49125]

principle #3 - comments (outside of javadoc for publishing apis) really have only one place in development: If you wrote something and half way through you hit a huge roadblock and have to start over with that issue in mind, so you write something that seems counter intuitive on that second pass (in hopes of bypassing that issue up the road)... THAT is where comments are useful. Explain the naïve solution, explain the current implementation. The hopes are to save people in the future from making the same mistake again, finding the same solution, then reverting it back to how you left it. Ideally this can be done at the class or namespace level (depending on your language of choice). In short: comments are not for saying how something works, its for saying how something can't work.

[@odouglascardoso]

The clean code is nice, but it is like being vegan. If you are, you are, just don't ask me to stop eating my meat. In 16 years I never asked to some entry or medium level software developer to change their code just because I don't like it. Of course, some discussion on how to make different for performance gain, or something like that. Also, to avoid build new standards and avoid doing things already done. But rebuild the same code differently just because a book says, fu

[@nielsdegroot9138]

I know it's an example, but: @3:20 price * 0.1 is a 90% discount! If you want a 10% discount you multiply by 0.9 (you pay 90/100 of the original price.)

[@ConnerArdman]

The functions represent the amount added/subtracted for the tax or discount, not the final amount. So price * 0.1 represents how much should be subtracted to account for the discount.

[@MasterHigure]

Making sure tests cover every single line of code might not cover everything tests should cover, but turn it around: Would you ever consider a suite of tests complete if there are provably lines that never get run during tests? The answer is no, or very close to it. So covering every line of code is *part of* what tests should do.

[@fswerneck]

Good video! While I agree with you on most points, I have to share my 2 cents on what I think of the very last one, the single responsibility principle. Disclaimer: in your example, I can see why it would be weird to separate responsibilities since the Logger class is, indeed, simple. And in real life cases where an object can be that simple, I see no problems. But then again, it's real life we're talking about; features get added to software, and code gets larger and more complex. Separating such responsibilities not only modularizes better, but also enables us to take advantage of patterns such as Dependency Injection and Composition, which in turn gives us more flexibility. By coding the Logger class like that, the only way to actually have different ways of creating and/or formatting messages is through inheritance, which cannot be done in runtime. (And when it's possible, it's often MacGyverish and hackish.) So you have to actually open up your source code file, create a new Logger subclass, and only then define how this specific class will handle these operations. But if you go the composition path, then you can have different types taking care of different things, and even switching back and forth becomes easy and simple during runtime. Say we have a Protocol (or Interface, or Trait, or Type Class; whatever sails your boat): // pseudo-code protocol LogFormatter { format(LogEntry) -> String } // where LogEntry contains metadata about the entry, such as log level, timestamp, etc // and then class Logger { formatter: LogFormatter // snip } With this, we can easily switch among formatters even in runtime, because the formatter is just another object that the logger uses. We can have e.g. Logger.setFormatter(LogFormatter), and suddenly there's no need to even bother to think about inheritance. We don't need to create a new Logger subclass, we don't need to recompile the code to switch formatters, and we have specific formatting logic that can itself become rather complex, separated into its own type. Bonus: we don't need a full, verbose protocol (/interface/...) if it's a one-method one: class Logger { formatter: (LogEntry) -> String } And we can always provide default implementations so that users of our libraries don't have to do everything from scratch: // again pseudo-code fn default_formatter(e: LogEntry) -> String { "${e.timestamp} [${e.logger_name}/${e.log_level}]: ${e.message}" } logger = Logger(default_formatter) logger.log("my log message")

[@penguindrummaster]

Not a bad video, in that dogma is terrible and you should understand the reasoning for the principle before trying to apply them. Where I disagree with the video is by the use of overly simple examples since a lot of these principles don't apply to mundane operations like simple string concatenation/interpolation or how to sum a series of numbers. For instance, the DRY principle really shines in my area of work, automated testing. All too often I see people copy an existing function, change one or two internal state values and then apply it to a new test. Instead, by hoisting those stateful values to arguments, you can drastically reduce your code footprint while adding only minimal complexity overall. The guiding rule I give to the people I support is "If you find yourself copying an existing function, that should be your red flag that the function should probably be extended to support your use case".

[@drcl7429]

principle #1 Guard clauses should always be used and make code so much easier to read by virtue of causing less indents. I'm sure at some point in the past there was a good reason for single return but there certainly isn't in any language I know. As far as I know clean code doesn't advocate for single return, as it is an artefact of structured programming, and was/is certainly a pattern to follow if you are allocating memory areas manually. #2 violates SRP and magic numbers should be constants. #4 test coverage should be measured in a different way to take account of multiple test cases over the same code. So coverage should be like 1000% or something. Your example isn't a problem with test coverage, it's a logical error. #5 I am a demon for premature optimisation. I know it's not needed but I still do it because I learned C first... I try to make my code readable though by making lots of extra temp variables and hoping the compiler will optimise them back out. #6 and #7 These work only if you use very descriptive function/method names, it could be a YAGNI issue depending on what you are doing. #8 You're just wrong on this. It has been pondered on by people with much more experience than you. It is about reasons to change and who might need it to change. If you think you will get a request for change from 2 different places in an organisation then the object is needs to be refactored to multiple objects or methods moved to other more appropriate objects. The responsibility is not about the object's responsibility, its about responsibilities in the customer's real organisation.

[@ConnerArdman]

I usually don't respond to comments like this, but I appreciate you being polite and taking the time to write all this out... so here it goes lol I think we agree on 95% of this. I'm not sure if you're misinterpreting the premise of the video (admittedly the idea of showing code principles that I disagree with is a bit of a confusing way to cover the topic, as maybe it wasn't clear enough what were my opinions and what were the common "principles" that I have heard and disagree with), or if this list was intended to include things that agree with the points in the video. Either way, here's my thoughts on your thoughts: #1: I'd be hesitant to say "always", but mostly agreed. This was essentially the point I make in the video (in fact, this one came as a result of another video I did on guard clauses where a ton of comments were arguing against them saying they break this "principle of one return"). #2: Agreed on magic numbers - this is also something I have talked about _a lot_ in other videos. In hindsight, I probably should have used constants, but for examples I try to usually minimize how much code is on screen when it makes no difference to the point being made. As for the SRP, not sure what you think violates that, unless you are referring to the calculate10Percent function that I argued against. #4: Yes, this is essentially the point I make. I'm not sure what you mean by it being a "logical error". The point is that if you simply aim for 100% test coverage, you can't assume you will catch 100% of errors or that you even have good tests at all. #5: There's no reason for premature optimization, it is as the name suggests premature. There's a time and a place for micro-optimizations (particularly if you have actual measurable proof it is improving the product), but most of the time it's just a recipe for hard to maintain code. #6 and #7: I'm not really sure what you're referring to here, but I'm always team good descriptive function names so probably agreed here haha. #8: I mostly agree with what you're saying here, I think you're misunderstanding my point in the video a bit (or more likely, I didn't do a great job of explaining it). My point was to push back at the idea that the single responsibility principle means "a class should do one thing" or "a class should only have one reason to change". These are both common ways to describe the principle, but they are not great ways to describe the actual intent of the principle. Like you alluded to, the SRP is about people, not code. If multiple organizations within a company have to change the same code, there's probably something wrong there. The originator of the principle, Robert Martin, actually confirmed exactly this and discussed how the principle is misunderstood on his blog back in 2014. In hindsight, I probably should have mentioned this explicitly in the video. What I do say in the video, is that a class should a single primary business purpose. While this might sound different, I'd argue this leads to essentially the same point, but is just a much simpler version of the principle to grasp (especially for beginners who have never worked in a codebase with multiple contributing orgs across a large company).

[@dave7244]

@@ConnerArdman "premature" is subjective e.g. when dealing with the DOM in JS, I always think about how many times I am accessing the DOM and if I really need to re-select an element. Technically it is "premature" but it stops dumb code and forces you to think about what you are doing.

[@eweer5398]

​@@ConnerArdman #1: "Single Return Principle" comes from Fortran/C, where codebases some codebases heavily rely on goto for resource clean-up. This principle doesn't refer to guard clauses, as they are done before allocating any memory. (In the case it were, it would mean to do a goto _fn_cleanup; instead of returning directly). The point you make on the video is also the thoughts people had back then, but sadly it has been misrepresented in current literature/discussions. Extract from the book Code Complete, written in 1993: 17.1 Multiple Returns from a Routine - Use a return when it enhances readability In certain routines, once you know the answer, you want to return it to the calling routine immediately. If the routine is defined in such a way that it doesn’t require any further clean-up once it detects an error, not returning immediately means that you have to write more code. - Use guard clauses (early returns or exits) to simplify complex error processing Code that has to check for numerous error conditions before performing its nominal actions can result in deeply indented code and can obscure the nominal case. - Minimize the number of returns in each routine It’s harder to understand a routine when, reading it at the bottom, you’re unaware of the possibility that it returned somewhere above. For that reason, use returns judiciously-only when they improve readability. #2: I agree with minimizing code on screen, and this is minimal, but as a teacher, I'm always advocating for visually representing what you are saying, as it helps relay your point better. I'd have also included: function calculateTotal(price) { return price + calculateDiscount(price) + calculateTax(price); } and show that the refractor makes no sense at all: function calculateTotal(price) { return price + calculate10Percent(price) + calculate10Percent(price); // What are we calculating? Why is it 10%? What if it changes in the future? ... }

[@ConnerArdman]

The point of this video isn’t to say all of the original principles are terrible. As I said in the video, many of them are good if followed well, but have just been horribly misinterpreted. To some extent, I would put the single return in that bucket, as _a lot_ of people follow it blindly including things like guard clauses as automatically bad. But also, with most modern languages where cleanup is extremely rare, I just don’t think it’s a particularly useful principle. And appreciate the feedback on the teaching style. If this were a classroom, that’s probably what I would have done. Unfortunately though, with KZbin lengthening the examples ends up hurting the performance of the videos. It’s an interesting balance to find of getting the point across without boring anyone lol. I probably should have spent more time on that one though 👍

[@julianh5167]

In the comments example, the thing that really needs a comment is the requirements on the arguments. If it's OK to pass in zero as width and/or height, then that console.log statement should be removed. You might also ask yourself whether passing in -2 and -3 should return 6 or throw an exception.

[@nalatreb1]

For the principle #8. If you have this simple formatter you shouldn't break it into small classes. But if you have more complex logic for the formatting, I think it should break into multiple classes. Or maybe if you need to handle multiple formatting. Anyway, great video! 😎

[@kneesnap1041]

I'm a bit bothered by the idea that comments should be avoided generally. I really think adding short statements which signal intent is really helpful to reading code quickly. What we don't need is comments which formally detail the code, but instead comments which document the author's thought process writing the code. Having a comment which says "Load the entities from the reader" followed by one later which says "Setup the data structures for efficiency entity lookup" goes a long way for easily reading and navigating code. It'll depend on the project of course but I think it's odd this advice is so widespread

[@ErazerPT]

Good overall, but some bad takes over bad examples, because they are too simple to showcase the point. 1: While i don't consider guard clauses part of the "one return point", you can not have them and use !=null and you only go into the loop IF it's not null. Done, one return point. Also, blame JS for lack of goto, THIS is where unconditional jumps are of use, so that you can have guard clauses AND single return point. 2: This is dumb, and you said why yourself. Just add a percentage parameter if what your function does is calculate a percentage. Why even have a function to do that, it's supposed to be inline. There are examples of why DRY is (sometimes) stupid, that isn't one. 3: Yes. Comment that which is not obvious as you said. And things that might seem to do x but just doesn't and it's not obvious why. 4: Yes. Test what you can (expect) to go wrong a priori. Then test anything that shows up later to prevent regressions once it's fixed. Tests first and foremost tell you that something isn't broken, not that something works properly. 5: Bad example. Math.Max(x) is actually harder to read for anyone not familiar with the method. The loop is basic, anyone failing to instantly read that isn't qualified to code at any serious level. And now, add to it that the "initialization cost" might be high. Anyone not grasping that, come to C# land and watch how LINQ eats away at performance when used over small'ish collections. 6: Yes. You lost "locality" of information the more you moved deeper. Also, you're introducing ever increasing "call cost" as you create a deeper call stack. 7: Yes. Capitalize should be a separate method, but the rest is for sure what happens when people take things literally instead of thinking "does this make sense at all?". 8: They are (sort of) right. And this one hits close to home because I've spent untold time working on "better logging". Your example is just too simple to show it. Logger should care about one thing only, logging. If the methods are private, you really don't needed them and if they are public you can't change them willy nilly or you might break something upstream that expects something from the "old one" if they return anything. If you want more flexibility, just take functions as parameters. Say Log(level,message,createfunc,prefixfunc,sufixfunc). All can be null if you don't want/need any. Now your Logger is only responsible for pushing it to storage, which itself should be an injected dependency so that Logger doesn't care where it's logging to.

[@lucaxtshotting2378]

I am yet to see a video about "amount of helper functions" that really adresses the topic: are you overriding these functions in a different module? if the answer is no then there's no reason to overdo. There's no discussion. A single file with a single function will always be as good or better as one-time-used abstractions that will stay used once.

[@joelv4495]

One thing that I find useful is to use true function declaration, then you can put the main idea at the top of the file, calling the "helpers" before they are defined (works because of hoisting).

[@tomatoslav]

or just throw an exception ... i think its great for branching and more versatile than returning null

[@CottidaeSEA]

Single return is idiocy. Yes, don't overdo things and try to keep returns close to each other for readability, but single return being forcibly added is just bad. I've even seen people go so far as to advocate for exceptions instead, which fair enough, but throwing is just a different type of return and now you're stuck with try-catch everywhere instead.

[@nathanfranck5822]

"single return" --- In Zig they made almost everything an expression, so it technically is possible to have a for-else-expression inside of an if-else-expression, all returned as a value... So it would satisfy the rule, but it also looks a bit ... Evil

[@leonidas14775]

Comments... the bulk of mine are disabled print statements :P

[@sascha-oliverprolic923]

Having a single return is actually a good thing as it enforces you to rethink the problem. The code example shown in this video to "disprove" this, can easily be refactored into a single line of code with a single return. Try some more functional programming and things start to make more sense to you.

[@psychic8872]

If the language does not support the functional syntax you have in mind, it will not work as well as you expect

[@FM1234]

Step 6 is only harder to understand if you're not familiar with it. Obviously if you have never worked with these things, you won't understand them. By that logic, you should not use some things, just because they are harder to understand

[@marcelo-ramos]

The exposition would have been better if the examples weren't that simple. Simple examples could be used to argue any side.

[@nicolasdenis7094]

Number 2 is stupid, you make one function (e.g. priceReduction()) and pass the price and the deduction percentage as variables.

[@anonymous49125]

principle #7 is misleading... (this is language dependent, I know people are class resistant in JS and some languages don't have classes, so ymmv): If you have access to user, and user.firstName in your current scope, you should NOT create a property (or whatever abomination this getFirstName function is) for that field (the variable in that 'user' object). It not only is redundant but it's also redundant. So that's a bad idea. That said, if you're doing stuff IN CLASS, then you absolutely should think about how your code will be used OUTSIDE of that class and ideally by other developers. For instance, that user object should have a 'GetUserFullName()' method on it that is public, and leave all the implementation internally for what that does... capitalization, smushing first and last names together - and everything else that needs to go into that getuserfullname method. That all could be done inside of getuserfullname (which is what I think is being argued for in the video), but you really should only do that up until a point in which it is hard to read then it should be distributed to private methods so that it can be easily followed and understood at a glance what is happening. When that switch happens really depends on preference and how readable you think the original method is. If it's just as simple as grabbing the first and last name, capitalizing them and returning them with a space between them, then that can be a single method without issue.... past that you really should think about splitting up things so each action is in its own method, and especially if that type of method will be used in other places in the class. tl;dr - split things into private methods when the host method is too complex. This isn't a "always do everything in one method" advice, nor is it "always do everything in as many methods as possible" advice --- it's a nuanced: You should do everything in one method until it's too confusing to look at, then start to make helper methods and hide them from other developers if possible depending on language.

[@FM1234]

14:06 javascript turning into java By the way, that example is really flawed. In this specific example, the names of the functions are really obvious so you know what they do. Well, if the functions were more complex, then maybe you would not understand right away, but they cannot all be in the same function

[@IvanToshkov]

The problem with this video, and frankly probably with any video that would try to tackle this, is that it has to find good examples. Too complex and the video becomes long and hard to follow. Too simple and the whole thing looks like straw-manning the principles. It's a tricky balance and I feel like this video is leaning a bit too much on choosing simple examples. Most of these principles are still helpful, especially for less experienced developers. They can give you hints at what parts of your code might be improved. But where you put the line is quite subjective and a matter of style. It also depends highly on the project and on the part of the project you're developing. The "single exit" rule is pretty much outdated, unless you program in a language like C, where you need to manually clean-up (e.g. to release dynamically allocated memory, close files, etc.). And I haven't programmed in C in many years, so it might already have better ways to deal with this. The "optimization" principle is actually the inverse. The principle is to not do premature optimization.

[@haydend3469]

#7 was horrible and is a gross misuse of the sufficient functions that are part of standard packages

[@dinoscheidt]

😮‍💨 Anybody who has ever worked with other peoples code knows that having early multiple returns in a function, aka using return as a switch statement, makes it incredible hard to understand. Code that is easy to write and hard to maintain and - most importantly - extend. Either extract that loop into its own function or leave it as is. The convention is: (1) a function should have one exit point (2) return should be on the first level of the function (3) early returns next to local variable declarations may be allowed as type and logic guards (you see that in react a lot).

[@Voidstroyer]

I kinda find your DRY example dumb. You could just write 1 function that takes in 2 arguments (price and percentage) and thereby still satisfy discount and tax having potentially different values, and you satisfy DRY. It also makes it not necessary to refactor the code later on unless the entire logic changes. I think that DRY is generally good because it makes to think in ways on how to write your code more efficiently.

[@Voidstroyer]

Okay I just finished the whole video and I would say that most examples are quite dumb. But I do get why you use them, it's simply to prove your point. But these examples don't dispute these principles. They just demonstrate that these principles shouldn't be used 100% of the time. But with different examples you can also prove the opposite of when these principles SHOULD be used.

[@EvolMate]

This video is OCD-inducing :D with first example one can wrap the for in if(user!=null), and it makes code ever less readable! (1 more nesting, negative clause). That's why it's so great in js we can just return users.find(...), and noone cares if its more or less effecient.

[@vincentnthomas1]

that cam looks really ai generated

[@hereallyfast]

Lolol don't trust try go

[@SakariyeMaxamuud-]

Second comment 😊😊

[@ajaydeshmukh69]

First Comment Pin😓

[@thatguy2567]

For 6, the refactor I would do is break up the addTax and summation operations. claculateTotal = (orderItems, taxRate) => sum(orderItems.map(item => taxRate(item.price, taxRate))) That's what really matters there