Addressing a few frequent asked questions below: Q: Why did I say that I don't recomment an single exit point approach. A: Single exit point approaches usually include nesting which is another thing I am addressing in this video. Sure you can do it without nesting but at that point your code is needlessly complicated and not as readable (subjective). Multiple exit points tell the story of the method's flow way better in my opinion and have the side effect of also exiting earlier. Even the StackOverflow question on the matter, has been closed and marked as “opinion-based”: stackoverflow.com/questions/4838828/why-should-a-function-have-only-one-exit-point Q: Why are I doing a positive if check in 10:10 A: Totally missed that. It should have been an "if(age < 18)" check instead. Thanks for bringing it up. Q: The Open-Close violation wasn’t completely fixed since it was now moved to the class level instead of the method level A: This is correct and I mentioned it in the video as well. I didn’t want to spend more time in the video for the second part since it was an afterthought to begin with. That being said, if I was to continue I would implement a strategy pattern and have the dictionary automatically created by some convention. That way we completely eliminated the OC violation
IMO using the return instead of else is only safe if the method is small (only a few lines) and can be seen in full with very little analysis. Your cases fit this and so it works but that would be my note. If you use the first rule you in essence solve that but for people not following that rule strictly they can get into trouble with forgetting all of the wild ways they exit lol.
@@fourlegsgoodtwolegsbad Methods should be kept small. My rule thumb is that if I need to scroll to see the whole method then the method is too long (normalized font sizes)
@@fourlegsgoodtwolegsbad Just extract methods. Whenever I open a file for my work and it has functions that are too big I just start extracting them and cleaning up the code before I even start on my actual changes. Sometimes that also means I have to write additional tests. But it really helps learning that part of the code and it can even expose bugs. If a superior complains about me taking that time I tell them that I'm hired for improving and maintaining the code, and not just "data entry".
The problem with this approach, that you will fragment even a really simple business logic into so small pieces, that it became harder to understand. Also the default case in a switch can be interpreted as an else :D
@@IIARROWS Agree to both comments , for me (maybe because I'm a little bit dislexic) the else is good visually to be there, is much more easy to understand is two different blocks of code, without the else if you don't read the return, you think it's the same flow. Some time ago we had a bug in the company because a return in the middle, and one line was only 3 lines lower, but visually it's much harder to see this line is in an incorrect block if you have the return (at least for my case) And for me, when you have a method with one line, something smells... usually this kind of code means that any newbie to the code requires 6 or 7 jumps between functions to really understand what is happening.
@@jaumesinglavalls5486 totally agree. for me the video looks like "hey, this is how you can write code without using keyword 'else'". But I dont see any explained profit that it brings. Nick is doing some refactoring - rewriting 'if' tree into switch case, also he extracts logical pieces of code into corresponding methods, which is totally ok. But when he started replacing 'if-else' by 'if return', I felt like he was trying to manipulate us, using "three yeses" trick.
@@Vol4ica Exactly, the if else or if return, is the same, only that some part of code is not idented (In my opinion, making it worst to read) don't forget that identations in most of the languages are here only to help us, to represent visually something the code is doing. In my opinion the rule (at least is what I've got when writting) try to avoid the if itself, as joke usually I say "I am alergic to conditions"
I don't use "else" that much either but I don't think that forcing yourself to never use it is a good idea. I know you can always add a return early but I only use it if something has gone wrong (like them being underage in the video) and the code between the if and the return is very short. Also, I think the "intended" code should end at the last line of the method because if you are reading the code backwards (which I do when I'm trying to figure out where a value comes from) it makes it easier than having to go to the middle of a method.
I am a beginner, learning C++ & Python, and from my perspective it does seem like a good exercise for learning to become more efficient, but not good as a rule that is written in stone. Sort of like when learning to play chess, a student may start a game with less than every piece, in order to more acutely learn how to take advantage of certain aspects of the game, but would never play a tournament game by starting out with missing pieces.
There’s a developer in our office who consistently puts else clauses back in while doing code reviews. Even when I write code.... if (condition) { throw new Exception(...); } He adds the else back in for the following code and does this down multiple levels of nesting. Same with return statements. Someone once taught him functions should only have a single exit point, and he’s carried that over to C# methods. When question he say “makes it more readable to me”. 😜
I agree with@@Inertia888, branchless programming, not using the else keyword and other techniques are just different ways to do the same thing. Sometimes they can be better than the traditional way, and sometimes they will not. As well, overusing any strategy can always make things messy, as you pointed out when reading the code backwards
@@stuartmcconnachie Does your office not have some kind of coding standards in place? Especially if you have code reviews, they should check by the ruleset in place, not personal opinion (apart from the fact that the reviewer shouldn't be the one doing changes, that's the author's responsibility; but that's just what I've learned from software quality courses at university, so I guess "the real world" looks different).
@@VanLouProductions That's one of the few principles they teach you that actually hold true, at least in well-funtioning teams. What they don't teach you is how to get into that place. Having a team with shared principles and values and a culture of mutual trust and respect and candid but constructive critcism is a rare thing. It took me most of my carreer to get somewhat close to that ideal, and it's still something we need to fight for every day.
Sorry if I missed something in the vid - as I was going through it in 1.75 speed. Some examples that you described are called guard cases - and I don't recall you mentioning it so I think it's something useful to look into. For example not proceeding to drinking if under 18 is potentially a good example for guard case - because we quit from otherwise possibly a long process. Still IMHO there are a lot situations where 'else' improves readability. Recently over past year/two or so I noticed that I'm constantly being asked by fellow devs to remove else in code like: f() { if( ... ) { return A() } else { return B() } } I wonder if there is some sort of fad going about this whole else thing as it's not first time I see people talking on how to write code without 'else' and I haven't seen this idea going around as much just couple years ago. Argument that I sometimes hear is that else is redundant here. But If I go about removing all redundant things in code then I would end up with golfed code (and spaces btw - are redundant too). From readability point of view to me 'else' is not redundant because it helps keep symmetry between A() and B() keeping it on same level of indentation - to me it's slightly easier to read A and B simultaneously with single glance xxx { xxx A() } xxx B() vs xxx { xxx A() } xxx { xxx B() } It also helps to maintain intent that neither B() or A() is more special in any way (unlike in guard case situation where intent is to show that there is no process symmetry and there is special cases).
I believe not using `else` is an optimisation. I remember reading about it but CPUs can have trouble with branching code, they'll make a best guess at which branch it should read ahead into, but if it makes a mistake it has to rebuild that code path again which is slow (obviously) By not using the `else` keyword we remove branches from our code in places they aren't necessary. Generally speaking if you're writing a simple tool or DB client interface for example, writing code with less branches isn't really any use because you're not worrying about raw performance in those applications, but you might on a server running checks against certain criteria in the DB. It is also important in video games that become very complex logically and other very low-level software like driver interfaces and such.
@@Rexhunterj branching happens in either case either if() {return} or if(){}else{} Unless one deals with very low level high performance code this usually won't be anything that will matter as the code we write ends up modified in multiple levels until it reaches CPU anyway.
@@Rexhunterj You should never code because compiler will do this or that, or optimize compiled code. If the compiler doesn't manage what it's designed for then you should switch to another language if you need the extra perf.
I was about to write nearly the same thing, but thank you for saying it first. I won’t try to pretend that leaving out the _else_ statement is philistine heathenry if you don’t pretend like it’s self-evidently preferable. They’re both perfectly adequate approaches that achieve the same result. All other things being equal (and in this case I think they really are equal: claims of optimization through _else_ omission ought to be presented with some evidence) I just happen to value aesthetics far more than I do brevity.
It always irks me though, that C# programmers seem to be always finding ways to take up more and more vertical space in the code... Every time I look at C# code I can only see like 5-10 lines of actual code in the entire screen.
Funny you say that as I am a C# programmer and I constantly think about how to use a few lines of code as possible. Idk if that makes me a good programmer but it certainly feels good having few lines of code
I think it largely depends on WHAT you're doing with that code. If you're conserving space just for the sake of conserving space, I'd rather just have the extra lines. Allman is very readable and good code should be easy to follow what's going on. Not like you're going to have to re-read every line of code multiple times every day just to understand what the code does again.
Why C# particularly? I see this everywhere. I think people think they code is more "clean" just because they spread it out, plus they can be proud how many lines they wrote. The thing is, if you had mess in your room and spread it evenly in a stadium, it may look clean there, but it's still the same mess. Personally, if I would refactor this I would write something like this: private void HandleBeerAgeCheck(int age) => _outputProvider(age >= 18 ? "Here you go! Cold beer." : "Sorry... blabla");
I agree with almost all of this, except for one key aspect: The recipe shouldn't be an "active" object, i.e. it shouldn't have a "make" method, and in particular it shouldn't take an input/output stream. I think the design becomes much better if the recipe is just a data object that is passed around, and the bartender object is responsible for the "production" of the drink by reading the recipe. This also entails that the recipe itself cannot (and should not) perform an age check. Instead, each recipe should have a "containsAlcohol" flag, and the bartender should check this flag prior to making it, and acting upon it accordingly. This also gets rid of the nasty lambdas in the recipes.
100% agree with this. I believe it's called the single responsibility principle. Also I really cringe at the dictionary of Action objects, incurring unnecessary allocations and invocation time. I feel like in this example Action objects were used as if they're as cheap as a function pointer, which they're not. TL;DR: Action, yuk!
But but, I want the glass to ask me if I'm of legal age, not the bartender! Jokes aside, it can be seen that understanding exactly who should do what, the responsability of each object, it's quite hard.
@@jason_v12345 that's a good solution... that is, until you have multiple "features" of a drink you need to check for. You could use multiple marker interfaces but it gets messy.
@@jason_v12345 I am absolutely in favor of OOP. But subtyping won't help you much here. You can have a "ContainsAlcohol" interface with no members to denote that. What's next? "ContainsMilk", "IsVegan", etc... zero-member interfaces are an anti-pattern as well.
Nick is probably aware of single responsibility and he chose to have his code be the way it is because he thought it would be more simple. See the wall of text at 14:17
I think returning early and then removing the else makes the code less readable and also extending it later on, you‘ll have to fully understand it first.
In this example he swapped the return early's around (see pinned comment), introducing confusion. However good practice is to return early on invalid input and then have your validated input be used at the end of the method. This is the guidance that most top developers give, and is used all throughout the modern .NET code base.
Yes, if your check is valid/invalid then an immediate return on invalid makes sense. Anything else adds confusion because "else" tells you something about the intended logic of the code that isn't obvious when removed. But then I code in a very different style that may not be readable to others...
This, I prefer keeping an "if ; else" flow because it's immediately clear from the structure that the two sections are mutually exclusive. If you have any sort of code folding going on, then you wouldn't see the return until you examine the contents and realize that section never goes past the block.
Okay, some comments on this. Moving code which is never reused to a function is a nightmare to read. Not only do you obfuscate the code by moving it somewhere else, but you also make it appear as if it's going to be reused, which is not the case. It also leaves you with naming, and when code bases grow big, having a plethora of obscurely named functions is not very good from a tech debt perspective. You also create a longer call stack, which is making it harder to debug. And, not to forget, even if function calls are fast, they do have a cost! Returning early is great for when you have simple conditionals or if you have a set of conditions which are supposed to terminate the function early, but if you have a scenario where it's not a binary condition, i.e. you will have an if-elseif chain, then how would you solve that? The returning early scenario is usually used or thought of as a function having a fast path and a long path, meaning you can, just like you say, return early if a function performs its task or has to abort. But having something run after the conditionals may also get forgotten over time and you end up running that code just because you didn't return in one of your conditionals. Isn't it better to be more explicit about what code you run? With that said, I do agree that big scope chains are annoying to read, especially when closing them, but I still don't agree that returning early is good a universal solution conditionals. Talking about code bloat, is it really less bloated to create a function for a conditional? Look at how many lines of code you have in the end, I am fairly sure you started at 50 and ended with like 70 in the first file and something over 30 in the new, so you doubled the amount of text for basically the same functionality save for a slightly more useful way to add more drinks. I could be wrong but this whole RecipeBook class could be reduced to just using a static map with a String -> lambda mapping, which would allow you to move the drink initialization to the same place without having to introduce an extra class which is used only once, and it would incur less coupling in the code. I hear these mantras and these manifestos, but I hear very little explanation or defense of them. Why do you only want one indentation per method? What's the logic behind it? What's are the benefits? I'd like to break down why these rules are useful and actually get a good argument for why one would want to use them. What I usually see is very typical OOP, abstract everything until you have made everything reusable, and then in the end, never reuse anything.
First point with moving code, highly disagree since you should be able to name small functions well, then i can just debug that single function that's giving an issue and the name should be able to be descriptive enough that the name gives away what it does. If it does more than one thing, then you've failed, and that's where naming issues come in. Second point, you're just not writing else since you exit early, so you're bringing in an issue you'd have by not correctly formatting and exiting early. For multiple if else if you have a switch statement with a default case, which is your else. More or less code doesn't really matter that much within reason, it's more important that it's easy to read and maintain. Making every function do one task means it's easy to debug later down the line, I know, I'm currently working on a bunch of spaghetti code for nearly 2 years now, and we're finally at the point where it's okay to introduce juniors to the code base since they don't need an extreme background in the flow etc. to be able to understand what's going on, they can take each function at a time to understand what it does, step by step. And in regards to his examples, all the stuff is simple, imagine it was more complex stuff, decisions with routing, calculating financial interest based on a multitude of factors, etc., that would all be more complex things, these examples or to explain what, not a best-case use of it. I like SRP, if it isn't properly followed in a large code base, issues crop up pretty fast.
@@Masterrunescapeer If you make tons of small functions you mismanage the purpose of a function. It’s meant to be used for reusable code, and making a bunch of small functions that just run in sequence is of no use when all that code could live in a single function. I’ve seen so much code where there are hundreds of functions being run in sequence and only in that sequence. That just adds a constant execution cost to your application (having to push arguments to stack and the return instruction pointer, as well as popping the return instruction pointer) and obfuscates the code for absolutely zero gain :).
@@gustav1416 in regards to issues with overhead, your compiler is smart enough that that should very rarely ever be an issue. It's easier to write multiple encapsulating functions that go step by step than one giant function, e.g. Flow of sending a message you'd have a controller that orchestrates accepting it, then process into usable, then send to task, then convert result to correct format, then return. In regards to too many small functions, doesn't matter if function name says exactly what it does, I'm not talking about taking a one liner and making a function out of it, I'm taking about a few lines that are repeated, e.g. Take header from a cookie and do comparison to check what user type and then check is valid for call or something, that's a recent five liner I split off and made generic. Or if it's a business rule, the function on its own is maybe mathematical, say function is area of a circle or something, you'd split that out so you do calcCircleArea, even though one liner so name is there for easy understanding. And no, would not do it with circle area calc, I'm saying rather business rule or other more complex formula. Then removes need of comment, comments get outdated/lost, and quick and easy to debug as an isolated function.
@@Masterrunescapeer If you’re talking about JavaScript you also have the increasing cost of the code download if you keep adding unnecessary text :). Also, don’t rely on the compiler to optimize, you always want to aim for your code to be as optimized as it can by you. The compiler might especially find it difficult to optimize calls between libraries as optimization is most efficiently and most simply done within a compilation unit. Breaking code down to single use functions is of no use at all, all it’s doing is forcing you to split code in arbitrary places and it stops you from moving code around easily without renaming functions and restructuring code. On top of that, you have to navigate the code, jumping back and forth trying to keep the trail of breadcrumbs in your head while you do so, when all of that code could be in a nice neat sequence. I’m not saying code should be repeated at all, I’m just saying I’ve seen code split into functions where the functions are used in only one place, thus making that split a waste of time, effort and at a detriment to readability.
So this is where that kind of extremism comes from. I mean i totally see the reasoning behind it: simple rules But simple rules followed this strictly are the exact kind of good intentions that led to certain bureaucracy hellscapes. So i have to kindly ask for your permit A38.
Totally true. Langages have if/else as well as switch because they're performing well for different scenarios. This video should be called : why using if/else in a switch case problem is bad. Aside from that there are real life cases where if/else is justified.
It is clearly stated multiple times that this is not a strict rule. The advantage of trying not to use else is not removing the else itself (although I think this is an added value), it is that in order to do it you need to write good code in the first place. So the rule should be: If you find yourself writing an else statement, try to remove it, if you cant you should probably refactor your method.
Yep thinking too much will reduce the productivity especially in big project. Also we dont usually have the luxury of time when its a product. Refactoring stage comes after anyhow.
Although I agree with the video. The example of jumping out of the function in the beer serve check seems to be backwards. It makes more sense to write. If (age < 18) { //Error here Return; } //Continue to serve beer here This way you jump out of the function when there is something wrong instead of assuming the code jumps out of the function when it's right. The rest of the video seems nice. :)
It's cleaner alright, but what about the cpu's branch prediction? Isn't there a silent assumption that expression in if statement (most of the time) is more expected to be true (except for null pointer/ref check)? In init functions cleaner approach is fine, but on the critical paths / in loops might cause a lots of cpu pipeline flushes resulting in drop of performance.
@@sprytnychomik you would need to check the compiled code to verify. It is not easy to say if it would cause any performance drop. Modern compilers are very good at optimizing code. In case of doubt you could check the execution time. It really depends on the scenario. In most cases I would assume any small performance loss is negligible. Only in other case where performance matters due to many loops etc I would try to optimize the routine for speed.
6:40 the problem with this approach is that many codebases require you to search through methods later on. Seperating one function into four makes that task a lot harder. It works great when you read the code from start to finish, but that's often not feasible in larger projects. This approach of micro-functions also creates a lot of vertical seperation throughout the file, which can drive related important pieces of code far apart when it would be much handier if they were adjacent.
I’ve been doing developer support for a total of over 6 years now, and lots of tiny functions nested deep through a dependency graph of a lot of objects that exist from who-knows-where (often set up long before you get to that code) in large codebases (either customer’s code, or OS code) especially when debugging release optimized code in assembly (without full source, but hopefully symbols) is a real nuisance. This is especially true and a major combinatorial explosion when there are multiple types of possible subclasses or implementations of interfaces/protocols (depending on language/terminology). You can read all the source code, but with a large enough OO codebase, do you really have an efficient way a priori to know which types of classes will actually be used without verifying all the possible types? Nope, not in a practical sense. Meanwhile, long procedural functions with relatively short stack depth is relatively easy to identify what’s going on, because you’ll likely have more information at hand to identify which things could actually be called: fewer potential breakpoints you need to set to identify actual flow. Multiple levels of OO dependency graphs with subtypes of objects is just a more abstract way of representing an arbitrarily complex dependency graph of hairy switch statements in a manner that’s deceptively simple in appearance: both an asset and a liability for understanding the code flow, depending on the time and reason you’re reading it and trying to modify it.
I know it leads to VERY different looking code, but if a function is only ever called in one place, put the code there and use comments. When I’m debugging, I don’t want twenty function calls to follow if I only needed three. Especially because after I’ve finished reading the nested function, I need to read back up the next twenty functions it was nested inside
Also if you put it in an extra function someone else might come along and call that, even if it was only ment to be used in that one context, and now you suddenly have to worry about backward compatibility when you need to change that function.
That was my concern as well. I would much rather have the code in front of me, rather than having to search for it, particularly when it’s a one off that still fits in two to three lines of code.
Well, you don't always need to read every line of code sometimes and function names can quickly tell you what a segment of code does. If you also write your code in a modular and functional way you can easily test your code too even if you only use that function once.
Agree. By treating "if else" statements instead as "if guard condition do something, return", code is much more readable. The hot path should be easily identifiable.
As with most rules, it's useful to know why it's a rule in the first place. While I do think using returns and switches cleans up the code nicely, there are certain cases where using an else is just simpler and cleaner than blindly following those rules. This was pretty informative and presented a good list of coding guidelines and best practices but one shouldn't be afraid to break a few occasionally if they make the code more convoluted than the simple approach.
I would argue that splitting out methods to remove nesting is often just as difficult to read as the nesting, especially for debugging purposes. Having to hop from subroutine to subroutine just to figure out the CONTEXT of how the bug happened is a lot of work. I personally find it a bit easier to debug the nested version, because I can quickly scan up and - for example - figure out exactly what line results in the object 20 lines later being unexpectedly null. Doing that through several levels of subroutine function calls often requires me to re-run the code in debug mode over and over, once for each level of function call. Of course, both arrangements are difficult to read in their own ways, but the REAL reason for the difficulty is that the design is a brute-force decision tree. Your refactoring into a dictionary of method calls is a great way to simplify things: each drink has its own code, so I know where to debug right away. Sometimes, however, we're stuck with a brute-force decision tree. Your dictionary works because it conceptually models the requirements of a bartender very well: one action per drink. Easy peasy. In real-world coding, we often have a fairly tangled mess of requirements that doesn't parse out so neatly, namely that the requirements are a set of arbitrary rules with all the "if then" logic stated in those rules. In such situations, I find that having the coded version of those requirements closely parallel the literal requirements, no matter the level of nesting, to be the most readable - because you really can't make it more readable than the requirements. You might be able to reduce the requirements into a simpler logical construct, but then everyone after you will have to prove to themselves that the simplification is correct, so while your code might be more readable, it's more difficult to verify that it's correct. Further, if code closely parallels requirements, then it is possible to verify whether the requirements are correct! For example, SME claims your code has a bug, you show SME that the code does exactly what the requirements say, then together you both can determine whether the requirements are wrong or the code is wrong. Yeah, the code is difficult to read because the requirements are difficult to read, but dang, matching the requirements closely saves a lot of work. The guardian "return" approach you use is good, especially as I find it very easy to debug code that returns a failure quickly, as it is obvious which condition isn't letting the main method proceed. Outside of guardian code, however, having multiple exits makes things difficult to debug, especially with several levels of function calls, because it's almost never clear which call in the chain is the reason for the early exit: the guardian code needs to be at the beginning of the method, not layered throughout. Then there's the overall question of what makes code more readable. A short essay is readable because the paragraphs organize the ideas in a way that shows how they all relate together in a global sense. Some ideas are more important than others. Some ideas deserve only a sentence fragment, not a full paragraph (their own method), while other ideas deserve to be fleshed out in their own paragraph (or even two). Similarly, code is made readable by making the lines of code clearly denote the relationships of the ideas involved. You determine that it is readable code by READING IT when you are done, and editing it to make it more clear in a global sense, not in a nitpicky "correct grammar" sense. In code, we tend to pay attention to the trees in preference to the forest, because we have to analytically verify each "tree" to make sure it works right, but those who read our code necessarily start at the "forest" level, and we want our forest to be arranged such that they can quickly figure out which tree(s) to look at. There is an art to this, as what is readable to one coder isn't necessarily readable to others, and it takes a while to learn what other coders think of as easy to work with, and what they find difficult to work with. This knowledge cannot be reduced to an analytical set of rules.
I _always_ have this problem, the deeper you have to go within the stack the more you have to rely on your brain to keep track of a given path through your program. Perhaps rules around nesting should relate to subroutines as well (a method can only call another method, and maybe that one can call a method only). Another rule might be that you're not allowed to split a method unless it's used in multiple places, readability be damned, DRY is more important than clean-looking.
I think your main topic isn't the debugging or the context but having to deal with poorly written code. All your arguments come down to the fact that the code probably sucks and has many side-effects in the subroutines. If you know why you break it down in individual functions and understand clean code it will be a pleasure for other coders. With (almost) no side-effects the context is super small at any place in your code and the functions/variables tell you a story instead of showing you a forest. 😉
@@malino24-souls The problem I find with "smaller code units" is that it becomes remarkably difficult to find the specific unit of code that has the bug. Even if I have a stack trace that specifies the right line, the cause of that exception is typically an unexpected null object: there is no bug in the line throwing the exception, the bug is whatever is providing bad arguments, and the bad arguments typically arise from some much earlier call that is also working just fine, but in that particular instance the database or REST service has no data. You might argue that we just need to handle the null case and be done with it. Once in a blue moon, that is the correct way to handle it. Far more often, the business requirement is to figure out why we have bad data in the first place, and at a minimum, my job is to figure out where that bad data is coming from, so someone else can hunt down why it exists on their end. "Smaller code units" is a good approach when considering only whether the code logic is self-consistent and correct, when the fix needs to be made in the part of the code throwing the exception, when the CONTEXT of the code doesn't matter. It's not so good for cases in which context matters, such as exceptions that arise from unexpected real-world data, because that bad data arises elsewhere. I need to know the overall context in order to have a clue what data was expected. And not just what class or what type, but enough info, for example, to tell whoever manages the data loading which specific id is missing from their data set. TL;DR - in my experience, most bugs don't arise from a bad line in a specific unit of code, but instead arise from the chaotic and unpredictable interactions of hundreds of perfectly-correct "smaller code units". The faster I can find which specific path through those code units causes the error, the better.
@@uumlau that makes sense to me. Unfortunately I'm not as proficient with real world examples. But what I can say is, that if a code unit is considered correct, it shouldnt be the source of error in said case. So if the unit accepting the passed data isnt verifying (let it be by simple asserts or complex analysis about "correctness" of data) the data, then may as well consider that unit not correct or at least error-prone. Not sure how wrong/correct I am, but that's at least from a logical point of view and with my own programming experience.
I'd rather use ELSE than have multiple exit points. Having said that, one way I reduce the use of ELSE is when flow isn't affected like setting variables. For example: x=1 If a=b { x=2 } Rather than if a=b { x=2 } else { x=1 }
But you'd not do that, something like that you'd do `var x = a == b ? 2 : 1;` which is technically also an else, but you're not using the keyword. Multiple exit points if they are correctly structured as guard statements reduce complexity, since nesting if I have a large trail to go follow to see how to get there.
i instantly subscribed when he did the outro in the middle of the video for the people who were just interested in the title. a content creator that cares about views time like that will always produce quality.
The clean coding aside, the extract method and where you created the switch shocked me... I knew the ide did stuff but I never really looked into it and just did things the hard way for 25 years.
Hey, Nick! Thank you for another amazing video! As for the second part of the video, it is even better that it was not prepared in advance, because watching you think out loud, analyze the code and make decisions is also very valuable. 👍
I preferred the original method, I don't wanna be scrolling up and down looking for other methods when I'm trying to find where the code flows after I've been debugging for 3 hours
If I was video blogger and decided to make video on this topic, I would call it "Why you should reduce your nesting as much as possible", problem is not with "else" keyword at all, problem is about unneccesary deep nesting. I rarely convert my code into switch statement, but if my method contains more that two nested ifs, I reconsider my architecture.
I highly doubt I would click on that video based on the title. This is both about else and nesting. When nesting was 1 level deep, I still removed the else because it's pointless. Nesting isn't a familiar concept to most people while "else" is.
I definitely agree that removing "else" can make your code look less bloated, thanks for tip. Also really liked the second part of your video and seeing your thought process, please keep doing it!
"Early return" has its benefits, yet it can also make code harder to read if it becomes longer as there are many places your method may be left instead of just one. Generally I only use else for "final statements", that is code that shall be executed no matter if the if-path or the alternative path has been taken and quite often such statements don't exist, in which case I will also use early return. However, following your sample, I would neither use if-else nor switches, I would actually use objects and dynamic dispatch at runtime, which is even less and even simpler code. The code is split upon more objects, yes, but the code each object is tiny and if you need to add another case, you don't need to touch existing code at all, you just implement a new object.
If you have a situation where you need this kind of if/else or switch constructs, you should just go polymorphic, create an interface, make different implementations, get the implementation and call the method. No more if/else or switch.
You may think they’re all right, until they take shortcuts and sign for your Apple m1 Mac Mini claiming they delivered it, saying you rejected it (even emailing me a picture of “proof of delivery” those assholes, when they tried to force it off on the apartment management, even though I was home the entire day and they never got in visual range of my building), so it gets shipped back to an Apple facility across the country automatically per signed package policy of Apple, and they put the cuss in customer service and leave it to you to get it. Lazy dishonest identity-fraud-committing driver, he and his manager should be fired and then charged and punished for that crap. A true story, they take “efficiency” to illegal extremes.
I really disagree with the no-else notion - enough even to actively refactor code away from it. I have two reasons for it: I can't see all of the returns at a glance and it hides nesting. The first point comes into play in the phase 1 of reading code: the skimming through. And here it already helps to understand that there are more cases. And at a glance I can't see whether or not an if block just does additional computing or is a possible exit point for the function. This is an important distinction. If I use "else" however, I can clearly see that there are multiple exit points depending or not on whether the return is indented or not. Moreover, I can quickly see how many (and where) they are by seeing the elses and ifs that aren't indented at the same level as the rest of the code. The usual answer to this, that I hear is that methods shouldn't be longer than 5 or 10 lines, and I partially agree, but that's not always possible, sadly. The second part of hiding nesting is that if you don't use elses, you can easily get dependencies in between the ifs or before the last if and the end, which is essentially nesting, just well hidden. E.g. the following (nonsensical) function: NO ELSE: int getMultipleOf3(int number) { if (number < 3) { return 0; } if (number == 3) { return 1; } int numberMinus3 = number - 3; if (numberMinus3 == 3) { return 2; } if (numberMinus3 == 6) { return 3; } return -1; // too much } THE SAME CODE WITH ELSE: int getMultipleOf3(int number) { if (number < 3) { return 0; } else if (number == 3) { return 1; } else { int numberMinus3 = number - 3; if (numberMinus3 == 3) { return 2; } else if (numberMinus3 == 6) { return 3; } else { return -1; // too much } } } With else I immediatedly see which dependencies are used only for one if and which are necessary for more and can see that the whole block using "numberMinus3" should actually be addressed (e.g. by extracting it into a method). In the first code, when I read the line where "numberMinus3" is define, I don't know where it is used afterwards. The IDE can help with that a bit by highlighting, but it's still not immediately apparent. P.S.: the approach using else is (part of) structural programming if somebody wants to google that.
Pff just use a language with pattern matching match/case statements and flatten all your conditions 😂 Condition-heavy code isn't great anyway. I don't think a hard rule like "only one level of nesting" is a good practice, don't extract methods just to keep low nesting, extract method when its a logical nameable thing. Of course you should strive for little nesting, but I don't stress over it either. Besides, that extra nesting where he has age >= 18 would go away if he used `else if {` instead of `else { if {`. I mean, I get that examples are hard to keep meaningful yet simple enough to understand and he does call this out when he mentions the SOLID principle. That would largely solve this particular case anyway.
@@guywithknife Oh sure, switch statements are great (I actually have a section of code right now that should be refactored to use it, heh). And you're right about "only one nesting level" being nice, but not critical. I've got nothing against something like: for...{ if(obj != null && obj.somevalue == checkvalue) { //do simple thing instead of for...{ if(obj == null || obj.somevalue != checkvalue) continue; but I will do things like for x...{ for y...{ if(x+ox < 0 || x+ox >= width || y+oy < 0 || y+ox >= width) continue; if(x == 0 && y == 0) continue;
@@draco18s even switch statements are quite limited, I was thinking more like what many functional languages provide. Switch statements can help, but in general the SOLID principles apply: don’t hardcode your conditions if they’re complex and may change. I agree with your example, I do that too. I suppose it’s just whatever is readable to you (in six months time when you’ve forgotten what the code does) One thing I would comment on with your example is that I rarely use continue because I rarely need to. Often it’s possible to apply Mike Actons data oriented programming advice and remove the items from the list in an earlier step, so in the loop you don’t need to skip items at all. Obviously that’s not always possible, in which case whatever, but when it is, I find it super helpful both for clarity and performance.
I must be missing something because in my opinion this is harder to read. I agree with this statement completely, if your code is nested that deep it's probably better to just methodize it out.
@@davidyoun8721 The lack of syntax highlighting probably isn't helping. I just meant that if the contents of my if-statement is like one or two lines, I generally won't bother with an early exit (because the early exit is *also* only one line). But I do early exits for for-loops to check for out-of-array-bounds-ness though. There's no point in extracting it out to a method because that for loop nest has *already* been extracted to a method (eg. count_adjacent_matches(grid, x, y, matchTarget))
10:13 usually instead of returning if your conditions are met, you would do it the other way round, also known as "returning early on error/invalid input". That way you can have multiple conditions at the top which will exit the method early, and the final chunk of code is where you handle your "valid" input.
Yeah that was a brain fart, I mention this in the pinned comment. I never actually code it the way I showed in the video so no idea why my brain at the time thought it was ok
@@nickchapsas No worries I just read the pinned comment after writing mine! Good video though as it does introduce some new concepts to new programmers. I'm just watching because I'm bored xD you are entertaining.
In my opinion, early returns are less readable than explicit else. Your code doesn't match your intentions: you intend to do the same thing as an else, but you instead use the early return. To me that's much worse than the indent and extra lines. If you put open braces on the same line, it's only one extra line with increased readability. (I know that's not C#'s thing, but if we're looking to reduce bloat...)
Early returns are meant for filtering. You filter out what you don't want to process. Instead of using if else which sometimes the filtering will be placed near at the end of the function. Another reason not to use "guard clause", is to reduce scope level, allowing you to have better control over the lifetime of the object in C++ for example. Free unused variable/memory early Imagine you have a function that will only process the data only if certain conditions are met, the instruct flow will go by, condition 1 not met? Out! Condition 2 not met? Out! So and so.. instead of going through don't know how many lines of code before I reach the point of "else return" Apart from that, it's harder to keep track of which condition leading to that return without collapsing the whole scope of if (reduced readability)
I see early returns as checks to go through. Not necessarily guard clauses, but rather specific cases for which we want a behavior different from the default one. When you see them like that, I personally find them readable. Get the edge cases out of the way and instead focus on the default behavior, really helps me with finding things readable.
Why use IEnumerable instead of List? Bonus Question: How would you write unit tests for those private methods? Also, thank you for 2 things: - Showing real world example of Delegates - Introducing me to Object Calisthenics
I use an IEnumerable because a List is a mutable data structure. I could use an IReadOnlyList instead but at that point an IEnumerable communicates the type well enough. The private methods are unit tested via the public method. You don't write unit tests for unaccessible methods but you write scenarios for the public methods that those private methods are being used in.
I haven't really been using the else keyword for a very long time either. I didn't really realize it until a few months back either, but I think it has to do with my personal theory of 'The less indentation, the better' and generally you get more indentation from else-statements
Interesting! I much prefer using the new switch expression over if...else if...else, but I can’t see the utility in completely removing simple if...else blocks. I think understanding the reasoning behind some of these “rules” is instructive, even if one doesn’t necessarily implement them.
You sacrifice the subtle simplicity of always having one place where methods return for what is essentially a distaste for indentation and brackets since using else is not nesting. I think multi exit is ok for 5 maybe 10 liners with max 2 exit points. Beyond that, it stops feeling good.
Well i wouldnt say that there is a distaste for identation and brackets but more an approach to make the projects more flexible and scaleable. Imagine you'd have to implement more complexity to your software like considering laws of different countries or translations or an API for third-party. To give you a more visual representation of this imagine the following: Always throwing your cloths on a big pile in your room has the simple simplicity that you can always find everything you want to wear in one place. You dont even need a drawer. ... Makes (maybe) sense if only have like 2 jeans and 2 shirts But what if you buy new clothes . What if you want to sepperate your expensive shirts from your old sweaters? What if you dont want your other shirts wrinkled just because you are looking for a pair of jeans. What if you're tired of spending every day 15 minutes just to find your socks? The bottomline is .. the more your software grows the more you want to keep a specific structure which allowes you to minimize the effort for changes or extensions.
Multiple exit points breaks one of my strictest rules, so that's a definite no. I have no issue with simple if-else statements, but if-elseif-elseif-elseif-else type statements should be avoided if possible.
I HATE having to scroll all over the source file to read code that can fit all in one screen because someone thought breaking it out was "more readable"!
Besides most experienced programmers already return early if they can and it is obvious to do so. Why use a temporary variable when you can just return the results directly. This video went way out of scope. Explaining why you don't need else takes 1 minute tops. All the rest is about clever ways to refactor your code.
@@NicolaiSyvertsen I was thinking the same. I like the guy's videos, but this one really felt like it stretched the material. Didn't need 20 minutes in what could have very easily been 10, and 5 is still more than enough.
Good practices are rarely universal. One needs a sense of 'taste' to determine whether the practice is applicable in a particular situation. You mention this quickly at the end 20:06. This applies to extracting methods, return instead of else and creating classes instead of switches. If you use extract method *where you shouldn't* you end with methods that have little meaning by themselves, are highly coupled together and code that is hard to understand because you have to keep jumping from function to function to follow what's going on. If you create class to remove switches *too aggressively* you may introduce a lot complexity (classes with composition/inheritances) that end up making code more complex than it needs to be. Other times it's the perfect choice.
I find smaller methods with most specific code way easier to work with and the method read way clearer to me. This is subjective, as is the whole video but I get your point as well.
Started doing this about half a year ago, really helps clean your code up. If you mix this with making sure your functions are honest will naturally make you follow the boy scout rule. By honest functions I mean, don't name a function something then hide some code in it that's not obvious to the name of the function. It's a very common problem because it's so easy to do. A quick example is something that returns a value. Don't manipulate your program in a function that, any dev reading the name would assume, just returns a value.
Is adding return instead of else really better? You can overlook the return and think the else part is always executed at the end. But you can't overlook an else which adds extra intention.
It's definitely not better. Totally violates one of the principles of Clean Code. The reason it looks cleaner is because adding an else creares an extra 3 lines of clde since he's using uncuddled curly braces. Really dumb imo. Just use ternaries.
Omg yes, that's what I thought. I feel like it actually makes the code less flexible. Say you have to edit one of the 10021 methods you need to create for this, and in one of them you forget to remove the return statement or whatever. It's gonna fuck you up. This is just so messy and illogical.
i really liked the part when you free styled and were applying open closed, the comments in hind sight were really usefull and gave me a clear insight into how i need to approach this kind of refactoring! you are great
Hmmm... numerous replies below already express the reaction I had, i.e. that pouncing out of a procedure in the middle makes things less, not more readable and maintainable, "s/return/goto theend/" comes to mind. Can programmers these days really only cope with one level of indentation? IMO the success and failure paths should both reach the end of the procedure and many client coding standards documents would agree. Sure having these return statements is easily spotted with very very short procedures but again, I'd say that's taking it too far. A well-crafted procedure of a hundred lines or so shoudn't present a problem to anyone to read and understand, if the job the procedure has to do is best expressed that way. If complicated parts can be sensibly extracted out, or are used elsewhere then fine, but to end up with several times the number of procedures so each can have just 3 or 4 lines of code makes the code base a nightmare. It's all very well with your example of half a dozen interfaces but get a team of 10 to code something for a couple of years, then a variety of changing staff to maintain and update it with all sorts of unforeseen client requirements for another couple of years with this approach and 20% of your code base will be function declaration and the notional namespace of procedure names will start to fill up.... "Dave, can you fix that bug in ServeMojitoToPersonWhoIsOldEnoughButAllergicToMintAndDoesntLikeStrawsAndPrefersABambooMatAndNoUmbrella() please" - sure, while I'm there can I put parameters in for mint allergy and straw, mat & umbrealla preference and remove the other 12 procedures that do almost exactly the same thing?" NO Dave, then the procedure would be more than 4 lines, go wash your mouth out with soap !!
This is just silly. The "default" in the switch is equivalent to the "else" keyword. Furthermore, the ternary operator has an equivalent else part. This really is a personal choice with highly subjective advantages.
If you watched the full video you’d see that the switch was refactored out as well. Also something can be the equivalent of something else but doesn’t mean it’s the same
@@nickchapsas Default is basically else... Seems like you're splitting hairs here. And what's wrong with else? It clearly conveys the intent of the code, unlike an abrupt return in the middle of your function.
I've been retired from coding for about 6 years, and I see that new syntax has been added to C# since then. Now I'm rather interested in finding out more -- thanks for waking my coding brain cells up!
No one: Clean coders: don't use "goto"! Everyone: ok, seems fine. Clean coders: don't use "else"! Everyone: hmm maybe? In future, no one: Clean coders: no more than 4 lines of code in a function! Everyone: what? Clean coders: don't use "if"! Everyone: wait, what? Clean coders: don't use ! Everyone: wai... Clean coders: reject humanity, return to monke! If you do not use "else" it looks a little less bloated, BUT at first glance, it looks like you print two messages instead of one. And if you collapse the if statement for whatever reason you will for sure think that second option is printed always. With "else" you know for sure, and at a glance that it will only be used sometimes. Also, else makes it more symmetrical, which is a aesthetically a plus for me. But that is only my opinion, it may be very well valid point. I, for sure am not a clean coder yet ;-).
What I hate most about C# is the lack of fall-through, and only way to do so is to use a goto case, which means everyone reviewing it instantly throws it out since dreaded goto keyword. /sigh (Do agree with not using Goto though, ends up with the most spaghetti code ever, 98% of the time there is a better way of doing it)
@@Masterrunescapeer case statement fall-through is one of the hardest features to maintain. It is terrible. It does let you write shorter code. But objectively harder to read.
I'm a great fan of using return to avoid if-else nesting whenever possible. However to state that only one nesting level is allowed per function and that else must be avoided is too much like a religion to me. It all depends on whatever is the most readable in the situation at hand. Having a plethora of small extracted functions is not necessarily easier to understand and maintain than a chunk of code, unless that chunk becomes larger than, say, one page or screen.. As for using returns, there are also people who insist that a function should always only exit at the very end. Which makes sense if something needs to be cleaned up like closing a file or disposing an object. All in all, there is no right and wrong here. Whatever is most simple and transparent is best, IMO. Never mind principles or paradigms. I'll also happily use a goto when it suits me (though never, as I've once seen in a COBOL program, to a label in another function 😵).
I stay by the principle that a method should have a well defined entry and one exit point. private void HandleBeerAgeCheck(int age) { string msg = "Enjoy your beverage of choice!"; if (age < 18) { msg = "Sorry, but you're not old enough to drink (in the UK)"; } _outputProvider(msg); } Accomplishes exactly the same thing - but there is one entry point and one exit point. You could streamline it by: private void HandleBeerAgeCheck(int age) { string ageOk_Msg = "Enjoy your beverage of choice!"; string underAge_Msg = ""Sorry, but you're not old enough to drink (in the UK)"; string msg = (age < 18) ? underAge_Msg : ageOk_Msg; _outputProvider(msg); } I hate troubleshooting a routine only to find out that it exited in the middle, or having to adjust the code to add some logic because of a premature return (For example, say later you want it to always output "Please do not purchase alcohol to be consumed by a minor!"). How would you do it? It's not hard a all to add that to either of my examples - one line.. done. There are simply much better ways to accomplish the goal. Myself, I don't mind else statements but I don't like a bunch of nested if then/else statements, which are easily handled by refactoring like you did. One entry point - one and only one... exit.
This is what I call "gated if" statements. Makes code very neat and easy to follow and update. Also makes it really easy to add a comment for each check. It is limited to methods because you have to be able to exit at each check.
The problem is the checks become order dependent and it may not always be clear where to add a new check. Also some checks can become convoluted if you need to check four things before deciding to return right away because now you get a complex comparison statement.
Problem with early returns is that it puts all code under your IF block into a virtual ELSE block and you need to keep inspecting the entire contents of the IF block for RETURN statements to see if the code below with be hit or not. I'd often prefer the ELSE blocks be explicit as opposed to virtual. You want to be clear about the fact that the code below only runs conditionally. I mean both approaches are defensible in the end... I'd mostly advise against hiding return statements in large IF blocks where they could realistically be missed.
Yep, I do that also. When I have lot of conditions to check, I start to check most common ones and go out of the method/function straight away. So no need of if else if else if else blocks. Only If statements with straight return if condition is true. This makes the code way more readable (and you can write on a single line) but also much faster because you won't check useless conditions. Looks like : if (condition1) {return false;} if (condition2) {return false;} .... if (conditionXX) {return false;} return true;
One thing to keep in mind is that this might clean up the look of the code, but functionally it is doing the exact same code either way. If you go down to the assembly language level that this generates, the return in the if statement does not actually return from the function right there. What is does is a jump (jmp) to the code at the end of the function right after the code at the end. This is the same that is does when there is the else there. It jumps (jmp) to the code after the else. From a performance standpoint, this does nothing. If you like the looks of it better, then feel free to do it, but it is not making your code perform any different. Everyone reads and can follow code differently, it used to be that making your code unreadable to others was job security, but today this is not the case. With coding standard, with other best practices that an entire team might use, you need to follow what is expected. DoD standards at one time said that every line of code needed to be commented. EVERY line of code. Not sure what they say now. But use whatever coding practice/standard that you and your team agree on and that make it readable for others. Being tricky is not just worth it today for many projects.
I find arguments like this completely pointless. Ofc the code will always be optimised on compilation, that doesn’t mean we can’t make the wait it looks better to make reading and working with it simpler.
It’s the same as using “break” in “switch” “case” as he enters a call to a function with the “if” statement. The readability issue is simply due to your being accustomed to the use of “else” in main code.
@@mikelang4853 Yes, but with switch statements we know beforehand that we will only enter one of those paths, and else statements promise the same thing, but he's breaking the prosumption that those two if statements could both be executed at the same time, and if he doesn't want that then why isn't he just using the else statement which specifically serves this purpose.
@@crash1998100 you are free to code how you want and so is he. As long as it works as you intend, it’s just personal choices and how you view it. He’s just sharing another way that it’s possible to write code and how he is doing it now. When a function is called and once inside it, the expectation is that you do something and exit once done. There could be several “if” statements each with “return” statements if it’s true. *obviously the below is not real code* Function stoplight (){ If red, {x=stop, return } If green {x=go, return} If yellow {x= go really fast, return} } As long as the coder knows that there can only be one of the three choices, once the voice has been evaluated as true, the other choices don’t matter for that function call.
You could change the RecipeBook class into a generic IRecipe interface and have BeerRecipe class and JuiceRecipe class implement the interface. So your bartender would just have a List. The IRecipe method would just have a boolean return that signals the success/failure of the method. The bartender would loop through its recipes and attempt to make drinks with the input keywords, and stop the loop if a success is returned. It would make adding new DrinkRecipes easier and expandable for wierd Recipe edge cases as each Drink is its own class.
yeah. I'm surprised it isn't a common knowledge for many programmers, in fact none knew about this in my company when I first joined. I had to teach all the developers about writing clean code and so on. Partly I blame the education system in uni that doesn't teach students about simple things as best practice and effective code writing. Me being a self taught programmer learned these through open source projects and articles, I encourage people do the same
It's expected for guard clauses to throw exceptions instead of returning an error or nothing. For example, returning an error or just returning won't work in methods with a return type. Then again this is specific to typed returns in languages that have them.
@@AyushkaPartohap not entirely true. But I get what you are trying to say. Guard clause is actually based on the return early rule, so it can also return result early not just throw exception early. Another principle guard clause follows is to reduce scope of variable, or number of scope
@@shikyokira3065 First of all, I completely understand where you are coming from. Everybody probably 'wish' they knew this from uni. It is however not necessarily the uni's task to teach you things like this. Learning things like this is something that comes with practice and even then it should not be learned as "truth" or the "Best way to do something". I think it is very much more valuable to learn it by 'seeing' and experiencing yourself that 'this' or 'that' is a good approach. So what you say that you teached other developers this way of thinking about it, is something SO VALUABLE and you definitely should not stop doing. Maybe someone else will teach you a thing or two and then you would also hope Uni taught you that, but they didnt.... I guess my point is that you cannot teach "EVERYTHING" in uni and some things you learn by experience. In 1 year you will maybe think back about what you wrote and the outcome 9/10 times is probably: "I would write this better now". That in my opinion is and should be completely fine, even though you wish you would know beforehand :) That is what learning and progressing really looks like.
@@bl1tz229 um.. you got a point there. And certainly we can't learn everything from uni. I'm just highlighting the importance of it, and since uni is kinda the process we normally go through to gear ourselves up, missing this out feels like missing out 1 of the important tools that we very much need when gearing up. And i totally agree with you that learning it by 'seeing' it and experience it, is valuable. Just like in uni, we normally have to go through research papers done by other researchers. Same should be done for "programmers", go through open source projects, and obviously must at least has a standard, not a "cmd snake game" kind of project... you get what i mean
Nice and very helpful video! I’ve been doing this unconsciously because I’ve found out the hard way that the “pyramid of doom” (multiple levels of nested indentations) isn’t the way to go. It’s good to be aware of what one’s doing so this video really helped me gaining awareness of these patterns. One thing that I’d have done differently is right around 9:55 you return in the “happy flow” and fall through to the fail state (if input >= 18 return;). Personally, I’d return in the fail state and always end up in the happy flow at the end of the method (guard, exit early and follow the expected result - kinda like the guard-statement encourages in Swift). Thanks for posing these video’s, they’re a great learning tool!
This is why in "interpreted" languages (I know... it's compiled to bytecode, but more or less, that means it's still interpreted) profiling is hard. If you did this in C++, you could possibly pay performance penalties, especially if you return from a function within the if statement (which you should, compiler optimizations can do a lot of great stuff). But I certainly would not know what outcome this would have on performance in interpreted languages. But so much in this video, is a direct result of Java-ish languages design. So now you are spending time ameliorating all the issues with that design, by doing things, which makes the code less clear.
Performance penalties? Dude its 2021 the RAM is cheap. Processors are way faster then they have been 20 years ago. HDD are cheap. C++ brings a lot of problems as: Slow codding(you have to get rid of the used memory by your own) in language as C# you have Garbage collector, Some times it will take you 1 hour to make something to work properly. In C# as Example that would work with 1 NuGet Package and 1 Line of code. I prefer to take more time in code logic at it self then in so trivial thinks as managing memory usage. Not to mention the fact that if you bring C++ code from Windows to Linux/Mac etc.. it will not work. If u code it under 64 bits and get it on 32bit machine ...it will not work. And this is HUGE minus. The era of C++ is near over.
@@FalioV I don't think you understand how memory works. You don't have to "get rid of the memory", that's the whole point of RAII, so your point about slow coding is objectively false, life times handle that for the user. Even data created on the heap, wrapped in unique pointers is handled automatically. As far as when it comes to package management? Using CMake you can now add whatever libraries you want, that lives on github, and it will automatically fetch those for you. But we can even take it one step further, and compare it to Rust, which has its cargo management build tools, making it even easier and better than C# since the project management is centralized and the overview much more compact than the project configuration of a C# project. But let's get back to the point of memory. RAM is cheap - but that was never what made a program slow. Not using the cache coherently is what makes programs slow, now tell me, how good is the cache utilized in interpreted languages? Not too great. Python does it well enough, because it has been incredibly optimised for data science, but general use which you and I are talking about? Not great. And lastly your point about cross platform, C# is the least cross platform friendly language out of all languages. It's worse than Java, exponentially much more worse than C, C++, Rust, Python. So that argument is not correct either. The 64-bit vs 32-bit is a non sequitur. The standards have well defined types that says "an int on a 32 bit platform is 32 bits, and int on 64 bit is 64 bits". So I don't think you understand cross platform thinking quite right, either.
Uhm, no, it is actually the exact opposite - profiling in C# is way more easy to do than in C++. In C++ you need to do a lot of work to prepare your code for effective profiling. In C# it works out of the box. Attach a profiler, study the result, that's it. But anyhow, what you are writing sounds more like you want to do premature "in your head" optimization. Don't do this. Compiler optimizations can not just do a lot of great stuff, they do a lot of stuff based on complex rules you could never predict in your head. Experience shows that any C++-Programmer who does one hundred changes to his code for performance reasons will have ~33 changes that do not change anything at all, ~33 that reduce performance, and ~33 that increase it, and pretty much all hundred of them reduce the code quality. Performance optimization should only ever be done using an actual profiler on the actual running application, running with actual real world data/inputs. And on this front, interpreted languages are way better equipped than native languages.
Guard clauses can also become hard to read, too, and they become order dependant. It means someone has to make sure when they add a new guard it comes i the right place/order and this isnt always clear
Loved the refactoring section! So many dev videos on UA-cam only show rehearsed videos, so it's nice to see the real thought processes as you went along.
I was expecting an early return here eventually. I think that it's a bit of a tradeoff. Early returns can also be pretty hard to work with, especially in long methods. At my work early returns are banned as per our coding standards.
9:55 the “not old enough” is an error condition if you ask me. It makes more sense if you code it as the if condition and fall through to “serve beer”.
@@digitig Yet safety critical libraries like OpenSSL actually use goto. I assure you the people who made that knew what they were doing. It's also the best way to get out of a nested loop or sometimes even switch. If you don't know how to use it, then by all means don't use it. Dismissing every case where it's being used is ill advised however.
What you describe is basically the same reason why we very rarely use the goto statement. We can do everything with gotos and ifs. But the codes becomes very hard to read. Instead, we use control flow mechanisms which are more specialised, thus less powerful, but also are easier to read (and also make it easier for the compiler to optimise). Concerning the if statement, in nearly all cases, its only use is for "easy exception management". I would write your HandleBearAgeCheck as follows. private void ServeBeer(int age){ if(age
I don't agree. The code he creates by using multiple returns is more like code having gotos than the code before. If you have functions, modules, or logical units that have one entry point and one exit point that is code that does not have the "goto problem" of not having a simple flow of control. This code without else statements has multiple functions that have multiple exits. This code is harder to maintain exactly for the same reason gotos make code hard to maintain.
Very subjective topic but single exit is usually assosiated with nesting with leads to less readable code for me. Also by using multiple exist points I am able to do less work in the method, exit early and have a clearer request lifetime story. This is all very subjective but having seen both in big and small services, multiple exit point work for me better.
Don't exit directly from a triple-nested loop. This makes your code a lot harder to read and to reason about. Exiting after a top-level if statement actually improves readability because (as the video beautifully shows) it gets rid of the nesting.
At 10:00, I was bothered that you mixed two different approaches. ServeBeer handles the sad path first, then the happy path, but HandleBeerAgeCheck does the opposite. I like to handle the sad path first, because it's usually the shortest and it just gets in the way of what really matters, the happy path. Anyway, I know you improvised and all, and I bet you'd normally watch out for such details in a real-world scenario.
As a novice, the part that boggled my mind was how simply you automated the string at 15:50. I'd have spent half an hour figuring out how to manually create a comma separated list from my keys without an extra comma at the end or something.
The chosen example is such poor code that it hurts the argument. And you pretty much acknowledge this. Why not just go into open source projects, search for an else and show the benefits with not using them? If this is a good rule to follow then it won't be a long search. I don't like to extract methods like this at all. In languages without local functions you've now got to consider if the code is reused somewhere and any change to the code must be careful to make sure that you aren't using anywhere else. You don't know the state changes of this call either. Everything is hidden. The code before was ugly but it was at least clear in what it does. Now we have no way to get an overview. An overview can be hurt by having too many lines of code, there functions become more appetizing. But this wasn't a case like that. Control flow is the last thing I ever consider making a function for because it usually is very context dependent. I don't see the benefit of hiding that context through function calls. In this trivial example it's still easy to read of course. Especially since we already know the entire purpose of the code. But when reading for the first time that's unlikely. The readability gains in the main method are mostly from having given the different branches names. We can think of a million ways to do this that don't have any side effects. The simplest I can think of is a comment labeling each case.
I think the advantage of trying not to use else is not removing the else itself (although I think this is an added value), it is that in order to do it you need to write good code in the first place. So the rule should be: If you find yourself writing an else statement, try to remove it, if you cant you should probably refactor your method.
Agree with that clean code rules. The if part with return statement is also called "early exit" because you don't need to proceed to the next statement even it has nothing to do more after it. Adding to your methods of replacing your if/else/switch statements, we can use open/close principles + attaching interface to your drink classes Ex: your IDrink has a properties and methods - DrinkName (juice,beer, wine etc) -Serve (your console.writeline of "Here's your {drinkName} and Sorry but not old enough to drink") -GetAge(int age) //Example: drinkInput=Beer, ageInput=16 IDrink drink=Drink.GetDrinkByDrinkName(drinkInput); //any class that hsa IDrink methods and props. Is it beer or juice? drink.GetAge(ageInput); drink.Serve();
Yeap you are 100% right, I've actually addressed it in the pinned comment. I didn't wanna spend more time refactoring but the irony is that people seem to have really liked the refacotirng part so I might make a refactoring dedicated video.
@@tuberklz kinda, but it's much more flexible, it's like a switch which has the full range of if functtionality available, also because it's Kotlin, you don't have to worry as much for unexpected NPE being thrown.
There are two different things going on here. One is replacing branches with dynamic dispatch. This is important for extensibility. It’s key to many patterns such as the strategy pattern, the abstract factory pattern, and others. It’s generally good object-oriented design, especially when more cases are foreseeable changes, and doubly so when the same case analysis has to be made in two or more places in the code. The other thing is trading an else for a return returns. To me this is replacing an “else” with something worse: anomalous control flow. Some general “rules” I follow (which have exceptions of course) (a) to have zero returns in void methods and one return in methods with results and (b) to avoid assigning to any local variable more than once in its lifetime unless it’s carrying information between loop iterations a loop. These rules mean that almost every “if” has an “else”. An “if” without an “else” is sign that there is something odd going on. Another way to look at is that every command has a job to do. In the “then” part of an “if” command does the “if” command’s job in the case where the guard is true and the “else” branch does the job of the “if” command in the case where the guard is false. It’s rare that the job can be done by doing nothing. When that happens I’ll sometimes put “else {/*do nothing*/}”.
Big no-no for me on this approach. Having alternative exit points for your function seems much worse to me than having an else statement. I'm happy to remove an else path when I can and make sense but not when I replace it with something uglier, like a return statement hidden inside the if body. If your language supports guard-else I would use that and put them at the beginning of the function, as a precondition validation step, otherwise I much rather prefer to keep the else. Also, some refactoring to remove excessive nesting is welcome, but having strict rules is counter-productive. In medio stat virtus.
In my opinion - the author missed the main reason why "else" is faulty. And that is the case that the logic that triggers the { else } section changes when editing parts of code that are not the { else } section. . The else in these cases has no explicit readable code that would state that the case for not having a drink to service is because the drink requested is not one of the available ones. Readers of code with a bunch of else-ifelse, have to manually combine in their brain all the conditions and logically come to the conclusion what are the cases in which the "else" would be triggered. . I would remove the "else" via explicit coding . bool unavailableDrink = listOfAvailableDrinks.Contains(requestedDrink) == false; else if( unavailableDrink) { //logic whoes trigger complexity will not escalate as more code and cases are added }
imo the ternary expression is far more readable than having an if block followed by a stranded line of code. With the ternary statement, it's you don't have the repeated output template, and it's clear that you're just toggling the content based on a check. It's just so much cleaner!
I think the plethora of functions created to keep the indentation level to 1 will make it hell to read the code as you’ll constantly have to jump from one function to another. In complex code this will also potentially slow down execution (function calls aren’t free unless the compiler decides to inline them) and there’s an increased risk of a stack overflow (although it needs to be said that if you run into that when there’s no bug in your code (e.g. endless recursion) your code is ripe for refactoring anyway).
This video made me realize I too don't use "else" anymore! It happened totally unintentionnaly. I just applied usual recommended refactoring (and often just followed Resharper suggestions) which led me here.
This largely comes down to personal preference. For me, it is way harder to have a single exit point in my methods. I much prefer multiple exit points because they are very clear in terms of where the method's "story" starts and ends. Code that uses single exit point is also really hard to read for me and usually involve heavy nesting due to that, which I don't find good to my eye. Again, all very subjective.
@@nickchapsas Exit point must be one line return of already prepared result, no deep nesting, and you have mostly readable and understandable method you can have, it just don't need comments, it's self explained.
@@henrivanwesemael Thank you so much! The beauty of programming is that there isn't a single way to do it right. We are all "right" and "wrong" to someone
I have to admit, I was extremely skeptical about this one. I was pleasantly surprised. Obviously not applicable in every single case. The refactoring thought process is really important. Especially for novice programmers.
This is a really good example of how following rules blindly can completely destroy the readability of your code. You started with easy to read code and turned it into an unreadable mess ;D
Completely agree with you. It's much easier to figure out what is going on in one function which is 20 lines of code than 20 functions 1 line of code each.
@@DenysenkoIvan I don't think that anyone advocated for 20 functions with 1 line of code each. It's all about having each function doing one thing only, no matter how many lines that is. It could very much be 20 lines.
Interesting approach, with javascript as my main language i dont really have many problem with the other things mentioned in these rules. I think they make sense to some degree, but with the ability to use ternary operators for simple single line if/else statements and the ability to use inline arrow functions to aviod unecessary indentations, I feel, that these rules are very easy to follow. for preconditions, like an age check or checking if we found db entries, a negative condition that uses a return statement or throws an error should be the default. Whatever rules you follow as a dev, I think the main thing to think about is, that the code we write in our highlevel programming languages should in most cases be optimized for readability and after that come things like speed and and memory usage (depends on application tough). Code thats hard to read is hard to maintain and will only get worse over time and eventually needs a large refactoring. Thanks nick for teaching the world how to write clean code
Functional safety coding (in C, C++) standards and guidelines dictate that you need to demonstrate that you have handled EVERY eventuality in your code. That includes using the DEFAULT keyword in SWITCH statements and ELSE in every IF. If there is no body to the ELSE, then it's still included in the code with empty braces after it (and a comment why you don't need to do anything!). This is to show that when you do a code review, that EVERY eventuality has been considered, even if there is no requirement to handle it. the choice as to use IF-ELSE or a SWITCH-CASE statement depends on the number of checks. Performance wise, a switch statement is only more efficient when you reach a certain number of check conditions. Returning mid-function is also frowned upon as you should always ensure the function exits in a single point so that you have handled other things such as 'clean-up' gracefully. I have seen code in things like memory allocation functions that exit (using a mid-function return) on failures and bypassing the free'ing of the memory that was allocated. Might be easy to read, but can cause major issues with deallocating object, clean up, etc. Hence an exception handler TRY-CATCH has a FINALLY keyword! Ref: MISRA C/C++
It makes no sense. When you have some function returning something and using if/return aproach, you have some last return satement with is forced by language, and this is your 'default' choise. Switch or if/else neither guarantees you checked all possible results, they are just equal language possibilities, if somebody extended some enum in your codebase, neiher if or switch can guarantee that it will not build until you handle this new case. But there some advanced approaches to handle case if you want your library refuse to build if you missed something unhandled, and I can share it with you.
@@iGexogen That's what the processes of code reviews and unit testing are for! If you extend an enum, it will be caught by the default statement in a switch (which should output to debug that an unknown case has not been handled) or even throw an exception in the default handler. Either way, you change the behaviour of the code, you should FULLY TEST IT ... Unit Testing and Regression Testing!
@@simonbaxter8001 Output to console makes no sense at all, you will not notice that your code is broken only after you will get report from real users on production. Only unit tests that enumerate all possiblilities can help, but there other cases, for example when you must handle some action for all subclasses of some base class, and you cannot predict all subclasses in cases of "plugins" system or something. No automation can handle this cases, only good architecture.
@@iGexogen You will notice in the console during debugging and development testing! If you need to handle 'plugin' type scenarios, that default handler should return an error from the function (i.e. this case is not supported), or as I say an exception. The developer of the plugin has the responsibility to test their code works with your framework and check return codes. Good Architecture and documentation is the key, but testing is a big key too. Automation and a good test framework should be part of your architecture design, not an after thought. That is where a lot of young/new coders go wrong these days. Look at the Microsoft c# libraries, they are full of error handling and passed in parameter validation. That is also key too. Make your framework scalable and robust and it will not let you down!!! My pet hate is people not testing for 'null' or NULL pointers before using them! Testing is key. Don't let your customers/end users be your beta testers ... it's not good for business and your product/game. They should never see bugs if you have done your job right. I write code for automotive ECU's (Engine, Brake, Transmission) ... I can't afford for my customers to find 'bugs'!
@@simonbaxter8001 So why you oppose this things? Good architecture and intensive testing only complement each other, and I completely agree that in some project types tests can be very vital, and most important subsystems must be doubled and maybe even more. But I have another example with business applications where errors are pretty tolerable. I have legacy document flow system in my organization, and it works somehow, and if I could ever cover it with tests, I will never make them all pass, no way! I could fix in one place, then break in other, and there is no way to fix it due to it architecture and way it evolved (last is even more important), but it is still used by 50K people, and will be used for next couple of years.
Addressing a few frequent asked questions below:
Q: Why did I say that I don't recomment an single exit point approach.
A: Single exit point approaches usually include nesting which is another thing I am addressing in this video. Sure you can do it without nesting but at that point your code is needlessly complicated and not as readable (subjective). Multiple exit points tell the story of the method's flow way better in my opinion and have the side effect of also exiting earlier.
Even the StackOverflow question on the matter, has been closed and marked as “opinion-based”: stackoverflow.com/questions/4838828/why-should-a-function-have-only-one-exit-point
Q: Why are I doing a positive if check in 10:10
A: Totally missed that. It should have been an "if(age < 18)" check instead. Thanks for bringing it up.
Q: The Open-Close violation wasn’t completely fixed since it was now moved to the class level instead of the method level
A: This is correct and I mentioned it in the video as well. I didn’t want to spend more time in the video for the second part since it was an afterthought to begin with. That being said, if I was to continue I would implement a strategy pattern and have the dictionary automatically created by some convention. That way we completely eliminated the OC violation
IMO using the return instead of else is only safe if the method is small (only a few lines) and can be seen in full with very little analysis. Your cases fit this and so it works but that would be my note.
If you use the first rule you in essence solve that but for people not following that rule strictly they can get into trouble with forgetting all of the wild ways they exit lol.
@@fourlegsgoodtwolegsbad Methods should be kept small. My rule thumb is that if I need to scroll to see the whole method then the method is too long (normalized font sizes)
@@nickchapsas I of course agree. However, real life demonstrates that many do not do this lol.
If you have nesting problems, it's likely the culprit is mixing levels of abstraction. Not single exit.
@@fourlegsgoodtwolegsbad Just extract methods. Whenever I open a file for my work and it has functions that are too big I just start extracting them and cleaning up the code before I even start on my actual changes. Sometimes that also means I have to write additional tests. But it really helps learning that part of the code and it can even expose bugs.
If a superior complains about me taking that time I tell them that I'm hired for improving and maintaining the code, and not just "data entry".
You have started down the path of enlightenment, at the end of which is the pristine beauty of the goto statement.
yikes
Sometimes i wish i had a goto statement in python, but i know many people hate it.
@@Theunitedlowshater goto is horrible for performance...
@@moralesvazquezmiguelangel6546
It's a jmp like any other. Can you elaborate?
Please no
The problem with this approach, that you will fragment even a really simple business logic into so small pieces, that it became harder to understand. Also the default case in a switch can be interpreted as an else :D
@@IIARROWS Agree to both comments , for me (maybe because I'm a little bit dislexic) the else is good visually to be there, is much more easy to understand is two different blocks of code, without the else if you don't read the return, you think it's the same flow. Some time ago we had a bug in the company because a return in the middle, and one line was only 3 lines lower, but visually it's much harder to see this line is in an incorrect block if you have the return (at least for my case)
And for me, when you have a method with one line, something smells... usually this kind of code means that any newbie to the code requires 6 or 7 jumps between functions to really understand what is happening.
@@jaumesinglavalls5486 totally agree.
for me the video looks like "hey, this is how you can write code without using keyword 'else'". But I dont see any explained profit that it brings. Nick is doing some refactoring - rewriting 'if' tree into switch case, also he extracts logical pieces of code into corresponding methods, which is totally ok. But when he started replacing 'if-else' by 'if return', I felt like he was trying to manipulate us, using "three yeses" trick.
@@Vol4ica Exactly, the if else or if return, is the same, only that some part of code is not idented (In my opinion, making it worst to read) don't forget that identations in most of the languages are here only to help us, to represent visually something the code is doing.
In my opinion the rule (at least is what I've got when writting) try to avoid the if itself, as joke usually I say "I am alergic to conditions"
@@jaumesinglavalls5486 i am also dyslexic and also find hard to follow when there is a return, so maybe it is indeed related
Just write in binary
Next video: Why I don't use my hands for typing
Actually I would love to see a video where he goes through all his most used shortcuts and plugins he uses.
😂😂😂
I don't use "else" that much either but I don't think that forcing yourself to never use it is a good idea. I know you can always add a return early but I only use it if something has gone wrong (like them being underage in the video) and the code between the if and the return is very short. Also, I think the "intended" code should end at the last line of the method because if you are reading the code backwards (which I do when I'm trying to figure out where a value comes from) it makes it easier than having to go to the middle of a method.
I am a beginner, learning C++ & Python, and from my perspective it does seem like a good exercise for learning to become more efficient, but not good as a rule that is written in stone.
Sort of like when learning to play chess, a student may start a game with less than every piece, in order to more acutely learn how to take advantage of certain aspects of the game, but would never play a tournament game by starting out with missing pieces.
There’s a developer in our office who consistently puts else clauses back in while doing code reviews.
Even when I write code....
if (condition)
{
throw new Exception(...);
}
He adds the else back in for the following code and does this down multiple levels of nesting.
Same with return statements.
Someone once taught him functions should only have a single exit point, and he’s carried that over to C# methods. When question he say “makes it more readable to me”. 😜
I agree with@@Inertia888, branchless programming, not using the else keyword and other techniques are just different ways to do the same thing. Sometimes they can be better than the traditional way, and sometimes they will not. As well, overusing any strategy can always make things messy, as you pointed out when reading the code backwards
@@stuartmcconnachie Does your office not have some kind of coding standards in place? Especially if you have code reviews, they should check by the ruleset in place, not personal opinion (apart from the fact that the reviewer shouldn't be the one doing changes, that's the author's responsibility; but that's just what I've learned from software quality courses at university, so I guess "the real world" looks different).
@@VanLouProductions That's one of the few principles they teach you that actually hold true, at least in well-funtioning teams. What they don't teach you is how to get into that place. Having a team with shared principles and values and a culture of mutual trust and respect and candid but constructive critcism is a rare thing. It took me most of my carreer to get somewhat close to that ideal, and it's still something we need to fight for every day.
First rule of programming: Don't stick too hard to the rules! ;o)
Also known as critical thinking ;D
I would say:
Understand rules - not just follow
Agree. The tricky part is to know when to use which rule.
@@nickchapsas WORD!
But the recipebook also violated open-closed, as every time you want to add new drink, you have to modify it
Sorry if I missed something in the vid - as I was going through it in 1.75 speed. Some examples that you described are called guard cases - and I don't recall you mentioning it so I think it's something useful to look into. For example not proceeding to drinking if under 18 is potentially a good example for guard case - because we quit from otherwise possibly a long process.
Still IMHO there are a lot situations where 'else' improves readability. Recently over past year/two or so I noticed that I'm constantly being asked by fellow devs to remove else in code like:
f() {
if( ... ) {
return A()
} else {
return B()
}
}
I wonder if there is some sort of fad going about this whole else thing as it's not first time I see people talking on how to write code without 'else' and I haven't seen this idea going around as much just couple years ago. Argument that I sometimes hear is that else is redundant here. But If I go about removing all redundant things in code then I would end up with golfed code (and spaces btw - are redundant too). From readability point of view to me 'else' is not redundant because it helps keep symmetry between A() and B() keeping it on same level of indentation - to me it's slightly easier to read A and B simultaneously with single glance
xxx {
xxx A()
}
xxx B()
vs
xxx {
xxx A()
} xxx {
xxx B()
}
It also helps to maintain intent that neither B() or A() is more special in any way (unlike in guard case situation where intent is to show that there is no process symmetry and there is special cases).
I believe not using `else` is an optimisation. I remember reading about it but CPUs can have trouble with branching code, they'll make a best guess at which branch it should read ahead into, but if it makes a mistake it has to rebuild that code path again which is slow (obviously)
By not using the `else` keyword we remove branches from our code in places they aren't necessary.
Generally speaking if you're writing a simple tool or DB client interface for example, writing code with less branches isn't really any use because you're not worrying about raw performance in those applications, but you might on a server running checks against certain criteria in the DB. It is also important in video games that become very complex logically and other very low-level software like driver interfaces and such.
@@Rexhunterj branching happens in either case either if() {return} or if(){}else{}
Unless one deals with very low level high performance code this usually won't be anything that will matter as the code we write ends up modified in multiple levels until it reaches CPU anyway.
@@Rexhunterj You should never code because compiler will do this or that, or optimize compiled code. If the compiler doesn't manage what it's designed for then you should switch to another language if you need the extra perf.
You don't need to write that long just to explain what is opinion based approach. 😜
I was about to write nearly the same thing, but thank you for saying it first.
I won’t try to pretend that leaving out the _else_ statement is philistine heathenry if you don’t pretend like it’s self-evidently preferable. They’re both perfectly adequate approaches that achieve the same result. All other things being equal (and in this case I think they really are equal: claims of optimization through _else_ omission ought to be presented with some evidence) I just happen to value aesthetics far more than I do brevity.
It always irks me though, that C# programmers seem to be always finding ways to take up more and more vertical space in the code... Every time I look at C# code I can only see like 5-10 lines of actual code in the entire screen.
You like spaghetti no
Put all the code in a single line, heck yeah!
Funny you say that as I am a C# programmer and I constantly think about how to use a few lines of code as possible. Idk if that makes me a good programmer but it certainly feels good having few lines of code
I think it largely depends on WHAT you're doing with that code. If you're conserving space just for the sake of conserving space, I'd rather just have the extra lines. Allman is very readable and good code should be easy to follow what's going on. Not like you're going to have to re-read every line of code multiple times every day just to understand what the code does again.
Why C# particularly? I see this everywhere. I think people think they code is more "clean" just because they spread it out, plus they can be proud how many lines they wrote. The thing is, if you had mess in your room and spread it evenly in a stadium, it may look clean there, but it's still the same mess. Personally, if I would refactor this I would write something like this:
private void HandleBeerAgeCheck(int age)
=> _outputProvider(age >= 18
? "Here you go! Cold beer."
: "Sorry... blabla");
I agree with almost all of this, except for one key aspect: The recipe shouldn't be an "active" object, i.e. it shouldn't have a "make" method, and in particular it shouldn't take an input/output stream. I think the design becomes much better if the recipe is just a data object that is passed around, and the bartender object is responsible for the "production" of the drink by reading the recipe. This also entails that the recipe itself cannot (and should not) perform an age check. Instead, each recipe should have a "containsAlcohol" flag, and the bartender should check this flag prior to making it, and acting upon it accordingly. This also gets rid of the nasty lambdas in the recipes.
100% agree with this. I believe it's called the single responsibility principle. Also I really cringe at the dictionary of Action objects, incurring unnecessary allocations and invocation time. I feel like in this example Action objects were used as if they're as cheap as a function pointer, which they're not.
TL;DR: Action, yuk!
But but, I want the glass to ask me if I'm of legal age, not the bartender!
Jokes aside, it can be seen that understanding exactly who should do what, the responsability of each object, it's quite hard.
@@jason_v12345 that's a good solution... that is, until you have multiple "features" of a drink you need to check for. You could use multiple marker interfaces but it gets messy.
@@jason_v12345 I am absolutely in favor of OOP. But subtyping won't help you much here. You can have a "ContainsAlcohol" interface with no members to denote that. What's next? "ContainsMilk", "IsVegan", etc... zero-member interfaces are an anti-pattern as well.
Nick is probably aware of single responsibility and he chose to have his code be the way it is because he thought it would be more simple. See the wall of text at 14:17
I think returning early and then removing the else makes the code less readable and also extending it later on, you‘ll have to fully understand it first.
In this example he swapped the return early's around (see pinned comment), introducing confusion. However good practice is to return early on invalid input and then have your validated input be used at the end of the method. This is the guidance that most top developers give, and is used all throughout the modern .NET code base.
@@ZintomV1 Yeah that's what I do and then use comments to explain exactly what you're doing ie. // Checking for invalid ? , where ? is any variable
Yes, if your check is valid/invalid then an immediate return on invalid makes sense. Anything else adds confusion because "else" tells you something about the intended logic of the code that isn't obvious when removed. But then I code in a very different style that may not be readable to others...
This, I prefer keeping an "if ; else" flow because it's immediately clear from the structure that the two sections are mutually exclusive. If you have any sort of code folding going on, then you wouldn't see the return until you examine the contents and realize that section never goes past the block.
@@platinummyrr but why would you have code folding inside a function? If you keep your functions fit a single screen it's not a problem.
Okay, some comments on this. Moving code which is never reused to a function is a nightmare to read. Not only do you obfuscate the code by moving it somewhere else, but you also make it appear as if it's going to be reused, which is not the case. It also leaves you with naming, and when code bases grow big, having a plethora of obscurely named functions is not very good from a tech debt perspective. You also create a longer call stack, which is making it harder to debug. And, not to forget, even if function calls are fast, they do have a cost!
Returning early is great for when you have simple conditionals or if you have a set of conditions which are supposed to terminate the function early, but if you have a scenario where it's not a binary condition, i.e. you will have an if-elseif chain, then how would you solve that? The returning early scenario is usually used or thought of as a function having a fast path and a long path, meaning you can, just like you say, return early if a function performs its task or has to abort. But having something run after the conditionals may also get forgotten over time and you end up running that code just because you didn't return in one of your conditionals. Isn't it better to be more explicit about what code you run? With that said, I do agree that big scope chains are annoying to read, especially when closing them, but I still don't agree that returning early is good a universal solution conditionals.
Talking about code bloat, is it really less bloated to create a function for a conditional? Look at how many lines of code you have in the end, I am fairly sure you started at 50 and ended with like 70 in the first file and something over 30 in the new, so you doubled the amount of text for basically the same functionality save for a slightly more useful way to add more drinks. I could be wrong but this whole RecipeBook class could be reduced to just using a static map with a String -> lambda mapping, which would allow you to move the drink initialization to the same place without having to introduce an extra class which is used only once, and it would incur less coupling in the code.
I hear these mantras and these manifestos, but I hear very little explanation or defense of them. Why do you only want one indentation per method? What's the logic behind it? What's are the benefits? I'd like to break down why these rules are useful and actually get a good argument for why one would want to use them. What I usually see is very typical OOP, abstract everything until you have made everything reusable, and then in the end, never reuse anything.
First point with moving code, highly disagree since you should be able to name small functions well, then i can just debug that single function that's giving an issue and the name should be able to be descriptive enough that the name gives away what it does. If it does more than one thing, then you've failed, and that's where naming issues come in.
Second point, you're just not writing else since you exit early, so you're bringing in an issue you'd have by not correctly formatting and exiting early. For multiple if else if you have a switch statement with a default case, which is your else.
More or less code doesn't really matter that much within reason, it's more important that it's easy to read and maintain. Making every function do one task means it's easy to debug later down the line, I know, I'm currently working on a bunch of spaghetti code for nearly 2 years now, and we're finally at the point where it's okay to introduce juniors to the code base since they don't need an extreme background in the flow etc. to be able to understand what's going on, they can take each function at a time to understand what it does, step by step.
And in regards to his examples, all the stuff is simple, imagine it was more complex stuff, decisions with routing, calculating financial interest based on a multitude of factors, etc., that would all be more complex things, these examples or to explain what, not a best-case use of it. I like SRP, if it isn't properly followed in a large code base, issues crop up pretty fast.
Thanks a lot, now I know about static maps.
@@Masterrunescapeer If you make tons of small functions you mismanage the purpose of a function. It’s meant to be used for reusable code, and making a bunch of small functions that just run in sequence is of no use when all that code could live in a single function. I’ve seen so much code where there are hundreds of functions being run in sequence and only in that sequence. That just adds a constant execution cost to your application (having to push arguments to stack and the return instruction pointer, as well as popping the return instruction pointer) and obfuscates the code for absolutely zero gain :).
@@gustav1416 in regards to issues with overhead, your compiler is smart enough that that should very rarely ever be an issue.
It's easier to write multiple encapsulating functions that go step by step than one giant function, e.g. Flow of sending a message you'd have a controller that orchestrates accepting it, then process into usable, then send to task, then convert result to correct format, then return.
In regards to too many small functions, doesn't matter if function name says exactly what it does, I'm not talking about taking a one liner and making a function out of it, I'm taking about a few lines that are repeated, e.g. Take header from a cookie and do comparison to check what user type and then check is valid for call or something, that's a recent five liner I split off and made generic. Or if it's a business rule, the function on its own is maybe mathematical, say function is area of a circle or something, you'd split that out so you do calcCircleArea, even though one liner so name is there for easy understanding. And no, would not do it with circle area calc, I'm saying rather business rule or other more complex formula. Then removes need of comment, comments get outdated/lost, and quick and easy to debug as an isolated function.
@@Masterrunescapeer If you’re talking about JavaScript you also have the increasing cost of the code download if you keep adding unnecessary text :). Also, don’t rely on the compiler to optimize, you always want to aim for your code to be as optimized as it can by you. The compiler might especially find it difficult to optimize calls between libraries as optimization is most efficiently and most simply done within a compilation unit. Breaking code down to single use functions is of no use at all, all it’s doing is forcing you to split code in arbitrary places and it stops you from moving code around easily without renaming functions and restructuring code. On top of that, you have to navigate the code, jumping back and forth trying to keep the trail of breadcrumbs in your head while you do so, when all of that code could be in a nice neat sequence. I’m not saying code should be repeated at all, I’m just saying I’ve seen code split into functions where the functions are used in only one place, thus making that split a waste of time, effort and at a detriment to readability.
So this is where that kind of extremism comes from.
I mean i totally see the reasoning behind it: simple rules
But simple rules followed this strictly are the exact kind of good intentions that led to certain bureaucracy hellscapes. So i have to kindly ask for your permit A38.
Totally true. Langages have if/else as well as switch because they're performing well for different scenarios. This video should be called : why using if/else in a switch case problem is bad.
Aside from that there are real life cases where if/else is justified.
It is clearly stated multiple times that this is not a strict rule. The advantage of trying not to use else is not removing the else itself (although I think this is an added value), it is that in order to do it you need to write good code in the first place. So the rule should be: If you find yourself writing an else statement, try to remove it, if you cant you should probably refactor your method.
Yep thinking too much will reduce the productivity especially in big project. Also we dont usually have the luxury of time when its a product.
Refactoring stage comes after anyhow.
Although I agree with the video. The example of jumping out of the function in the beer serve check seems to be backwards. It makes more sense to write.
If (age < 18) {
//Error here
Return;
}
//Continue to serve beer here
This way you jump out of the function when there is something wrong instead of assuming the code jumps out of the function when it's right.
The rest of the video seems nice. :)
Actually I sent through the code again to refresh my memory and you're 100% right. It should be a "negative" check instead
Love when the code tutorials have mistakes 🤣🤣🤣
Yeah, I do the same error checking... if (!condition) return; instead of
if (condition) {
// CODE BLOCK
}
It's cleaner alright, but what about the cpu's branch prediction? Isn't there a silent assumption that expression in if statement (most of the time) is more expected to be true (except for null pointer/ref check)? In init functions cleaner approach is fine, but on the critical paths / in loops might cause a lots of cpu pipeline flushes resulting in drop of performance.
@@sprytnychomik you would need to check the compiled code to verify. It is not easy to say if it would cause any performance drop. Modern compilers are very good at optimizing code. In case of doubt you could check the execution time. It really depends on the scenario. In most cases I would assume any small performance loss is negligible. Only in other case where performance matters due to many loops etc I would try to optimize the routine for speed.
There is no need for if, else, loops or anything like that. Real men use assembler and jumps
6:40 the problem with this approach is that many codebases require you to search through methods later on. Seperating one function into four makes that task a lot harder. It works great when you read the code from start to finish, but that's often not feasible in larger projects.
This approach of micro-functions also creates a lot of vertical seperation throughout the file, which can drive related important pieces of code far apart when it would be much handier if they were adjacent.
I’ve been doing developer support for a total of over 6 years now, and lots of tiny functions nested deep through a dependency graph of a lot of objects that exist from who-knows-where (often set up long before you get to that code) in large codebases (either customer’s code, or OS code) especially when debugging release optimized code in assembly (without full source, but hopefully symbols) is a real nuisance. This is especially true and a major combinatorial explosion when there are multiple types of possible subclasses or implementations of interfaces/protocols (depending on language/terminology). You can read all the source code, but with a large enough OO codebase, do you really have an efficient way a priori to know which types of classes will actually be used without verifying all the possible types? Nope, not in a practical sense.
Meanwhile, long procedural functions with relatively short stack depth is relatively easy to identify what’s going on, because you’ll likely have more information at hand to identify which things could actually be called: fewer potential breakpoints you need to set to identify actual flow.
Multiple levels of OO dependency graphs with subtypes of objects is just a more abstract way of representing an arbitrarily complex dependency graph of hairy switch statements in a manner that’s deceptively simple in appearance: both an asset and a liability for understanding the code flow, depending on the time and reason you’re reading it and trying to modify it.
I know it leads to VERY different looking code, but if a function is only ever called in one place, put the code there and use comments.
When I’m debugging, I don’t want twenty function calls to follow if I only needed three. Especially because after I’ve finished reading the nested function, I need to read back up the next twenty functions it was nested inside
Totally agree with you, there is a reason we have comments
Also if you put it in an extra function someone else might come along and call that, even if it was only ment to be used in that one context, and now you suddenly have to worry about backward compatibility when you need to change that function.
That was my concern as well. I would much rather have the code in front of me, rather than having to search for it, particularly when it’s a one off that still fits in two to three lines of code.
@@Madbird95 Yes. John Carmack thinks so too.
Well, you don't always need to read every line of code sometimes and function names can quickly tell you what a segment of code does. If you also write your code in a modular and functional way you can easily test your code too even if you only use that function once.
Agree. By treating "if else" statements instead as "if guard condition do something, return", code is much more readable. The hot path should be easily identifiable.
I was taught 'one point of entry, one point of exit' in 1988. Multiple returns to avoid an ELSE would violate that.
As with most rules, it's useful to know why it's a rule in the first place. While I do think using returns and switches cleans up the code nicely, there are certain cases where using an else is just simpler and cleaner than blindly following those rules. This was pretty informative and presented a good list of coding guidelines and best practices but one shouldn't be afraid to break a few occasionally if they make the code more convoluted than the simple approach.
I would argue that splitting out methods to remove nesting is often just as difficult to read as the nesting, especially for debugging purposes. Having to hop from subroutine to subroutine just to figure out the CONTEXT of how the bug happened is a lot of work. I personally find it a bit easier to debug the nested version, because I can quickly scan up and - for example - figure out exactly what line results in the object 20 lines later being unexpectedly null. Doing that through several levels of subroutine function calls often requires me to re-run the code in debug mode over and over, once for each level of function call. Of course, both arrangements are difficult to read in their own ways, but the REAL reason for the difficulty is that the design is a brute-force decision tree. Your refactoring into a dictionary of method calls is a great way to simplify things: each drink has its own code, so I know where to debug right away.
Sometimes, however, we're stuck with a brute-force decision tree. Your dictionary works because it conceptually models the requirements of a bartender very well: one action per drink. Easy peasy. In real-world coding, we often have a fairly tangled mess of requirements that doesn't parse out so neatly, namely that the requirements are a set of arbitrary rules with all the "if then" logic stated in those rules. In such situations, I find that having the coded version of those requirements closely parallel the literal requirements, no matter the level of nesting, to be the most readable - because you really can't make it more readable than the requirements. You might be able to reduce the requirements into a simpler logical construct, but then everyone after you will have to prove to themselves that the simplification is correct, so while your code might be more readable, it's more difficult to verify that it's correct. Further, if code closely parallels requirements, then it is possible to verify whether the requirements are correct! For example, SME claims your code has a bug, you show SME that the code does exactly what the requirements say, then together you both can determine whether the requirements are wrong or the code is wrong. Yeah, the code is difficult to read because the requirements are difficult to read, but dang, matching the requirements closely saves a lot of work.
The guardian "return" approach you use is good, especially as I find it very easy to debug code that returns a failure quickly, as it is obvious which condition isn't letting the main method proceed. Outside of guardian code, however, having multiple exits makes things difficult to debug, especially with several levels of function calls, because it's almost never clear which call in the chain is the reason for the early exit: the guardian code needs to be at the beginning of the method, not layered throughout.
Then there's the overall question of what makes code more readable. A short essay is readable because the paragraphs organize the ideas in a way that shows how they all relate together in a global sense. Some ideas are more important than others. Some ideas deserve only a sentence fragment, not a full paragraph (their own method), while other ideas deserve to be fleshed out in their own paragraph (or even two). Similarly, code is made readable by making the lines of code clearly denote the relationships of the ideas involved. You determine that it is readable code by READING IT when you are done, and editing it to make it more clear in a global sense, not in a nitpicky "correct grammar" sense. In code, we tend to pay attention to the trees in preference to the forest, because we have to analytically verify each "tree" to make sure it works right, but those who read our code necessarily start at the "forest" level, and we want our forest to be arranged such that they can quickly figure out which tree(s) to look at. There is an art to this, as what is readable to one coder isn't necessarily readable to others, and it takes a while to learn what other coders think of as easy to work with, and what they find difficult to work with. This knowledge cannot be reduced to an analytical set of rules.
I _always_ have this problem, the deeper you have to go within the stack the more you have to rely on your brain to keep track of a given path through your program. Perhaps rules around nesting should relate to subroutines as well (a method can only call another method, and maybe that one can call a method only). Another rule might be that you're not allowed to split a method unless it's used in multiple places, readability be damned, DRY is more important than clean-looking.
I think your main topic isn't the debugging or the context but having to deal with poorly written code.
All your arguments come down to the fact that the code probably sucks and has many side-effects in the subroutines.
If you know why you break it down in individual functions and understand clean code it will be a pleasure for other coders.
With (almost) no side-effects the context is super small at any place in your code and the functions/variables tell you a story instead of showing you a forest. 😉
Smaller code units makes it easier to debug though? If you have a small code you can easily tell or even test if its doing what it's supposed to do.
@@malino24-souls The problem I find with "smaller code units" is that it becomes remarkably difficult to find the specific unit of code that has the bug. Even if I have a stack trace that specifies the right line, the cause of that exception is typically an unexpected null object: there is no bug in the line throwing the exception, the bug is whatever is providing bad arguments, and the bad arguments typically arise from some much earlier call that is also working just fine, but in that particular instance the database or REST service has no data. You might argue that we just need to handle the null case and be done with it. Once in a blue moon, that is the correct way to handle it. Far more often, the business requirement is to figure out why we have bad data in the first place, and at a minimum, my job is to figure out where that bad data is coming from, so someone else can hunt down why it exists on their end.
"Smaller code units" is a good approach when considering only whether the code logic is self-consistent and correct, when the fix needs to be made in the part of the code throwing the exception, when the CONTEXT of the code doesn't matter. It's not so good for cases in which context matters, such as exceptions that arise from unexpected real-world data, because that bad data arises elsewhere. I need to know the overall context in order to have a clue what data was expected. And not just what class or what type, but enough info, for example, to tell whoever manages the data loading which specific id is missing from their data set.
TL;DR - in my experience, most bugs don't arise from a bad line in a specific unit of code, but instead arise from the chaotic and unpredictable interactions of hundreds of perfectly-correct "smaller code units". The faster I can find which specific path through those code units causes the error, the better.
@@uumlau that makes sense to me. Unfortunately I'm not as proficient with real world examples. But what I can say is, that if a code unit is considered correct, it shouldnt be the source of error in said case. So if the unit accepting the passed data isnt verifying (let it be by simple asserts or complex analysis about "correctness" of data) the data, then may as well consider that unit not correct or at least error-prone. Not sure how wrong/correct I am, but that's at least from a logical point of view and with my own programming experience.
I loved seeing the refactoring approach to explaining this. Would definitely watch you refactor more code.
Me too! Refactoring is always insightful
I'd rather use ELSE than have multiple exit points. Having said that, one way I reduce the use of ELSE is when flow isn't affected like setting variables. For example:
x=1
If a=b {
x=2
}
Rather than
if a=b {
x=2
}
else {
x=1
}
But you'd not do that, something like that you'd do `var x = a == b ? 2 : 1;` which is technically also an else, but you're not using the keyword.
Multiple exit points if they are correctly structured as guard statements reduce complexity, since nesting if I have a large trail to go follow to see how to get there.
@@Masterrunescapeer I do like that syntax, ngl.
This seems silly. What if x=bla is a really expensive operation?
@@aussieexpat Exactly, very often such statements will be much more expensive then this example.
I agree with you, love the ternary operator.
i instantly subscribed when he did the outro in the middle of the video for the people who were just interested in the title. a content creator that cares about views time like that will always produce quality.
k
The clean coding aside, the extract method and where you created the switch shocked me... I knew the ide did stuff but I never really looked into it and just did things the hard way for 25 years.
yea, modern IDEs have a lot of capability now!
Hey, Nick! Thank you for another amazing video! As for the second part of the video, it is even better that it was not prepared in advance, because watching you think out loud, analyze the code and make decisions is also very valuable. 👍
I preferred the original method, I don't wanna be scrolling up and down looking for other methods when I'm trying to find where the code flows after I've been debugging for 3 hours
If I was video blogger and decided to make video on this topic, I would call it "Why you should reduce your nesting as much as possible", problem is not with "else" keyword at all, problem is about unneccesary deep nesting. I rarely convert my code into switch statement, but if my method contains more that two nested ifs, I reconsider my architecture.
I highly doubt I would click on that video based on the title. This is both about else and nesting. When nesting was 1 level deep, I still removed the else because it's pointless. Nesting isn't a familiar concept to most people while "else" is.
@@nickchapsas If somebody has one level deep nesting and has elses, "why you do it? " - is the first question you gonna ask.
I definitely agree that removing "else" can make your code look less bloated, thanks for tip.
Also really liked the second part of your video and seeing your thought process, please keep doing it!
"Early return" has its benefits, yet it can also make code harder to read if it becomes longer as there are many places your method may be left instead of just one. Generally I only use else for "final statements", that is code that shall be executed no matter if the if-path or the alternative path has been taken and quite often such statements don't exist, in which case I will also use early return. However, following your sample, I would neither use if-else nor switches, I would actually use objects and dynamic dispatch at runtime, which is even less and even simpler code. The code is split upon more objects, yes, but the code each object is tiny and if you need to add another case, you don't need to touch existing code at all, you just implement a new object.
If you have a situation where you need this kind of if/else or switch constructs, you should just go polymorphic, create an interface, make different implementations, get the implementation and call the method. No more if/else or switch.
I guess you didn’t see the full video :)
@@nickchapsas Busted... :)
Make me feel like UPS drivers that never turn left 😜
I don't use else i use otherwise 😌
You may think they’re all right, until they take shortcuts and sign for your Apple m1 Mac Mini claiming they delivered it, saying you rejected it (even emailing me a picture of “proof of delivery” those assholes, when they tried to force it off on the apartment management, even though I was home the entire day and they never got in visual range of my building), so it gets shipped back to an Apple facility across the country automatically per signed package policy of Apple, and they put the cuss in customer service and leave it to you to get it. Lazy dishonest identity-fraud-committing driver, he and his manager should be fired and then charged and punished for that crap.
A true story, they take “efficiency” to illegal extremes.
They are trying to equalize the offset made by NASCAR 😂😂😂😂
I really disagree with the no-else notion - enough even to actively refactor code away from it.
I have two reasons for it: I can't see all of the returns at a glance and it hides nesting.
The first point comes into play in the phase 1 of reading code: the skimming through. And here it already helps to understand that there are more cases.
And at a glance I can't see whether or not an if block just does additional computing or is a possible exit point for the function. This is an important distinction.
If I use "else" however, I can clearly see that there are multiple exit points depending or not on whether the return is indented or not. Moreover, I can quickly see how many (and where) they are by seeing the elses and ifs that aren't indented at the same level as the rest of the code.
The usual answer to this, that I hear is that methods shouldn't be longer than 5 or 10 lines, and I partially agree, but that's not always possible, sadly.
The second part of hiding nesting is that if you don't use elses, you can easily get dependencies in between the ifs or before the last if and the end, which is essentially nesting, just well hidden.
E.g. the following (nonsensical) function:
NO ELSE:
int getMultipleOf3(int number) {
if (number < 3) {
return 0;
}
if (number == 3) {
return 1;
}
int numberMinus3 = number - 3;
if (numberMinus3 == 3) {
return 2;
}
if (numberMinus3 == 6) {
return 3;
}
return -1; // too much
}
THE SAME CODE WITH ELSE:
int getMultipleOf3(int number) {
if (number < 3) {
return 0;
} else if (number == 3) {
return 1;
} else {
int numberMinus3 = number - 3;
if (numberMinus3 == 3) {
return 2;
} else if (numberMinus3 == 6) {
return 3;
} else {
return -1; // too much
}
}
}
With else I immediatedly see which dependencies are used only for one if and which are necessary for more and can see that the whole block using "numberMinus3" should actually be addressed (e.g. by extracting it into a method). In the first code, when I read the line where "numberMinus3" is define, I don't know where it is used afterwards. The IDE can help with that a bit by highlighting, but it's still not immediately apparent.
P.S.: the approach using else is (part of) structural programming if somebody wants to google that.
Agreed, not using else can hide order of operation dependencies which are normally visible with indentation
I don't think this is an `else` problem, but a `nested statement` problem.
Pff just use a language with pattern matching match/case statements and flatten all your conditions 😂 Condition-heavy code isn't great anyway.
I don't think a hard rule like "only one level of nesting" is a good practice, don't extract methods just to keep low nesting, extract method when its a logical nameable thing. Of course you should strive for little nesting, but I don't stress over it either.
Besides, that extra nesting where he has age >= 18 would go away if he used `else if {` instead of `else { if {`. I mean, I get that examples are hard to keep meaningful yet simple enough to understand and he does call this out when he mentions the SOLID principle. That would largely solve this particular case anyway.
@@guywithknife Oh sure, switch statements are great (I actually have a section of code right now that should be refactored to use it, heh). And you're right about "only one nesting level" being nice, but not critical. I've got nothing against something like:
for...{
if(obj != null && obj.somevalue == checkvalue) {
//do simple thing
instead of
for...{
if(obj == null || obj.somevalue != checkvalue) continue;
but I will do things like
for x...{
for y...{
if(x+ox < 0 || x+ox >= width || y+oy < 0 || y+ox >= width) continue;
if(x == 0 && y == 0) continue;
@@draco18s even switch statements are quite limited, I was thinking more like what many functional languages provide. Switch statements can help, but in general the SOLID principles apply: don’t hardcode your conditions if they’re complex and may change.
I agree with your example, I do that too. I suppose it’s just whatever is readable to you (in six months time when you’ve forgotten what the code does)
One thing I would comment on with your example is that I rarely use continue because I rarely need to. Often it’s possible to apply Mike Actons data oriented programming advice and remove the items from the list in an earlier step, so in the loop you don’t need to skip items at all. Obviously that’s not always possible, in which case whatever, but when it is, I find it super helpful both for clarity and performance.
I must be missing something because in my opinion this is harder to read. I agree with this statement completely, if your code is nested that deep it's probably better to just methodize it out.
@@davidyoun8721 The lack of syntax highlighting probably isn't helping. I just meant that if the contents of my if-statement is like one or two lines, I generally won't bother with an early exit (because the early exit is *also* only one line). But I do early exits for for-loops to check for out-of-array-bounds-ness though. There's no point in extracting it out to a method because that for loop nest has *already* been extracted to a method (eg. count_adjacent_matches(grid, x, y, matchTarget))
10:13 usually instead of returning if your conditions are met, you would do it the other way round, also known as "returning early on error/invalid input". That way you can have multiple conditions at the top which will exit the method early, and the final chunk of code is where you handle your "valid" input.
Yeah that was a brain fart, I mention this in the pinned comment. I never actually code it the way I showed in the video so no idea why my brain at the time thought it was ok
@@nickchapsas No worries I just read the pinned comment after writing mine! Good video though as it does introduce some new concepts to new programmers. I'm just watching because I'm bored xD you are entertaining.
In my opinion, early returns are less readable than explicit else. Your code doesn't match your intentions: you intend to do the same thing as an else, but you instead use the early return. To me that's much worse than the indent and extra lines. If you put open braces on the same line, it's only one extra line with increased readability. (I know that's not C#'s thing, but if we're looking to reduce bloat...)
Agreed, not really convinced by this
Early returns are meant for filtering. You filter out what you don't want to process. Instead of using if else which sometimes the filtering will be placed near at the end of the function. Another reason not to use "guard clause", is to reduce scope level, allowing you to have better control over the lifetime of the object in C++ for example. Free unused variable/memory early
Imagine you have a function that will only process the data only if certain conditions are met, the instruct flow will go by, condition 1 not met? Out! Condition 2 not met? Out! So and so.. instead of going through don't know how many lines of code before I reach the point of "else return"
Apart from that, it's harder to keep track of which condition leading to that return without collapsing the whole scope of if (reduced readability)
I see early returns as checks to go through. Not necessarily guard clauses, but rather specific cases for which we want a behavior different from the default one.
When you see them like that, I personally find them readable. Get the edge cases out of the way and instead focus on the default behavior, really helps me with finding things readable.
I disagree. Nested code is harder to read and refactor.
Why use IEnumerable instead of List?
Bonus Question: How would you write unit tests for those private methods?
Also, thank you for 2 things:
- Showing real world example of Delegates
- Introducing me to Object Calisthenics
I use an IEnumerable because a List is a mutable data structure. I could use an IReadOnlyList instead but at that point an IEnumerable communicates the type well enough. The private methods are unit tested via the public method. You don't write unit tests for unaccessible methods but you write scenarios for the public methods that those private methods are being used in.
@@nickchapsas Thank you for the response sir!
I haven't really been using the else keyword for a very long time either.
I didn't really realize it until a few months back either, but I think it has to do with my personal theory of 'The less indentation, the better' and generally you get more indentation from else-statements
Another advantage is that it reduces the temptation to violate the single responsibility principal - you are not doing something else.
Look up cyclomatic complexity. The same as ur personal theory
If you are worried about indentation, stop using C# entirely. You have now saved yourself multiple indentation levels.
Interesting! I much prefer using the new switch expression over if...else if...else, but I can’t see the utility in completely removing simple if...else blocks. I think understanding the reasoning behind some of these “rules” is instructive, even if one doesn’t necessarily implement them.
You sacrifice the subtle simplicity of always having one place where methods return for what is essentially a distaste for indentation and brackets since using else is not nesting.
I think multi exit is ok for 5 maybe 10 liners with max 2 exit points. Beyond that, it stops feeling good.
Well the point is also to not have such long methods
Well i wouldnt say that there is a distaste for identation and brackets but more an approach to make the projects more flexible and scaleable. Imagine you'd have to implement more complexity to your software like considering laws of different countries or translations or an API for third-party.
To give you a more visual representation of this imagine the following:
Always throwing your cloths on a big pile in your room has the simple simplicity that you can always find everything you want to wear in one place. You dont even need a drawer. ... Makes (maybe) sense if only have like 2 jeans and 2 shirts
But what if you buy new clothes . What if you want to sepperate your expensive shirts from your old sweaters? What if you dont want your other shirts wrinkled just because you are looking for a pair of jeans. What if you're tired of spending every day 15 minutes just to find your socks?
The bottomline is .. the more your software grows the more you want to keep a specific structure which allowes you to minimize the effort for changes or extensions.
@@lukassinkus6162 then you just add even more mental context switching
You took complicated code, made it simple and then made it even more complicated than before!
Multiple exit points breaks one of my strictest rules, so that's a definite no. I have no issue with simple if-else statements, but if-elseif-elseif-elseif-else type statements should be avoided if possible.
Yeah, it’s a bit of a code smell at that stage
Yeah I leave MEP for nuclear situations only
I HATE having to scroll all over the source file to read code that can fit all in one screen because someone thought breaking it out was "more readable"!
I loved this video! I would 100% watch more video of you just coding/talking/refactoring. Very educational. Thanks, Nick!
Thank you for leaving the last part unedited! It's always interesting to follow someone's thought process.
Liked it, but it's more mental gymnastics, good when You have time to do some refactoring, but in real life scenarios, it is quite not afordable.
Besides most experienced programmers already return early if they can and it is obvious to do so. Why use a temporary variable when you can just return the results directly. This video went way out of scope. Explaining why you don't need else takes 1 minute tops. All the rest is about clever ways to refactor your code.
@@NicolaiSyvertsen I was thinking the same. I like the guy's videos, but this one really felt like it stretched the material. Didn't need 20 minutes in what could have very easily been 10, and 5 is still more than enough.
Good practices are rarely universal. One needs a sense of 'taste' to determine whether the practice is applicable in a particular situation. You mention this quickly at the end 20:06.
This applies to extracting methods, return instead of else and creating classes instead of switches.
If you use extract method *where you shouldn't* you end with methods that have little meaning by themselves, are highly coupled together and code that is hard to understand because you have to keep jumping from function to function to follow what's going on.
If you create class to remove switches *too aggressively* you may introduce a lot complexity (classes with composition/inheritances) that end up making code more complex than it needs to be.
Other times it's the perfect choice.
I find smaller methods with most specific code way easier to work with and the method read way clearer to me. This is subjective, as is the whole video but I get your point as well.
I noticed this thing when I read that Visual Studio counted both if and if-else statements as the same level of complexity during code analysis.
Started doing this about half a year ago, really helps clean your code up. If you mix this with making sure your functions are honest will naturally make you follow the boy scout rule.
By honest functions I mean, don't name a function something then hide some code in it that's not obvious to the name of the function.
It's a very common problem because it's so easy to do.
A quick example is something that returns a value. Don't manipulate your program in a function that, any dev reading the name would assume, just returns a value.
Is adding return instead of else really better? You can overlook the return and think the else part is always executed at the end. But you can't overlook an else which adds extra intention.
It's definitely not better. Totally violates one of the principles of Clean Code. The reason it looks cleaner is because adding an else creares an extra 3 lines of clde since he's using uncuddled curly braces. Really dumb imo. Just use ternaries.
Omg yes, that's what I thought. I feel like it actually makes the code less flexible. Say you have to edit one of the 10021 methods you need to create for this, and in one of them you forget to remove the return statement or whatever. It's gonna fuck you up. This is just so messy and illogical.
i really liked the part when you free styled and were applying open closed, the comments in hind sight were really usefull and gave me a clear insight into how i need to approach this kind of refactoring! you are great
I Really enjoyed this refactoring focused video! I Would love more refactoring content in the future.
Hmmm... numerous replies below already express the reaction I had, i.e. that pouncing out of a procedure in the middle makes things less, not more readable and maintainable, "s/return/goto theend/" comes to mind. Can programmers these days really only cope with one level of indentation? IMO the success and failure paths should both reach the end of the procedure and many client coding standards documents would agree. Sure having these return statements is easily spotted with very very short procedures but again, I'd say that's taking it too far. A well-crafted procedure of a hundred lines or so shoudn't present a problem to anyone to read and understand, if the job the procedure has to do is best expressed that way. If complicated parts can be sensibly extracted out, or are used elsewhere then fine, but to end up with several times the number of procedures so each can have just 3 or 4 lines of code makes the code base a nightmare.
It's all very well with your example of half a dozen interfaces but get a team of 10 to code something for a couple of years, then a variety of changing staff to maintain and update it with all sorts of unforeseen client requirements for another couple of years with this approach and 20% of your code base will be function declaration and the notional namespace of procedure names will start to fill up....
"Dave, can you fix that bug in ServeMojitoToPersonWhoIsOldEnoughButAllergicToMintAndDoesntLikeStrawsAndPrefersABambooMatAndNoUmbrella() please" - sure, while I'm there can I put parameters in for mint allergy and straw, mat & umbrealla preference and remove the other 12 procedures that do almost exactly the same thing?" NO Dave, then the procedure would be more than 4 lines, go wash your mouth out with soap !!
This is just silly. The "default" in the switch is equivalent to the "else" keyword. Furthermore, the ternary operator has an equivalent else part. This really is a personal choice with highly subjective advantages.
If you watched the full video you’d see that the switch was refactored out as well. Also something can be the equivalent of something else but doesn’t mean it’s the same
@@nickchapsas Default is basically else... Seems like you're splitting hairs here. And what's wrong with else? It clearly conveys the intent of the code, unlike an abrupt return in the middle of your function.
I've been retired from coding for about 6 years, and I see that new syntax has been added to C# since then. Now I'm rather interested in finding out more -- thanks for waking my coding brain cells up!
No one:
Clean coders: don't use "goto"!
Everyone: ok, seems fine.
Clean coders: don't use "else"!
Everyone: hmm maybe?
In future, no one:
Clean coders: no more than 4 lines of code in a function!
Everyone: what?
Clean coders: don't use "if"!
Everyone: wait, what?
Clean coders: don't use !
Everyone: wai...
Clean coders: reject humanity, return to monke!
If you do not use "else" it looks a little less bloated, BUT at first glance, it looks like you print two messages instead of one. And if you collapse the if statement for whatever reason you will for sure think that second option is printed always. With "else" you know for sure, and at a glance that it will only be used sometimes. Also, else makes it more symmetrical, which is a aesthetically a plus for me. But that is only my opinion, it may be very well valid point. I, for sure am not a clean coder yet ;-).
Clean coders be like
Int main(){
EntireProgram();
}
What I hate most about C# is the lack of fall-through, and only way to do so is to use a goto case, which means everyone reviewing it instantly throws it out since dreaded goto keyword. /sigh
(Do agree with not using Goto though, ends up with the most spaghetti code ever, 98% of the time there is a better way of doing it)
@@Masterrunescapeer case statement fall-through is one of the hardest features to maintain. It is terrible. It does let you write shorter code. But objectively harder to read.
I'm a great fan of using return to avoid if-else nesting whenever possible. However to state that only one nesting level is allowed per function and that else must be avoided is too much like a religion to me. It all depends on whatever is the most readable in the situation at hand. Having a plethora of small extracted functions is not necessarily easier to understand and maintain than a chunk of code, unless that chunk becomes larger than, say, one page or screen.. As for using returns, there are also people who insist that a function should always only exit at the very end. Which makes sense if something needs to be cleaned up like closing a file or disposing an object. All in all, there is no right and wrong here. Whatever is most simple and transparent is best, IMO. Never mind principles or paradigms. I'll also happily use a goto when it suits me (though never, as I've once seen in a COBOL program, to a label in another function 😵).
I agree with most of your early points ... but the level of oo is making me nauseous
I stay by the principle that a method should have a well defined entry and one exit point.
private void HandleBeerAgeCheck(int age)
{
string msg = "Enjoy your beverage of choice!";
if (age < 18)
{
msg = "Sorry, but you're not old enough to drink (in the UK)";
}
_outputProvider(msg);
}
Accomplishes exactly the same thing - but there is one entry point and one exit point. You could streamline it by:
private void HandleBeerAgeCheck(int age)
{
string ageOk_Msg = "Enjoy your beverage of choice!";
string underAge_Msg = ""Sorry, but you're not old enough to drink (in the UK)";
string msg = (age < 18) ? underAge_Msg : ageOk_Msg;
_outputProvider(msg);
}
I hate troubleshooting a routine only to find out that it exited in the middle, or having to adjust the code to add some logic because of a premature return (For example, say later you want it to always output "Please do not purchase alcohol to be consumed by a minor!"). How would you do it? It's not hard a all to add that to either of my examples - one line.. done.
There are simply much better ways to accomplish the goal. Myself, I don't mind else statements but I don't like a bunch of nested if then/else statements, which are easily handled by refactoring like you did.
One entry point - one and only one... exit.
This is what I call "gated if" statements. Makes code very neat and easy to follow and update. Also makes it really easy to add a comment for each check. It is limited to methods because you have to be able to exit at each check.
It's not limited to methods at all
I think the word your looking for is "guard clause"
@@GavHern I learnt it informally. I don't think I ever heard the proper name for it. Thanks.
The problem is the checks become order dependent and it may not always be clear where to add a new check. Also some checks can become convoluted if you need to check four things before deciding to return right away because now you get a complex comparison statement.
Problem with early returns is that it puts all code under your IF block into a virtual ELSE block and you need to keep inspecting the entire contents of the IF block for RETURN statements to see if the code below with be hit or not.
I'd often prefer the ELSE blocks be explicit as opposed to virtual. You want to be clear about the fact that the code below only runs conditionally.
I mean both approaches are defensible in the end... I'd mostly advise against hiding return statements in large IF blocks where they could realistically be missed.
I am a noob and today I learnt that you can return in a void method to stop executing the following statements. Neat.
Yep, I do that also.
When I have lot of conditions to check, I start to check most common ones and go out of the method/function straight away.
So no need of if else if else if else blocks.
Only If statements with straight return if condition is true.
This makes the code way more readable (and you can write on a single line) but also much faster because you won't check useless conditions.
Looks like :
if (condition1) {return false;}
if (condition2) {return false;}
....
if (conditionXX) {return false;}
return true;
One thing to keep in mind is that this might clean up the look of the code, but functionally it is doing the exact same code either way. If you go down to the assembly language level that this generates, the return in the if statement does not actually return from the function right there. What is does is a jump (jmp) to the code at the end of the function right after the code at the end. This is the same that is does when there is the else there. It jumps (jmp) to the code after the else. From a performance standpoint, this does nothing. If you like the looks of it better, then feel free to do it, but it is not making your code perform any different.
Everyone reads and can follow code differently, it used to be that making your code unreadable to others was job security, but today this is not the case. With coding standard, with other best practices that an entire team might use, you need to follow what is expected. DoD standards at one time said that every line of code needed to be commented. EVERY line of code. Not sure what they say now. But use whatever coding practice/standard that you and your team agree on and that make it readable for others. Being tricky is not just worth it today for many projects.
I find arguments like this completely pointless. Ofc the code will always be optimised on compilation, that doesn’t mean we can’t make the wait it looks better to make reading and working with it simpler.
Am i the only one that prefers to do the maximum inline or in the same method??? Cant stand splitting up methods arbitrarily like this
Having an else keyword is more readable than returning out of the method if the former is true.
It’s the same as using “break” in “switch” “case” as he enters a call to a function with the “if” statement.
The readability issue is simply due to your being accustomed to the use of “else” in main code.
@@mikelang4853 Yes, but with switch statements we know beforehand that we will only enter one of those paths, and else statements promise the same thing, but he's breaking the prosumption that those two if statements could both be executed at the same time, and if he doesn't want that then why isn't he just using the else statement which specifically serves this purpose.
@@crash1998100 you are free to code how you want and so is he. As long as it works as you intend, it’s just personal choices and how you view it. He’s just sharing another way that it’s possible to write code and how he is doing it now.
When a function is called and once inside it, the expectation is that you do something and exit once done. There could be several “if” statements each with “return” statements if it’s true.
*obviously the below is not real code*
Function stoplight (){
If red, {x=stop, return }
If green {x=go, return}
If yellow {x= go really fast, return}
}
As long as the coder knows that there can only be one of the three choices, once the voice has been evaluated as true, the other choices don’t matter for that function call.
You could change the RecipeBook class into a generic IRecipe interface and have BeerRecipe class and JuiceRecipe class implement the interface. So your bartender would just have a List. The IRecipe method would just have a boolean return that signals the success/failure of the method. The bartender would loop through its recipes and attempt to make drinks with the input keywords, and stop the loop if a success is returned.
It would make adding new DrinkRecipes easier and expandable for wierd Recipe edge cases as each Drink is its own class.
I have already mentioned this in the pinned comment
As far as i know this pattern has a Name: "Guard Clause"
yeah. I'm surprised it isn't a common knowledge for many programmers, in fact none knew about this in my company when I first joined. I had to teach all the developers about writing clean code and so on. Partly I blame the education system in uni that doesn't teach students about simple things as best practice and effective code writing. Me being a self taught programmer learned these through open source projects and articles, I encourage people do the same
It's expected for guard clauses to throw exceptions instead of returning an error or nothing. For example, returning an error or just returning won't work in methods with a return type. Then again this is specific to typed returns in languages that have them.
@@AyushkaPartohap not entirely true. But I get what you are trying to say. Guard clause is actually based on the return early rule, so it can also return result early not just throw exception early. Another principle guard clause follows is to reduce scope of variable, or number of scope
@@shikyokira3065 First of all, I completely understand where you are coming from. Everybody probably 'wish' they knew this from uni. It is however not necessarily the uni's task to teach you things like this. Learning things like this is something that comes with practice and even then it should not be learned as "truth" or the "Best way to do something". I think it is very much more valuable to learn it by 'seeing' and experiencing yourself that 'this' or 'that' is a good approach. So what you say that you teached other developers this way of thinking about it, is something SO VALUABLE and you definitely should not stop doing. Maybe someone else will teach you a thing or two and then you would also hope Uni taught you that, but they didnt....
I guess my point is that you cannot teach "EVERYTHING" in uni and some things you learn by experience. In 1 year you will maybe think back about what you wrote and the outcome 9/10 times is probably: "I would write this better now". That in my opinion is and should be completely fine, even though you wish you would know beforehand :) That is what learning and progressing really looks like.
@@bl1tz229 um.. you got a point there. And certainly we can't learn everything from uni. I'm just highlighting the importance of it, and since uni is kinda the process we normally go through to gear ourselves up, missing this out feels like missing out 1 of the important tools that we very much need when gearing up.
And i totally agree with you that learning it by 'seeing' it and experience it, is valuable. Just like in uni, we normally have to go through research papers done by other researchers. Same should be done for "programmers", go through open source projects, and obviously must at least has a standard, not a "cmd snake game" kind of project... you get what i mean
Nice and very helpful video! I’ve been doing this unconsciously because I’ve found out the hard way that the “pyramid of doom” (multiple levels of nested indentations) isn’t the way to go. It’s good to be aware of what one’s doing so this video really helped me gaining awareness of these patterns.
One thing that I’d have done differently is right around 9:55 you return in the “happy flow” and fall through to the fail state (if input >= 18 return;). Personally, I’d return in the fail state and always end up in the happy flow at the end of the method (guard, exit early and follow the expected result - kinda like the guard-statement encourages in Swift).
Thanks for posing these video’s, they’re a great learning tool!
This is why in "interpreted" languages (I know... it's compiled to bytecode, but more or less, that means it's still interpreted) profiling is hard. If you did this in C++, you could possibly pay performance penalties, especially if you return from a function within the if statement (which you should, compiler optimizations can do a lot of great stuff). But I certainly would not know what outcome this would have on performance in interpreted languages. But so much in this video, is a direct result of Java-ish languages design. So now you are spending time ameliorating all the issues with that design, by doing things, which makes the code less clear.
Performance penalties? Dude its 2021 the RAM is cheap. Processors are way faster then they have been 20 years ago. HDD are cheap.
C++ brings a lot of problems as:
Slow codding(you have to get rid of the used memory by your own) in language as C# you have Garbage collector,
Some times it will take you 1 hour to make something to work properly. In C# as Example that would work with 1 NuGet Package and 1 Line of code.
I prefer to take more time in code logic at it self then in so trivial thinks as managing memory usage.
Not to mention the fact that if you bring C++ code from Windows to Linux/Mac etc.. it will not work. If u code it under 64 bits and get it on 32bit machine ...it will not work.
And this is HUGE minus.
The era of C++ is near over.
@@FalioV I don't think you understand how memory works. You don't have to "get rid of the memory", that's the whole point of RAII, so your point about slow coding is objectively false, life times handle that for the user. Even data created on the heap, wrapped in unique pointers is handled automatically.
As far as when it comes to package management? Using CMake you can now add whatever libraries you want, that lives on github, and it will automatically fetch those for you. But we can even take it one step further, and compare it to Rust, which has its cargo management build tools, making it even easier and better than C# since the project management is centralized and the overview much more compact than the project configuration of a C# project.
But let's get back to the point of memory. RAM is cheap - but that was never what made a program slow. Not using the cache coherently is what makes programs slow, now tell me, how good is the cache utilized in interpreted languages? Not too great. Python does it well enough, because it has been incredibly optimised for data science, but general use which you and I are talking about? Not great.
And lastly your point about cross platform, C# is the least cross platform friendly language out of all languages. It's worse than Java, exponentially much more worse than C, C++, Rust, Python. So that argument is not correct either.
The 64-bit vs 32-bit is a non sequitur. The standards have well defined types that says "an int on a 32 bit platform is 32 bits, and int on 64 bit is 64 bits". So I don't think you understand cross platform thinking quite right, either.
Uhm, no, it is actually the exact opposite - profiling in C# is way more easy to do than in C++. In C++ you need to do a lot of work to prepare your code for effective profiling. In C# it works out of the box. Attach a profiler, study the result, that's it.
But anyhow, what you are writing sounds more like you want to do premature "in your head" optimization. Don't do this. Compiler optimizations can not just do a lot of great stuff, they do a lot of stuff based on complex rules you could never predict in your head. Experience shows that any C++-Programmer who does one hundred changes to his code for performance reasons will have ~33 changes that do not change anything at all, ~33 that reduce performance, and ~33 that increase it, and pretty much all hundred of them reduce the code quality.
Performance optimization should only ever be done using an actual profiler on the actual running application, running with actual real world data/inputs. And on this front, interpreted languages are way better equipped than native languages.
Guard clauses can also become hard to read, too, and they become order dependant. It means someone has to make sure when they add a new guard it comes i the right place/order and this isnt always clear
Impressive work, I am thinking how much effort you put in the work. Nice to watch your videos
Loved the refactoring section! So many dev videos on UA-cam only show rehearsed videos, so it's nice to see the real thought processes as you went along.
I was expecting an early return here eventually. I think that it's a bit of a tradeoff. Early returns can also be pretty hard to work with, especially in long methods. At my work early returns are banned as per our coding standards.
There should be no long methods ;)
9:55 the “not old enough” is an error condition if you ask me. It makes more sense if you code it as the if condition and fall through to “serve beer”.
Check the pinned comment
Uses ternary operator everywhere like a boss
Instead of an else you can always use goto.
@@nextlifeonearth thats illegal
@@csabraxas if (condition) {
doThatThingy();
goto afterElse;
}
doTheOtherThingy();
afterElse:
I love programming.
Usually forbidden in safety critical code because in practice it's been found to lead to write-only code and too many bugs.
@@digitig Yet safety critical libraries like OpenSSL actually use goto. I assure you the people who made that knew what they were doing. It's also the best way to get out of a nested loop or sometimes even switch.
If you don't know how to use it, then by all means don't use it. Dismissing every case where it's being used is ill advised however.
What you describe is basically the same reason why we very rarely use the goto statement.
We can do everything with gotos and ifs. But the codes becomes very hard to read. Instead, we use control flow mechanisms which are more specialised, thus less powerful, but also are easier to read (and also make it easier for the compiler to optimise).
Concerning the if statement, in nearly all cases, its only use is for "easy exception management". I would write your HandleBearAgeCheck as follows.
private void ServeBeer(int age){
if(age
Check the pinned comment. That was a mistake. On the actual script I had the check the other way round, basically how you described it.
I don't agree. The code he creates by using multiple returns is more like code having gotos than the code before. If you have functions, modules, or logical units that have one entry point and one exit point that is code that does not have the "goto problem" of not having a simple flow of control. This code without else statements has multiple functions that have multiple exits. This code is harder to maintain exactly for the same reason gotos make code hard to maintain.
In this video: How to multiply your LOC by 10x and make it less understandable and straight forward!
this video should be considered harmful - as an example of over-over-over-engineering.
Could you explain why you don't recommend using the single exit pattern?
Very subjective topic but single exit is usually assosiated with nesting with leads to less readable code for me. Also by using multiple exist points I am able to do less work in the method, exit early and have a clearer request lifetime story. This is all very subjective but having seen both in big and small services, multiple exit point work for me better.
@@nickchapsas thanks for the quick reply, I appreciate it!
Don't exit directly from a triple-nested loop. This makes your code a lot harder to read and to reason about. Exiting after a top-level if statement actually improves readability because (as the video beautifully shows) it gets rid of the nesting.
At 10:00, I was bothered that you mixed two different approaches. ServeBeer handles the sad path first, then the happy path, but HandleBeerAgeCheck does the opposite. I like to handle the sad path first, because it's usually the shortest and it just gets in the way of what really matters, the happy path. Anyway, I know you improvised and all, and I bet you'd normally watch out for such details in a real-world scenario.
Check the pinned comment
As a novice, the part that boggled my mind was how simply you automated the string at 15:50. I'd have spent half an hour figuring out how to manually create a comma separated list from my keys without an extra comma at the end or something.
The chosen example is such poor code that it hurts the argument. And you pretty much acknowledge this. Why not just go into open source projects, search for an else and show the benefits with not using them? If this is a good rule to follow then it won't be a long search.
I don't like to extract methods like this at all. In languages without local functions you've now got to consider if the code is reused somewhere and any change to the code must be careful to make sure that you aren't using anywhere else.
You don't know the state changes of this call either. Everything is hidden. The code before was ugly but it was at least clear in what it does. Now we have no way to get an overview. An overview can be hurt by having too many lines of code, there functions become more appetizing. But this wasn't a case like that. Control flow is the last thing I ever consider making a function for because it usually is very context dependent. I don't see the benefit of hiding that context through function calls. In this trivial example it's still easy to read of course. Especially since we already know the entire purpose of the code. But when reading for the first time that's unlikely.
The readability gains in the main method are mostly from having given the different branches names. We can think of a million ways to do this that don't have any side effects. The simplest I can think of is a comment labeling each case.
I think the advantage of trying not to use else is not removing the else itself (although I think this is an added value), it is that in order to do it you need to write good code in the first place. So the rule should be: If you find yourself writing an else statement, try to remove it, if you cant you should probably refactor your method.
I really dislike short-circuited methods, and when I use the technique I make sure to make a big deal of it so the "return" is readily seen.
Totally agree, I find myself rarely use 'else' as I normally would have a return at the end of an if block.
Agree with that clean code rules. The if part with return statement is also called "early exit" because you don't need to proceed to the next statement even it has nothing to do more after it.
Adding to your methods of replacing your if/else/switch statements, we can use open/close principles + attaching interface to your drink classes
Ex:
your IDrink has a properties and methods
- DrinkName (juice,beer, wine etc)
-Serve (your console.writeline of "Here's your {drinkName} and Sorry but not old enough to drink")
-GetAge(int age)
//Example: drinkInput=Beer, ageInput=16
IDrink drink=Drink.GetDrinkByDrinkName(drinkInput); //any class that hsa IDrink methods and props. Is it beer or juice?
drink.GetAge(ageInput);
drink.Serve();
Yeap you are 100% right, I've actually addressed it in the pinned comment. I didn't wanna spend more time refactoring but the irony is that people seem to have really liked the refacotirng part so I might make a refactoring dedicated video.
Kotlins WHEN keyword: "Let me introduce myself"
is it kind of like switch expression? i just loked at an introductory video. i think if you are familiar with .net stack, you would love f#
@@tuberklz kinda, but it's much more flexible, it's like a switch which has the full range of if functtionality available, also because it's Kotlin, you don't have to worry as much for unexpected NPE being thrown.
There are two different things going on here. One is replacing branches with dynamic dispatch. This is important for extensibility. It’s key to many patterns such as the strategy pattern, the abstract factory pattern, and others. It’s generally good object-oriented design, especially when more cases are foreseeable changes, and doubly so when the same case analysis has to be made in two or more places in the code.
The other thing is trading an else for a return returns. To me this is replacing an “else” with something worse: anomalous control flow. Some general “rules” I follow (which have exceptions of course) (a) to have zero returns in void methods and one return in methods with results and (b) to avoid assigning to any local variable more than once in its lifetime unless it’s carrying information between loop iterations a loop. These rules mean that almost every “if” has an “else”. An “if” without an “else” is sign that there is something odd going on. Another way to look at is that every command has a job to do. In the “then” part of an “if” command does the “if” command’s job in the case where the guard is true and the “else” branch does the job of the “if” command in the case where the guard is false. It’s rare that the job can be done by doing nothing. When that happens I’ll sometimes put “else {/*do nothing*/}”.
Big no-no for me on this approach. Having alternative exit points for your function seems much worse to me than having an else statement. I'm happy to remove an else path when I can and make sense but not when I replace it with something uglier, like a return statement hidden inside the if body.
If your language supports guard-else I would use that and put them at the beginning of the function, as a precondition validation step, otherwise I much rather prefer to keep the else.
Also, some refactoring to remove excessive nesting is welcome, but having strict rules is counter-productive. In medio stat virtus.
In my opinion - the author missed the main reason why "else" is faulty. And that is the case that the logic that triggers the { else } section changes when editing parts of code that are not the { else } section.
.
The else in these cases has no explicit readable code that would state that the case for not having a drink to service is because the drink requested is not one of the available ones. Readers of code with a bunch of else-ifelse, have to manually combine in their brain all the conditions and logically come to the conclusion what are the cases in which the "else" would be triggered.
.
I would remove the "else" via explicit coding
.
bool unavailableDrink = listOfAvailableDrinks.Contains(requestedDrink) == false;
else if( unavailableDrink) { //logic whoes trigger complexity will not escalate as more code and cases are added }
I haven’t used “else” in 2 years now. But first I wanted to talk about something “else”.
imo the ternary expression is far more readable than having an if block followed by a stranded line of code.
With the ternary statement, it's you don't have the repeated output template, and it's clear that you're just toggling the content based on a check. It's just so much cleaner!
I think the plethora of functions created to keep the indentation level to 1 will make it hell to read the code as you’ll constantly have to jump from one function to another. In complex code this will also potentially slow down execution (function calls aren’t free unless the compiler decides to inline them) and there’s an increased risk of a stack overflow (although it needs to be said that if you run into that when there’s no bug in your code (e.g. endless recursion) your code is ripe for refactoring anyway).
default in switch case is "else" =D
Shhh, you’ll rouse the rabble in the insane asylum :p
This video made me realize I too don't use "else" anymore! It happened totally unintentionnaly. I just applied usual recommended refactoring (and often just followed Resharper suggestions) which led me here.
The use of multiple exit points in a function makes it harder to maintain.
This largely comes down to personal preference. For me, it is way harder to have a single exit point in my methods. I much prefer multiple exit points because they are very clear in terms of where the method's "story" starts and ends. Code that uses single exit point is also really hard to read for me and usually involve heavy nesting due to that, which I don't find good to my eye. Again, all very subjective.
@@nickchapsas Exit point must be one line return of already prepared result, no deep nesting, and you have mostly readable and understandable method you can have, it just don't need comments, it's self explained.
@@nickchapsas thanks for the reply. As an old school programmer, I guess I am used to structured programming. Enjoying the youtube channel btw!
@@henrivanwesemael Thank you so much! The beauty of programming is that there isn't a single way to do it right. We are all "right" and "wrong" to someone
I have to admit, I was extremely skeptical about this one. I was pleasantly surprised. Obviously not applicable in every single case. The refactoring thought process is really important. Especially for novice programmers.
This is a really good example of how following rules blindly can completely destroy the readability of your code. You started with easy to read code and turned it into an unreadable mess ;D
I would argue that it started as an unmaintainable mess and because more readable. Readability is totally subjective ;D
Completely agree with you. It's much easier to figure out what is going on in one function which is 20 lines of code than 20 functions 1 line of code each.
@@DenysenkoIvan I don't think that anyone advocated for 20 functions with 1 line of code each. It's all about having each function doing one thing only, no matter how many lines that is. It could very much be 20 lines.
Interesting approach, with javascript as my main language i dont really have many problem with the other things mentioned in these rules. I think they make sense to some degree, but with the ability to use ternary operators for simple single line if/else statements and the ability to use inline arrow functions to aviod unecessary indentations, I feel, that these rules are very easy to follow. for preconditions, like an age check or checking if we found db entries, a negative condition that uses a return statement or throws an error should be the default.
Whatever rules you follow as a dev, I think the main thing to think about is, that the code we write in our highlevel programming languages should in most cases be optimized for readability and after that come things like speed and and memory usage (depends on application tough). Code thats hard to read is hard to maintain and will only get worse over time and eventually needs a large refactoring.
Thanks nick for teaching the world how to write clean code
Functional safety coding (in C, C++) standards and guidelines dictate that you need to demonstrate that you have handled EVERY eventuality in your code. That includes using the DEFAULT keyword in SWITCH statements and ELSE in every IF. If there is no body to the ELSE, then it's still included in the code with empty braces after it (and a comment why you don't need to do anything!). This is to show that when you do a code review, that EVERY eventuality has been considered, even if there is no requirement to handle it. the choice as to use IF-ELSE or a SWITCH-CASE statement depends on the number of checks. Performance wise, a switch statement is only more efficient when you reach a certain number of check conditions.
Returning mid-function is also frowned upon as you should always ensure the function exits in a single point so that you have handled other things such as 'clean-up' gracefully. I have seen code in things like memory allocation functions that exit (using a mid-function return) on failures and bypassing the free'ing of the memory that was allocated. Might be easy to read, but can cause major issues with deallocating object, clean up, etc. Hence an exception handler TRY-CATCH has a FINALLY keyword!
Ref: MISRA C/C++
It makes no sense. When you have some function returning something and using if/return aproach, you have some last return satement with is forced by language, and this is your 'default' choise. Switch or if/else neither guarantees you checked all possible results, they are just equal language possibilities, if somebody extended some enum in your codebase, neiher if or switch can guarantee that it will not build until you handle this new case. But there some advanced approaches to handle case if you want your library refuse to build if you missed something unhandled, and I can share it with you.
@@iGexogen That's what the processes of code reviews and unit testing are for! If you extend an enum, it will be caught by the default statement in a switch (which should output to debug that an unknown case has not been handled) or even throw an exception in the default handler. Either way, you change the behaviour of the code, you should FULLY TEST IT ... Unit Testing and Regression Testing!
@@simonbaxter8001 Output to console makes no sense at all, you will not notice that your code is broken only after you will get report from real users on production. Only unit tests that enumerate all possiblilities can help, but there other cases, for example when you must handle some action for all subclasses of some base class, and you cannot predict all subclasses in cases of "plugins" system or something. No automation can handle this cases, only good architecture.
@@iGexogen You will notice in the console during debugging and development testing! If you need to handle 'plugin' type scenarios, that default handler should return an error from the function (i.e. this case is not supported), or as I say an exception. The developer of the plugin has the responsibility to test their code works with your framework and check return codes. Good Architecture and documentation is the key, but testing is a big key too. Automation and a good test framework should be part of your architecture design, not an after thought. That is where a lot of young/new coders go wrong these days. Look at the Microsoft c# libraries, they are full of error handling and passed in parameter validation. That is also key too. Make your framework scalable and robust and it will not let you down!!! My pet hate is people not testing for 'null' or NULL pointers before using them! Testing is key. Don't let your customers/end users be your beta testers ... it's not good for business and your product/game. They should never see bugs if you have done your job right. I write code for automotive ECU's (Engine, Brake, Transmission) ... I can't afford for my customers to find 'bugs'!
@@simonbaxter8001 So why you oppose this things? Good architecture and intensive testing only complement each other, and I completely agree that in some project types tests can be very vital, and most important subsystems must be doubled and maybe even more. But I have another example with business applications where errors are pretty tolerable. I have legacy document flow system in my organization, and it works somehow, and if I could ever cover it with tests, I will never make them all pass, no way! I could fix in one place, then break in other, and there is no way to fix it due to it architecture and way it evolved (last is even more important), but it is still used by 50K people, and will be used for next couple of years.