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.
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.
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)
@@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
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.
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.
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.
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.
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
@@vitcermak7737 It's really only useful if you're using functions that are formatting or creating data with a specific format, so you know you're not missing something. Otherwise just hit-it-and-quit-it.
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
@@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.
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 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!!
The way I would describe it is that you should test all intentional behaviors explicitly (with individual testcases), ie document the behaviors a user can expect through tests. So for the mentioned example from the video: the function doesn't recognize all caps commands inputs, this is not obvious behavior by reading only the interface, so best to create an additional test to document and demonstrate that this is intentional. In the end you should have many small tests that test 'one thing/concept/assertion' (but don't have to be strictly one assertion). The best part is that if a testcase fails, its usually very clear in telling you which specific behavior was broken.
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.
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.
@@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.
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.
If I have a large function where all of its logic is used only by itself, I consider moving it to its own class / module / file and then refactoring to get those "only ever used once" functions.
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
That's the same stupid argument people use against using modern C++ STL algorithms. If you're hired to develop code professionally, you owe it to your yourself, your colleagues and your employer to know your tools. If you're re-implementingan algorithm (like reduce) that is already part of the standard library, you better have a really good reason and "more readable" is not one of them. 'reduce' tells you exactly what the reader can expect it to do (because they know their tools, if they take their profession seriously) and saves you the effort of having the read an entire for-loop with unnecessary details (like the iterator variable), therefore it is _more_ expressive than a handwritten for-loop.
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
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.
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.
@@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.
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 😅
@@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.
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.
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.
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 ;)
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 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.
@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.
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.
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.
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.
Yes, indeed. Experienced programmers know that every design decision is about balancing tensions to find the least shit solution. The software "principles" we are taught/read/etc are really just ways to name and identify those tensions when they occur. "Principle" is the wrong word really. We need a word like "smell" that indicates they are more like suggestions than absolutes.
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")
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.
"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.
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 ;)
1:21 (#1) a bigger difference before adding that break statement is the first function will return the first user with the given name but the second will return the last. Depending on the context and implementation, there might not be any guarantee that more than one user has the same name
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! 😎
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
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.
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)?
I have two suggestions for a situation like this. First: the code that deals with reading from / writing to disk can be separated from the code that does processing of file contents. That way you can test the processing in isolation. Secondly, try to think about testing what the code does rather than how it does it. If you're patching compiled binaries, I imagine your goal is to find specific tokens and either modify the following code or redirect where the token executes. You can test that your processing code performs those actions on any synthetic byte input that has those tokens present, it doesn't need to be the bytes from the real binary file. Alternatively, for integration style tests: exclude the binary files source control and any other ways the tests are distributed. Any developer who wants to run the has to have their own legal copy of the files as a prerequisite. That's all very generalised advice of course. Since I don't know the specifics of your project there may be reasons why none of that is feasible. Hopefully it's of some use to either you or to someone else trying to do something similar but different.
@@anewbimproves5622 My specific project is for reading and writing data in the game Where's My Water, which stores most of it's data in xml files. It's hard to test the code in isolation, because an object file has links to sprite files, which have links to imagelist (spritesheet) files, which has links to image files. Since all the files are xml files, I have actually thought about creating a mockup file structure, the same as the game, but all the files are written by me (including image files),l that are specifically made to test different situations (like sprites having a width scale of 0, yes, there are a few of those in the game, or at least in later games).
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
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
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
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.
I think the spirit of what he's talking about is not against general documentation. Most code is fairly straight forward if you're comfortable in the language, so it shouldn't need a comment above every line. When it comes to function documentation however, I completely agree, I think it can have huge benefits especially after 6 months, coming back to your own code. Commenting every line though is definitely excessive and unnecessary. I would even say that 10-line functions generally don't need documentation, since a function that small should have a name suitable for its purpose. That all being said, there are outliers and edge-cases to consider, depending on the complexity of the code and the domain its written for.
// I begin my reply to you bro just because your codebase is horribly written and designed doesn't mean comments everywhere are a good thing // I say I will explain the problem with comments let me explain what is wrong with comments // I begin the explanation the comments usually end up duplicating what is already obvious anyway (unless you're extremely new and unfamiliar with reading code, in which case you have no reason to be employed as SW developer anyway). But it gets worse. Code changes, all the time. So then the code changes but the comments might not, because evidently it is very easy to forget to update comments to reflect status of the code. Good job, now you have misleading comments that are lying to you. Much worse than _no comments_ // I teach you how to solve the problem No, if you have dogshit function names that don't do as they are named, then you have to fucking rename your dogshit function names. Too hard? try splitting them up. Also, learn abstract thinking. The function name should be an abstraction of the implementation to the point that if you read the function call you shouldn't need to see the actual implemention to understand **what** it is doing. The implementation should only show you **how** it is doing it. // Are you tired of reading these inlined comments yet?? Aren't they annoying as getting bit by a goddamn buzzing bzzzz malaria mosquito while you can't sleep in a hot summer night? I hope this helps you understand, I truly do. // Do you see how annoying comments when they are literally copy paste of the code itself? bool do_you_see_how_fucking_annoying_comments_are_that_are_literally_copypasted_from_the_code = false;
@@LPFan33 I wrote out a whole ass long comment, but no one has time to read those; so all I am going to say is that if you have a million short functions without comments vs a few long functions with comments, I will hate you because now I have to draw up a whole fucking graph to figure out what functions call what functions. Functions need not be short just because some book says they should be short lol. Also I literally do not find additional comments to be "annoying". It's ridiculous that you could even think such a thing. The only time they are annoying is when it's a whole function commented out because someone forgot source control exists.
@@Internetzspacezshipz if you feel the need to read every function, probably whoever created these small functions created shitty abstractions/names and didn't make inputs/outputs explicit, so have to read the details because you cannot trust that they do what they say they do. That's a very big problem, but function size isn't the issue. Do you read every function impl. of every standard library that comes with your language? Or do you trust that container.sort() sorts the data, that log(msg) logs a message, and so on? If you can trust those functions, why can't you trust your own small functions? Therein lies the problem. If you always need to know implementation details, you cannot scale. You will be stuck reading and rereading every single line in your codebase because you couldn't rely on proper abstractions.
@@LPFan33 My man, I am not writing code like "log" and "sort". I live in the real world where code is more than sorting and logging things, the place where you need to write complex systems to handle complex problems. Where a single function call changes state all over the place out of necessity. Where someone might not immediately know why I decided to copy a variable rather than using a reference or pointer... Where someone might need to know that in order to do a particular action, they should actually use a different class that already exists... There are a million cases where writing a single comment would change someone's search for a way to solve a problem from hours to seconds. It's honestly probably just an environment difference. I work in an environment where very complex code is an everyday reality, where maybe you work in an environment where things are more straightforward. No judgement there, just that your situation is not going to be like mine, and mine is not going to be like yours. So maybe you don't need to write any comments to make your code easy to understand, but I certainly do.
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.
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.
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.
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.
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.
I also don't like principle #7. Not only is it harder to follow, it also abuses the stack for no reason. I had a senior that would say a function should be no more than 5 lines, and I'd often tell him that if he can't understand more than 5 lines at a time he should seek a different job.
Regarding 100% test coverage, if it's important to consider uppercase commands then eventually someone will file a bug report and you can write a regression test before you fix the code. However, that's actually a different type of test. It's testing business logic not application logic. Code coverage adds a level of certainty that application logic is fully understood, but it can only measure coverage of code you've already written so it doesn't help at all with missing business logic.
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.
Imagine doing a undo redo history stack with a manager class and only being allowed to have either the undo or redo function inside the class. Can only have one or it ain't clean 😤
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.
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.
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)
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.
@@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
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.
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.
The one return statement came from lower level languages. It doesn't apply to higher level languages like C, C++ or any other higher language. I write in assembly for robots, it's very important then. But that is because of the way the program compiles.
Ironic that #3 (Don't comment what, prefer why) follows #2 (Don't DRY unless WET) as #3 is often explained with "Don't repeat yourself, you already told me and the computer what the code does. Don't tell me again."
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).
There "was" a good reason for no multiple returns, but it was tied to historical practices that are less relevant today. I think it is an example of a poorly stated "good practice" heuristic. It was meant to avoid spaghetti code, which occurred more often in multi-hundred line procedures (excluding procedures with flat boilerplate.) The reasoning evaporates in smaller procedures less than roughly 50-ish lines of code. Spaghetti code often contains multiple returns, but multiple returns do not cause spaghetti code. The difficulty is in describing and defining what is meant by "spaghetti code" and its causes. It's described by its symptoms, so good practice heuristics target the symptoms in avoidance of the illness. ---- Also, I've grown weary of these "clean code" discussions and code reviews. Clean code has always been MY OPINION is better than YOUR OPINION, because YOUR OPINION sucks, but MY OPINION is awesome and on loan from the Gods. These opinions are "backed up" with coding bible verses (such as DRY, KISS, SOLID, YAGNI, SRP, OCP, LSP, ISP, DIP, TDA, SLAP, and many more.) The principles are weaponized in code review by those with very limited viewpoints on what code implementation should be. I find most of the clean code troublemakers are fresh graduates heads brimming with verses and void of battle scars from dealing with production code, real problems, shifting business logic, and hard and fast deadlines with scant resources. It's been my experience 1) you'll never get it right the 1st time; 2) you won't be given a 2nd attempt; 3) simple, short code is easier to refactor than fancy, long code; 4) you will always have to refactor (see 1); 5) premature abstraction is worse than premature optimization.
I went from almost commenting everything like shown in coding tutorials (a rare case where overcommenting seems appropriate to me) to basically only *# TODO: * or *# debug* .
and then 20 years from now people are reading your TODO's that never got done and wondering what exactly should be done about it and no one will dare to remove the comments. so I hope that you're only doing this in your own personal projects!
@@LPFan33 I have two non-personal projects, but they are basically done the same way. But wherever I feel the code isn't self-explaining I do add some comments.
@@Lampe2020 it can be appropriate, but in my experience, most of the times by far, the code could be made self explanatory which would be better than writing a comment. My suggestion is, when you feel the urge to write a comment, think for a while "How could I rewrite the code to make it more expressive", challenge yourself and it will pay off!
@@LPFan33 I write the code in a very self-explanatory way, even using type annotations where possible. But sometimes I like to compress simple loops into list comprehensions etc., so then I sometimes have to comment i it's three of them nested into eachother. But that rarely happens.
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.
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).
@@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.
@@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? ... }
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 UA-cam 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 👍
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
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.
No, it doesn't violate SRP. The only reason for calculate10Percent to change would be if _somehow_ the fundamental maths for calculating 10% of something would change. Not only is it pretty damn unlikely to happen, it's also a _single reason for change_, there are no other reasons, because this function is isolating one very specific functionality.
The explanations are great but it would be even more enjoyable if the timestamps would include the topics 🤓 because when you lose track you don't have to watch the timestamp from the beginning just to find out what it was
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.)
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.
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.
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.
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.
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
I don't like the term anti-pattern. It's an intuitive subjective personal opinion, but I think I can defend it. It's a buzz word, and I'm physically allergic to these. Ok, that's still not an objective enough reason. The term anti-pattern suggests that something is ALWAYS a bad construct. It stops you from thinking critically. I was going to give an example here...but actually your video already proves my point.
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?)
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
The log writer makes sense, but the log formatter would be relatively useless as long as you don't implement multiple logging formats. Most logs are written in multiple places at once. This should be made generic. Unless you know you won't write logs to disk or to an api endpoint or something like that
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.
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.
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.
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".
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
The problem with this kind of videos: trivial examples. So when you show those principles, I can't understand why someone would use them here? Well, maybe except `reduce` example: functional approach is more reliable and readable in simple tasks. So I agree with everything you said. Well, except spread operator performance impact. I'm not js programmer, but, as I understand, spread operator creates a new array, therefore, it allocates memory. In real code, if this spread operator is used inside a loop and operates with big arrays - it WILL BE the biggest bottleneck. But, ofc, without real code context and trivial example, your take seems reasonable, but... It can shoot you in a leg one day. Especially if we are talking about new programmers who don't use profilers to resolve real bottlenecks, not imaginary ones.
"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
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.
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).
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.
Not disagreeing with the content but the title is pretty misleading here. Anti-patterns are common "design patterns" that turn out to be counterproductive. Principles are not design patterns. Just because a principle is abused or misunderstood doesn't make it an "anti-pattern".
😮💨 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).
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.
I dont even think "most cases try to have a single return" is even good advice. I dont think the nunber of returns has any bearing on whether the code is good.
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.
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
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.
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
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.
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.
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.
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.
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)
@@ilonachan the same as c# try catch finally, comming from old GOTO cleanup back in the days
@@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
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.
No five minute intro? No sneaky sponsor? Barely even a goodbye? This guy understands getting to the point.
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.
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.
I’ve always felt that throw is mostly just goto in disguise.
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.
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
@@Fafr People wrote an entire operating system using systems Hungarian notation. Never underestimate cargo cults.
I, very proudly, break the 1st principle, every single time. 😌
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.
@@vitcermak7737 It's really only useful if you're using functions that are formatting or creating data with a specific format, so you know you're not missing something. Otherwise just hit-it-and-quit-it.
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
For compiled / jitted languages, that will get optimized fortunately
Also cleaner, I think:
return taxRate * items.reduce((total, item) => total + item.price)
@@IlariVallivaara JavaScript user detected!! You can just use .sum() in any sensible language
@@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.
@@IlariVallivaara soydev detected
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.
What if you pass null instead?
@@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!!
Yes, this is generally called integration testing. You test the code using your code lol, that's how I like to look at it.
The way I would describe it is that you should test all intentional behaviors explicitly (with individual testcases), ie document the behaviors a user can expect through tests. So for the mentioned example from the video: the function doesn't recognize all caps commands inputs, this is not obvious behavior by reading only the interface, so best to create an additional test to document and demonstrate that this is intentional. In the end you should have many small tests that test 'one thing/concept/assertion' (but don't have to be strictly one assertion). The best part is that if a testcase fails, its usually very clear in telling you which specific behavior was broken.
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.
I don't like it, but I agree. The problem is what happens if you say that to the wrong person.
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.
@@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.
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.
If I have a large function where all of its logic is used only by itself, I consider moving it to its own class / module / file and then refactoring to get those "only ever used once" functions.
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
That's the same stupid argument people use against using modern C++ STL algorithms. If you're hired to develop code professionally, you owe it to your yourself, your colleagues and your employer to know your tools.
If you're re-implementingan algorithm (like reduce) that is already part of the standard library, you better have a really good reason and "more readable" is not one of them. 'reduce' tells you exactly what the reader can expect it to do (because they know their tools, if they take their profession seriously) and saves you the effort of having the read an entire for-loop with unnecessary details (like the iterator variable), therefore it is _more_ expressive than a handwritten for-loop.
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
Your findUserByName function requires the break to be functionally equivalent to the original function. It's not an 'extra' break statement.
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.
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.
@@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.
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 😅
@@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.
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.
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.
"DRY means you shouldn't repeat business logic" - this is a great way to look at it.
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 ;)
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.
I fail to see how named constants are DRY
@@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.
@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.
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.
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.
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.
Yes, indeed. Experienced programmers know that every design decision is about balancing tensions to find the least shit solution. The software "principles" we are taught/read/etc are really just ways to name and identify those tensions when they occur.
"Principle" is the wrong word really. We need a word like "smell" that indicates they are more like suggestions than absolutes.
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")
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.
"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.
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 ;)
1:21 (#1) a bigger difference before adding that break statement is the first function will return the first user with the given name but the second will return the last. Depending on the context and implementation, there might not be any guarantee that more than one user has the same name
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! 😎
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
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.
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)?
You would create test doubles for the tests.
I have two suggestions for a situation like this.
First: the code that deals with reading from / writing to disk can be separated from the code that does processing of file contents. That way you can test the processing in isolation.
Secondly, try to think about testing what the code does rather than how it does it.
If you're patching compiled binaries, I imagine your goal is to find specific tokens and either modify the following code or redirect where the token executes. You can test that your processing code performs those actions on any synthetic byte input that has those tokens present, it doesn't need to be the bytes from the real binary file.
Alternatively, for integration style tests: exclude the binary files source control and any other ways the tests are distributed. Any developer who wants to run the has to have their own legal copy of the files as a prerequisite.
That's all very generalised advice of course. Since I don't know the specifics of your project there may be reasons why none of that is feasible. Hopefully it's of some use to either you or to someone else trying to do something similar but different.
@@anewbimproves5622 My specific project is for reading and writing data in the game Where's My Water, which stores most of it's data in xml files. It's hard to test the code in isolation, because an object file has links to sprite files, which have links to imagelist (spritesheet) files, which has links to image files. Since all the files are xml files, I have actually thought about creating a mockup file structure, the same as the game, but all the files are written by me (including image files),l that are specifically made to test different situations (like sprites having a width scale of 0, yes, there are a few of those in the game, or at least in later games).
Yeah man I have the same problem sometimes. People tend to add sooooo many comments for obvious stuff, its so annoying
// me agreeing
I agree
I actually liked the AddTax thingy, it certainly makes the code a lot easier to read compared to the reduce one
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
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
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
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.
I think the spirit of what he's talking about is not against general documentation. Most code is fairly straight forward if you're comfortable in the language, so it shouldn't need a comment above every line. When it comes to function documentation however, I completely agree, I think it can have huge benefits especially after 6 months, coming back to your own code. Commenting every line though is definitely excessive and unnecessary. I would even say that 10-line functions generally don't need documentation, since a function that small should have a name suitable for its purpose.
That all being said, there are outliers and edge-cases to consider, depending on the complexity of the code and the domain its written for.
// I begin my reply to you
bro just because your codebase is horribly written and designed doesn't mean comments everywhere are a good thing
// I say I will explain the problem with comments
let me explain what is wrong with comments
// I begin the explanation
the comments usually end up duplicating what is already obvious anyway (unless you're extremely new and unfamiliar with reading code, in which case you have no reason to be employed as SW developer anyway). But it gets worse. Code changes, all the time. So then the code changes but the comments might not, because evidently it is very easy to forget to update comments to reflect status of the code. Good job, now you have misleading comments that are lying to you. Much worse than _no comments_
// I teach you how to solve the problem
No, if you have dogshit function names that don't do as they are named, then you have to fucking rename your dogshit function names. Too hard? try splitting them up. Also, learn abstract thinking. The function name should be an abstraction of the implementation to the point that if you read the function call you shouldn't need to see the actual implemention to understand **what** it is doing. The implementation should only show you **how** it is doing it.
// Are you tired of reading these inlined comments yet?? Aren't they annoying as getting bit by a goddamn buzzing bzzzz malaria mosquito while you can't sleep in a hot summer night?
I hope this helps you understand, I truly do.
// Do you see how annoying comments when they are literally copy paste of the code itself?
bool do_you_see_how_fucking_annoying_comments_are_that_are_literally_copypasted_from_the_code = false;
@@LPFan33 I wrote out a whole ass long comment, but no one has time to read those; so all I am going to say is that if you have a million short functions without comments vs a few long functions with comments, I will hate you because now I have to draw up a whole fucking graph to figure out what functions call what functions. Functions need not be short just because some book says they should be short lol.
Also I literally do not find additional comments to be "annoying". It's ridiculous that you could even think such a thing. The only time they are annoying is when it's a whole function commented out because someone forgot source control exists.
@@Internetzspacezshipz if you feel the need to read every function, probably whoever created these small functions created shitty abstractions/names and didn't make inputs/outputs explicit, so have to read the details because you cannot trust that they do what they say they do. That's a very big problem, but function size isn't the issue. Do you read every function impl. of every standard library that comes with your language? Or do you trust that container.sort() sorts the data, that log(msg) logs a message, and so on? If you can trust those functions, why can't you trust your own small functions? Therein lies the problem. If you always need to know implementation details, you cannot scale. You will be stuck reading and rereading every single line in your codebase because you couldn't rely on proper abstractions.
@@LPFan33 My man, I am not writing code like "log" and "sort". I live in the real world where code is more than sorting and logging things, the place where you need to write complex systems to handle complex problems. Where a single function call changes state all over the place out of necessity. Where someone might not immediately know why I decided to copy a variable rather than using a reference or pointer... Where someone might need to know that in order to do a particular action, they should actually use a different class that already exists...
There are a million cases where writing a single comment would change someone's search for a way to solve a problem from hours to seconds.
It's honestly probably just an environment difference. I work in an environment where very complex code is an everyday reality, where maybe you work in an environment where things are more straightforward. No judgement there, just that your situation is not going to be like mine, and mine is not going to be like yours. So maybe you don't need to write any comments to make your code easy to understand, but I certainly do.
One of my commonest suggestions during PR reviews is to remove comments not adding value.
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.
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.
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.
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.
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.
Yeah it leads to very deep nesting of code which I hate.
I also don't like principle #7. Not only is it harder to follow, it also abuses the stack for no reason. I had a senior that would say a function should be no more than 5 lines, and I'd often tell him that if he can't understand more than 5 lines at a time he should seek a different job.
Regarding 100% test coverage, if it's important to consider uppercase commands then eventually someone will file a bug report and you can write a regression test before you fix the code.
However, that's actually a different type of test. It's testing business logic not application logic.
Code coverage adds a level of certainty that application logic is fully understood, but it can only measure coverage of code you've already written so it doesn't help at all with missing business logic.
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.
Imagine doing a undo redo history stack with a manager class and only being allowed to have either the undo or redo function inside the class. Can only have one or it ain't clean 😤
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.
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.
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)
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.
@@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
@@Veretax are you perhaps referring to error case handling, not language exceptions?
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.
Return statements are the only way to handle edge cases and invalid inputs, so one return statement rule won't work in most cases.
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.
The one return statement came from lower level languages. It doesn't apply to higher level languages like C, C++ or any other higher language. I write in assembly for robots, it's very important then. But that is because of the way the program compiles.
Ironic that #3 (Don't comment what, prefer why) follows #2 (Don't DRY unless WET) as #3 is often explained with "Don't repeat yourself, you already told me and the computer what the code does. Don't tell me again."
@5:15 there IS a way to declare the return type in JavaScript, using the @returns annotation.
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).
There "was" a good reason for no multiple returns, but it was tied to historical practices that are less relevant today. I think it is an example of a poorly stated "good practice" heuristic. It was meant to avoid spaghetti code, which occurred more often in multi-hundred line procedures (excluding procedures with flat boilerplate.) The reasoning evaporates in smaller procedures less than roughly 50-ish lines of code.
Spaghetti code often contains multiple returns, but multiple returns do not cause spaghetti code. The difficulty is in describing and defining what is meant by "spaghetti code" and its causes. It's described by its symptoms, so good practice heuristics target the symptoms in avoidance of the illness.
----
Also, I've grown weary of these "clean code" discussions and code reviews. Clean code has always been MY OPINION is better than YOUR OPINION, because YOUR OPINION sucks, but MY OPINION is awesome and on loan from the Gods. These opinions are "backed up" with coding bible verses (such as DRY, KISS, SOLID, YAGNI, SRP, OCP, LSP, ISP, DIP, TDA, SLAP, and many more.) The principles are weaponized in code review by those with very limited viewpoints on what code implementation should be. I find most of the clean code troublemakers are fresh graduates heads brimming with verses and void of battle scars from dealing with production code, real problems, shifting business logic, and hard and fast deadlines with scant resources.
It's been my experience 1) you'll never get it right the 1st time; 2) you won't be given a 2nd attempt; 3) simple, short code is easier to refactor than fancy, long code; 4) you will always have to refactor (see 1); 5) premature abstraction is worse than premature optimization.
I went from almost commenting everything like shown in coding tutorials (a rare case where overcommenting seems appropriate to me) to basically only *# TODO: * or *# debug* .
and then 20 years from now people are reading your TODO's that never got done and wondering what exactly should be done about it and no one will dare to remove the comments.
so I hope that you're only doing this in your own personal projects!
@@LPFan33
I have two non-personal projects, but they are basically done the same way. But wherever I feel the code isn't self-explaining I do add some comments.
@@Lampe2020 it can be appropriate, but in my experience, most of the times by far, the code could be made self explanatory which would be better than writing a comment. My suggestion is, when you feel the urge to write a comment, think for a while "How could I rewrite the code to make it more expressive", challenge yourself and it will pay off!
@@LPFan33
I write the code in a very self-explanatory way, even using type annotations where possible. But sometimes I like to compress simple loops into list comprehensions etc., so then I sometimes have to comment i it's three of them nested into eachother. But that rarely happens.
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.
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).
@@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.
@@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? ...
}
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 UA-cam 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 👍
4:33
Maybe calculate10Percent function solves DRY, but it violates SRP and open-close rules.
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
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.
No, it doesn't violate SRP. The only reason for calculate10Percent to change would be if _somehow_ the fundamental maths for calculating 10% of something would change. Not only is it pretty damn unlikely to happen, it's also a _single reason for change_, there are no other reasons, because this function is isolating one very specific functionality.
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.
The explanations are great but it would be even more enjoyable if the timestamps would include the topics 🤓 because when you lose track you don't have to watch the timestamp from the beginning just to find out what it was
For the 6 and 7, I can hear my colleagues shouting : "YAGNI" ;)
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.)
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.
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.
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.
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.
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
I don't like the term anti-pattern. It's an intuitive subjective personal opinion, but I think I can defend it.
It's a buzz word, and I'm physically allergic to these. Ok, that's still not an objective enough reason.
The term anti-pattern suggests that something is ALWAYS a bad construct. It stops you from thinking critically.
I was going to give an example here...but actually your video already proves my point.
All the anti-patterns spreading out code (2, 7 and 8) hurt my soul
6 too even though it's kinda the opposite
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?)
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
The log writer makes sense, but the log formatter would be relatively useless as long as you don't implement multiple logging formats.
Most logs are written in multiple places at once. This should be made generic.
Unless you know you won't write logs to disk or to an api endpoint or something like that
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.
Great explanations, agree with you on almost all points!
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.
or just throw an exception ... i think its great for branching and more versatile than returning null
Does anyone know what font is that?
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.
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".
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
The problem with this kind of videos: trivial examples. So when you show those principles, I can't understand why someone would use them here? Well, maybe except `reduce` example: functional approach is more reliable and readable in simple tasks.
So I agree with everything you said. Well, except spread operator performance impact. I'm not js programmer, but, as I understand, spread operator creates a new array, therefore, it allocates memory. In real code, if this spread operator is used inside a loop and operates with big arrays - it WILL BE the biggest bottleneck. But, ofc, without real code context and trivial example, your take seems reasonable, but... It can shoot you in a leg one day. Especially if we are talking about new programmers who don't use profilers to resolve real bottlenecks, not imaginary ones.
"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
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.
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).
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.
If the language does not support the functional syntax you have in mind, it will not work as well as you expect
Comments... the bulk of mine are disabled print statements :P
Not disagreeing with the content but the title is pretty misleading here. Anti-patterns are common "design patterns" that turn out to be counterproductive. Principles are not design patterns. Just because a principle is abused or misunderstood doesn't make it an "anti-pattern".
😮💨 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).
The exposition would have been better if the examples weren't that simple. Simple examples could be used to argue any side.
great video. SRP is bullshit. locality of behaviour is king.
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.
#7 was horrible and is a gross misuse of the sufficient functions that are part of standard packages
Number 2 is stupid, you make one function (e.g. priceReduction()) and pass the price and the deduction percentage as variables.
I dont even think "most cases try to have a single return" is even good advice. I dont think the nunber of returns has any bearing on whether the code is good.
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.
Founduser = null
If name != null:
...for loop
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
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.
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
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.
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.
that cam looks really ai generated
Lolol don't trust try go
Second comment 😊😊
First Comment Pin😓