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.
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
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.
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 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.
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.
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.
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.
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");
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.
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 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!
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.
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
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.
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.
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.
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.
@Nick Chapsas are there any Visual Studio Code or Visual Studio refactoring extensions that have this 'extract method' functionality and works with Blazor (.razor) files?
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.
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!
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))
Loved the refactoring section! So many dev videos on TH-cam only show rehearsed videos, so it's nice to see the real thought processes as you went along.
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.
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.
Great video. Thanks. Regarding the Open-Close principal - the method that initialize the dictionary still need to be changed when adding new drink. You can use IOC container/Reflection to complete the refactoring.
Yeah I mentioned this one. It can be done with either composition or attributes depending on how the dev wants to go about it but I didn't want to overbload the video. At least we eliminated method specific Open-close violations :)
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
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;
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.
9:18 is it possible to do some speed/performance and size analysis here or this is just readability optimization ? I ask because this can be multiplied by pretty large number if you work on big project and I imagine size is not that important these days but possibly performance is in some cases. It seems to me in this short simple example you tripled the code used for same result and yes it is more readable and maintainable but is it better in other ways like size and speed and ... or do we sacrifice some performance to gain readability ? For example if primary goals were speed and size would you still do the same ? Thank you.
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.
Nice presentation! For years I've not quite understood those SOLID principles, though most of them made some sense, in some situations. Now I feel enlightened!
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.
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 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
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
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.
"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.
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!
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.
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.
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.
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.
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.
What advantage is the early exit supposed to serve (other than compliance with an arbitrary rule)? It makes the code harder to read, by obscuring the logic.
If it makes it harder for you to read then you shouldn’t use it. It makes it significantly easier for me which is why I personally use it. It has nothing to do with the rule since you can return at the end and still honour the rule. The other “advantage” would be that your code doesn’t need to check more things than it needs to return a result.
@@nickchapsas Hmm. The version with "else" reads to me as "do this or that". The version without reads "maybe do this, then do that", and depends on spotting the early return to realise it might *not* "then do that". If you are the only person who will ever maintain your code then fair enough, but I think there's a reason the commercial coding standards I've worked with have deprecated early returns. What extra decision does the else involve?
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.
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.
Great stuff man! I've also completely ditched the 'else' keyword. I wrote an article a year back on this that blew up. I guess lots of developers still have to discover how awesome it is to live in an 'else'-free world.
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 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.
@@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.
I watched the whole video and I appreciate your video style. The title made me think there was some significant reason not to use else (performance or clarity perhaps), though it seems this is a 100% aesthetic choice. I am not sure I agree with this being a better aesthetic, I agree that it resulted in fewer lines per method but to me you made the code harder to follow. I personally think indention in the if/else makes the code more readable because I can instantly see that the indention level indicates that these lines of code could be equally weighted in relation to each other because they follow one another at the same indention and I can clearly see that from the if/else structure. When the else is removed for an early exit I now initially read the code as an escape condition preceding the rest of the intended processing which unequally weights the importance of the code blocks in the mind.
@@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.
You pointed out exactly what I am not understanding about OCP. From the perspective of the Bartender the refactoring is fine and follows the principle but hasn't the problem simply been pushed down? From my understanding RecipeBook in turn is now violating OCP. Literally it needs to be "modified". The dictionary needs to be extended and the method needs to be added. I think I am missing an exact definition of modification vs. extension. Does modification mean adding additional logical flows?
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.
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.
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.
Was it not closing off if statements with else what literally caused the 737MAX disaster? Everyone was talking about that like 2 years ago. the else is actually expected and funky stuff happens if its not there no?
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 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 agree completely about extracting the methods and keeping indentation levels to a minimum. It is much easier to quickly identify what the code is doing without getting bogged down in details. The details are still there if you need to see them so it's a win-win for me.
The more I develop the more I agree. I usually use a little more flexibility than what he's describing, though. I didn't used to like extracting code out of methods into private methods that only get called in that one place, but the readability benefit is pretty nice. I also found that if I was doing it a lot in a given method, then it was time to possibly refactor that some other way, too.
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.
Very high level content. I am glad I found your channel, I was just annoyed by all these elementary videos about coding, design patterns etc... Your content is very detailed, and I can see your efforts to try to make new developers better by writing readable code! Thanks.
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.
Quick question for experienced coder. Is there any way to write this shorter without repeating Y and Z condition twice?: if((x=1)&&(y=2)&&(z=3)||(x=5)&&(y=2)&&(z=3))
@Nick: when you changed the to function/method call, ok, it's more readable. But most importantly, it is the right way to think about any algorithm. From the general to the specifics. From the high level to the finer details. Anyway, a pretty good video you have here, mate. What you gave us here is a nice rediscovery of the theory we learned at the U. Well done. And keep going. ps: also, in the refactoring, when you remove the else and place a return, you're helping the reader to read your algorithm. I mean, the method servebeer is ... to serve beer. so that's the "algorithm", the most important part of the code. It can run without the age check. Therefore, it's good practice to do what you did. It allows the reader to see 1: the main intent 2: the exceptions.
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.
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 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.
Douglas Crockford said in his JavaScript: The Good Parts that don't use the 'continue' keyword as the removal of that always brings about better code. Any idea on that?
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).
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.
😂😂😂
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
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.
I loved seeing the refactoring approach to explaining this. Would definitely watch you refactor more code.
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
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.
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 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.
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!
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.
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
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");
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.
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.
Thank you for leaving the last part unedited! It's always interesting to follow someone's thought process.
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!
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.
There is no need for if, else, loops or anything like that. Real men use assembler and jumps
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
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 😂😂😂😂
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.
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.
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'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.
@Nick Chapsas are there any Visual Studio Code or Visual Studio refactoring extensions that have this 'extract method' functionality and works with Blazor (.razor) files?
I loved this video! I would 100% watch more video of you just coding/talking/refactoring. Very educational. Thanks, Nick!
I was taught 'one point of entry, one point of exit' in 1988. Multiple returns to avoid an ELSE would violate that.
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.
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!
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))
Loved the refactoring section! So many dev videos on TH-cam only show rehearsed videos, so it's nice to see the real thought processes as you went along.
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.
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.
I Really enjoyed this refactoring focused video! I Would love more refactoring content in the future.
Great video. Thanks.
Regarding the Open-Close principal - the method that initialize the dictionary still need to be changed when adding new drink.
You can use IOC container/Reflection to complete the refactoring.
Yeah I mentioned this one. It can be done with either composition or attributes depending on how the dev wants to go about it but I didn't want to overbload the video. At least we eliminated method specific Open-close violations :)
@@nickchapsas by composition do you mean adding IDrink with PrepareDrink method then getting a dictionary of IDrink to the ctor with ioc?
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
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;
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.
9:18 is it possible to do some speed/performance and size analysis here or this is just readability optimization ? I ask because this can be multiplied by pretty large number if you work on big project and I imagine size is not that important these days but possibly performance is in some cases. It seems to me in this short simple example you tripled the code used for same result and yes it is more readable and maintainable but is it better in other ways like size and speed and ... or do we sacrifice some performance to gain readability ? For example if primary goals were speed and size would you still do the same ? Thank you.
Done that already. Check my video "Is clean code slower?"
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.
Nice presentation! For years I've not quite understood those SOLID principles, though most of them made some sense, in some situations. Now I feel enlightened!
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!
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 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.
Wow, thanks for your expalanation and examples! I finally understand how these things work, can't wait to try myself tomorrow.🥳
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
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.
You took complicated code, made it simple and then made it even more complicated than before!
Your videos are so excellent! The best around! ❤️🙏🏼🙏🏼🙏🏼
"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.
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!
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.
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.
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.
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.
Impressive work, I am thinking how much effort you put in the work. Nice to watch your videos
Solid demo. I like your off-the-cuff style. Fluid and flexible, but still easy to follow.
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.
What advantage is the early exit supposed to serve (other than compliance with an arbitrary rule)? It makes the code harder to read, by obscuring the logic.
If it makes it harder for you to read then you shouldn’t use it. It makes it significantly easier for me which is why I personally use it. It has nothing to do with the rule since you can return at the end and still honour the rule. The other “advantage” would be that your code doesn’t need to check more things than it needs to return a result.
@@nickchapsas Hmm. The version with "else" reads to me as "do this or that". The version without reads "maybe do this, then do that", and depends on spotting the early return to realise it might *not* "then do that". If you are the only person who will ever maintain your code then fair enough, but I think there's a reason the commercial coding standards I've worked with have deprecated early returns.
What extra decision does the else involve?
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
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... :)
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.
Great stuff man!
I've also completely ditched the 'else' keyword.
I wrote an article a year back on this that blew up. I guess lots of developers still have to discover how awesome it is to live in an 'else'-free world.
Is there any problem with "else" or do you just like to jump through hoops not to use it?
@@vast634 when would you ever need the else keyword besides for micro optimizations?
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 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
Totally agree, I find myself rarely use 'else' as I normally would have a return at the end of an if block.
I like how you use the keyboard shortcuts to efficiently navigate and refactor your code.
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.
I watched the whole video and I appreciate your video style. The title made me think there was some significant reason not to use else (performance or clarity perhaps), though it seems this is a 100% aesthetic choice.
I am not sure I agree with this being a better aesthetic, I agree that it resulted in fewer lines per method but to me you made the code harder to follow. I personally think indention in the if/else makes the code more readable because I can instantly see that the indention level indicates that these lines of code could be equally weighted in relation to each other because they follow one another at the same indention and I can clearly see that from the if/else structure. When the else is removed for an early exit I now initially read the code as an escape condition preceding the rest of the intended processing which unequally weights the importance of the code blocks in the mind.
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.
You pointed out exactly what I am not understanding about OCP. From the perspective of the Bartender the refactoring is fine and follows the principle but hasn't the problem simply been pushed down? From my understanding RecipeBook in turn is now violating OCP. Literally it needs to be "modified". The dictionary needs to be extended and the method needs to be added. I think I am missing an exact definition of modification vs. extension. Does modification mean adding additional logical flows?
Just saw your pinned answer, thanks!
In this video: How to multiply your LOC by 10x and make it less understandable and straight forward!
Agreed. I've never really liked using "else", I've always done what you have shown here in regards to using a switch and returning from methods.
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.
if(bool1) {
doA();
if(bool2) return value1;
doB();
} else if(bool2) {
doC();
return value2;
}
return value3;
How do you refactor this?
I haven’t used “else” in 2 years now. But first I wanted to talk about something “else”.
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
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.
Was it not closing off if statements with else what literally caused the 737MAX disaster? Everyone was talking about that like 2 years ago. the else is actually expected and funky stuff happens if its not there no?
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 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 agree with most of your early points ... but the level of oo is making me nauseous
I agree completely about extracting the methods and keeping indentation levels to a minimum. It is much easier to quickly identify what the code is doing without getting bogged down in details. The details are still there if you need to see them so it's a win-win for me.
The more I develop the more I agree. I usually use a little more flexibility than what he's describing, though. I didn't used to like extracting code out of methods into private methods that only get called in that one place, but the readability benefit is pretty nice. I also found that if I was doing it a lot in a given method, then it was time to possibly refactor that some other way, too.
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.
I am a noob and today I learnt that you can return in a void method to stop executing the following statements. Neat.
Very high level content. I am glad I found your channel, I was just annoyed by all these elementary videos about coding, design patterns etc... Your content is very detailed, and I can see your efforts to try to make new developers better by writing readable code! Thanks.
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.
Quick question for experienced coder.
Is there any way to write this shorter without repeating Y and Z condition twice?:
if((x=1)&&(y=2)&&(z=3)||(x=5)&&(y=2)&&(z=3))
(((x=1)||(x=5)) && (y=2) && (z=3))
default in switch case is "else" =D
Shhh, you’ll rouse the rabble in the insane asylum :p
@Nick: when you changed the to function/method call, ok, it's more readable. But most importantly, it is the right way to think about any algorithm. From the general to the specifics.
From the high level to the finer details.
Anyway, a pretty good video you have here, mate. What you gave us here is a nice rediscovery of the theory we learned at the U.
Well done. And keep going.
ps: also, in the refactoring, when you remove the else and place a return, you're helping the reader to read your algorithm. I mean, the method servebeer is ... to serve beer. so that's the "algorithm", the most important part of the code. It can run without the age check. Therefore, it's good practice to do what you did. It allows the reader to see 1: the main intent 2: the exceptions.
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.
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 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 ;)
Douglas Crockford said in his JavaScript: The Good Parts that don't use the 'continue' keyword as the removal of that always brings about better code. Any idea on that?
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).
Hey Nick...I also use Rider but it does not display any expand/collapse button in cs code (wherever it should)...kindly suggest any settings if exist