For those of you that are confused about me not using a return statement in the first example, it's because `log.Fatalf()` actually exits the program e.g.: ``` // Fatalf is equivalent to [Printf] followed by a call to [os.Exit](1). func Fatalf(format string, v ...any) { std.Output(2, fmt.Sprintf(format, v...)) os.Exit(1) } ``` Sorry if it made the example a bit confusing! ref: cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/log/log.go;l=417
If you have to explain what "Fatalf" does you have exactly that naming problem you talked about. "Fatalf" - tells nothing what it does - and why the ending 'f' ? A name like "LogErrorAndExit" would do the trick. Using exit() is very questionable, but that depends on the language used.
@guruware8612 😆 I didn't write the Fatalf function. This is part of the log package which is part of Go's standard library. It's a variation of Printf the only difference being that it will exit the program. Printf is somewhat historically understood from the C Programming language. The "f" is because it prints "formatted" output I left a link in my original comment. If you don't like the naming you need to talk to google about that one. And also some of the pioneers that created C
@@guruware8612 I prefer Rust's "panic!" :) I guess "fatal" implies that the program cannot recover, so it makes sense. But the problem is that in many logging libraries, "fatal" is just a log level like any other, and won't exit the app automatically. As for calling exit() on error, I think it depends more on the type of application than on the language. If it is a command line app that only does one thing (which this example kind of is), then it makes sense. But if it is something like a GUI, you probably want to display a message to the user and let them try again or something.
I agree with your point about calling exit() but actually this isn’t a real app, it is just artificial code I wrote to illustrate examples. The reason I used the panic approach is because adding error handling made the code too long to fit on the screen. Not to mention laziness.
Yes, your future self who will have completely forgotten having ever written that code. You may even switch on Git Blame just to figure out who wrote this garbage, praying that it wasn't you. The easier it is to understand, the less likely it is that your colleagues will start calling you for an explanation. And if they do it anyway, on a morning after you had a horrible night and can barely focus, you'll spare yourself the humiliation and headache of having to decypher your own code quickly and not understanding it either. Source: Guess.
In my opinion avoiding duplicate code is often taken too far. You avoid duplicate code if the code has the same reason to change, not just to make it look prettier. If you try to extract every inch of shared logic you’ll end up with high coupling and your code will be even harder to maintain
I approach it in the same way as normalizing a database (as it's effectively the same issue in the data domain as opposed to the logic domain). Normalization is best for the integrity of your code or data, but there are reasons to have denormalization as well.
DRY only applies to code where the "actor" is the same, as you explained. Code that happens to do the same thing, but has two different responsibilities according to SRP, is ok to do. :) If you end up using it in MANY different components, you likely need to abstract it into a util function, or you may need another business logic interface, depending on what this repeated interface does.
big difference between "accidental" and "real" duplication. I have seen countless time people abstracting "accidental" duplication, which in fact wasn't the same code given its context.
I've once seen a variable called qr_t_us_135_uss_t. The variabke name meant "Query result, Table, Users, Using Columns 1, 3 and 5, which are of type unsigned int, string, and string, and the whole variable is temporary". The codebase was a mess
1. don't deeply nested code and not using inversion; avoid deep nesting by inverting conditionals, merging related if statements, and extracting complex logic into separate methods or functions 2. organize code; prevent code duplication by extracting shared logic into its own functions. 3. don't use naming that only you understand; use clear and descriptive naming conventions to make the code self-explanatory and easier for others to understand.
For me some of the worst issues with #3 is if you try to change a variable name at a later stage and you called it x or y or such. If you "find and replace" it will replace every instance of x and y, and not just the variable names.
@@perrymaskell3508 IDEs have a language-aware rename feature that only replaces the name when it refers to the variable, so renaming single-letter variables shouldn't be an issue if you use an IDE
@@RegisMichelLeclerc Then the package you're using for IDE features should have a command to rename variables, like :YcmCompleter RefactorRename for YouCompleteMe
I'm leading a project which consist of migrating old PHP code. I found up to six levels of nesting, functions with bodies of more than 3000 lines (yes, three thousand), copy-paste engineering galore and many, many other issues. They blame PHP, but I blame the programmers, who even today still write very sub-standard code
It is possible to write good code in PHP, but 15 years ago, there were a lot of mediocre programmers producing bad PHP because it was so easy to get going in it and there were far fewer ways to learn what was good and what was not.
@@kristianlavigne8270 Do you mean that other languages like Rust or JAVA are inmune to high cyclomatic complexity, copy paste engineering, poor class architecture and poor typing?
In my Software class, we learned the following principles: Don’t repeat yourself (DRY) Comment where needed Fail fast (code should reveal its bugs as early as possible) Avoid magic numbers One purpose for each variable Use good names Use whitespace and punctuation to help the reader Don’t use global variables Functions should return results, not print them Avoid special-case code I think this video summarizes many of them!
@hungryhikaru Well, if let's say somewhere in my code, I have let costOfBag: number = 67.25 * costOfApple There, the "67.25" is a magic number because we have no information about it on our code (though it could be inferred). This a silly example, though. I remember working on a project that involved geometry and had to write various angles across the code, such as 72. The reason why magic numbers should be avoided is to make the code more readable and to show the relationship between the variables, so that the code is ready for change and easy to understand.
Astonishingly it leaves out the most important one! - documentation(or comments) There is no substitute for it. Even if you have taken an unusual approach to an unusual project, explaining it in plainspeak in the comments will make the code very easy to read(if only to your own self in the future) Documentation, modularity(also great for debugging!) and avoiding nested as opposed to linear thinking are probably the three golden rules
Astonishingly it leaves out the most important one! - documentation(or comments) There is no substitute for it. Even if you have taken an unusual approach to an unusual project, explaining it in plainspeak in the comments will make the code very easy to read(if only to your own self in the future) Documentation, modularity(also great for debugging!) and avoiding nested as opposed to linear thinking are probably the three golden rules
Watching this video I remembered collaborating with some friends on a school coding project years ago. One of my friends had a proclivity for writing very messy code, using variable names like "habbababa". I asked her on that occasion why on earth she would name a variable like that - and reportedly, "habbababa" sounded "kind of cute". I still find it hilarious up to this day.
Yeah, I've seen many programmers learning to code use nonsensical names, just because they don't care much about programming. If they got into programming more, they would use much much better variable names.
I used variables like that for a couple years. Well, not usually COMPLETE nonsense, just unrelated to their function. "makeToast", "batman", "spoooon", etc. I eventually got over it.
I did this on a school project once because I was the only programmer and the 3 other team members were "artists" (only 1 of them made all the art, the other two did lord knows what for the whole semester). I was super bored so I started naming things random stuff like "laughingLlama" and "pb_and_j" etc. It was actually a pretty cool platformer game in the end, but tons of copy pasta for the different levels.
2:46 I feel like the if statement that checks if it is a valid user does not need to be in its own method, as it is literally two lines of code. I get that making it into its own method would allow whoever is reading the code to instantly understand what the if statements are for, but another part of clean readable code is conciseness and being succint. If we are adding another two lines of code that doesn't actually much affect the readability of the code, then it isn't very necessary.
True, but let’s say you’re working with React, Vue of something. It’s better to turn your longer conditionals into computer state for better readability. In terms of executable code, I agree. It’s not necessary to create a small isValidUser func if you’re only using it once.
the point of extraction in this case is to define a new logic property for the entity user: whether it's valid or not. currently user is valid when they're authenticated and authorized, but who knows, maybe there will be some additional protection in future, like user will be required to have some special security ID, or restrictions like no banwords in username, not being deactivated or banned by administrator
@@zyriab5797 isn't calling a function like that defies the whole purpose? why write isUserAuthenticatedAndAuthorized() when you can write isAuthenticated && isAuthorized
@@semplar2007 It's more so that when new validation method is added such as TFA, modifying the function will automatically apply to the entire codebase. Edit: I misread you comment and wholeheartedly agree with you
The reason math is poorly understandable is not its inherent complexity, but the fact that mathematicians for some reason are all using single letter meaningless variable names
Coming from a math background I actually find having to using explicit variable names makes it hard to calculate more than using Greek letters. There's a topic about this on Math Stack Exchange.
hard disagree on that one, simple variables are way better for the kinds of things math uses, especially in the domains mathematicians would actually be working on. The variable names are short, sure, but the notation is very formal as way and, in general, the formalism plus conventions capture what matters fairly well.
Well, as you can see I don't have any actual argument. I can only provide information and my experience. (Or you can say secondary evidence, supportive argument, or whatever). From your arguments, then a consequence is that computer scientist must understand that meaningful variable names are better than abstract notations. Is that correct? But if you look at how they write math formulae, only notations are used.
This depends a lot on what you’re doing and how complex it is. If I had to work with differential equations using camelCased variable names I think I would end myself. However, it’s usually super helpful with physics or advanced engineering topics to stop, and take a moment to walk through the equation, keeping in mind what each variable actually corresponds to, or even saying it out loud. Except transport phenomena. Fuck that shit.
they use them because digits are single letters too. So the data is represented by the letters and the operations are represented by other symbols different from letters
Overuse of these principles (except the naming one) can also be a sign of an inexperienced developer, because they treat them like rules instead of guidelines and only applying when it really does make sense to do so. Overuse of extracting methods would be the most common one - creating a function for the sake of it when it's just a few lines of code that's only used in one place. If the name of the function helps to make the code more readable, this is where a comment would be more helpful than a function call. Bear in mind that calls have performance implications too.
Tbh context matters a lot. Even if it is used only once if it is kind of thing that would make sense to be used again as code develops then it still makes sense to make a function out of it. Otherwise you can get code duplication from seperate Devs rewriting same logic on seperate files. I will take over functionalization over huge monolithic files but I get your point, striking the perfect balance takes expertise
Avoiding extraction for "a few lines of code" is exactly the wrong thing to do. An ideal function will only have a few lines of code. If you see a few lines of code that perform an encapsulated task, it absolutely should be extracted into a function, even if it is only called once. Readability and maintainability are at least as important as reuse.
I agree 100% with rule 1 and 3. For rule 2, you have to be careful to not split things too early. This could lead to premature abstractions that don't work well in the future. Instead, don't be afraid to rewrite/refactor code early and often. So first keep it as simple as possible, and if some logic is user 3 times or more often, extract it. By then it should be clear what a good abstraction should look like. And because you're in the habit of refactoring early and often, you'd be confident to adjust the logic if the abstraction doesn't work nicely anymore later. So my top priorities for good code are: 1. Write code that's easy to understand 2. Write code that's easy to change
I think what people who abstract prematurely do wrong is they don't understand why they should make abstractions. The real goal of abstractions isn't to avoid code duplication, it's to increase separation of concerns. Instead of focusing on abstracting the HOW, one should focus on abstracting the WHAT. I think a reason for that misunderstanding is that many beginner programmers will only encounter simplified examples of abstractions like those in the video. They learn only that they should avoid duplicated code, but not the reason why. It reminds me of when schoolchildren first encounter equations in maths class. They could get an example like "2 + x = 3" and they wonder why the heck they should use equations since the answer what x is, is obvious. They don't get that the true power of equations isn't to find the value of x, it's to express a generalisation of an equality.
Extracing is a double edged sword. You now need to find the function, understand it, remember it and keep it in your head while youre trying to understand what follows after it. Also if its something simple then the function is pointless. The only reason to extract something is if youre using it multiple times. Also that first example for extraction it would be much better just having bool isValidUser = ...; if(!isValidUser){} right next to each other.
As long as the interface (func name and param names) are clear and meaningful, then this is not a problem because each lower level function should have been verified and tested, so you treat it as a trusted black box when debugging the higher level function.
You don't need to have intricate knowledge _how_ a function works as long as it's clear _what_ it does. (E.g. no one thinks how the regex method works in detail or how file read eventually makes a sys call, or what happens so that you get electricity out of the wall socket) Creating good abstractions is hard, and all are leaky, but they can still help to reduce cognitive load.
@@piff57paff _"Creating good abstractions is hard"_ I think that's exactly the point. It's very hard to consider all use cases and edge cases in advance. That's why abstraction shouldn't be a kneejerk reaction to make code prettier. It should be born out of necessity. When you know _why_ you need it it can be better tailored for its purpose.
Separation of Concerns is good but I feel like people sometimes take it too far. So you'll find a function which calls 3 other functions and each of those functions perform only 1 line operations. A rule of thumb I like is that I only extract functionality to another function if it's used in multiple places.
the overly nesting problem often comes (at least where i live) from the way coding is taught at university. i was taught that a function should have a single return, and early returns where a bad thing, so that resulted in a deeply nested code
There was a reason for having one return: cleanup code; Imagine the following function() { setupStuff1(); //Remember to cleanupStuff1(); setupStuff2(); //Remember to cleanupStuff2(); // now bury this into logic if (badPleaseStopNow) { return shitHittingFan; // Forgot to cleanStuff1(); and cleanStuff2(); } setupStuff3(); // Remember to cleanupStuff3(); cleanupStuff3(); cleanupStuff2(); cleanupStuff1(); return result } The single return makes it so you only have to check one "cleanup path"; I've actually found a bug in a friends' assignment "it just hangs"... It was a mutex locking... and unlocking if the function went to the end... but it had an early return inside a loop... which didn't unlock the mutex. If you are using a modern language with a modern library that usually isn't a problem because there are tools to deal with that: C++ has destructor methods Java has "finally" block Python has "with" block Go has the "defer"
At my first job we were doing the next version of a project for a customer. The old code had been written in ADA i believe, and ADA has some sort of facility where you jump to some end code, kind of like a finally or ensure section, and inside that section there was one return statement (I have never programed in ADA, this i what i was told). In the new version of the project it was begin coded in C and C did not have that feature, but the requirements to have one and only one return from a function remained, it led heavy nesting.
the logic inversion is so valuable specially for beginners, usually you create a lot of weird situations and temporal dependencies by not structuring conditons properly
Its personal preference, but i probably wouldnt create those tinty functions isValidUser and calculateTaxes. The first one you can clearly guess what its doing without having to create the isValidUser func. The second one, the file is probably named tax calculator service or the function instead of "main" is already probably named "calculateTaxes" or something like that. The base function is already short enough and easy to understand, i prefer reading it sequentially than having different layers of abstraction by creating tiny functions like those. You already know what the base function will probably do because u previously read the filename or the base function name. If the base function was larger or more complex, then yes, i would probably extract some functions tho. Great advices btw!
The "merge related if statements" example you gave is in fact a bad advice. Authentication and authorization are different things. You are not just "losing granularity", you are shooting your foot with a wrong log message. You'll waste time trying to solve an authentication issue thinking it's an authorization issue.
Yeah! Additionally, it makes it difficult for the programmer because if they have to change the authorization code, they would also have to touch the authentication code (or at least, it would look like it for someone doing a diff on the code). A function should ideally just be responsible for doing a single thing.
@@kantancoding The user validation could actually be extracted functions like "requireAuthenticated" and "requireAuthorized" that both throw their own errors. The authorized function calls the authenticated function, and the main function function only needs to call the authorized function.
I’m luke warm about the DRY principal. It only makes sence if the spun out code can be understood in isolation. If you need to see where a function is called to understand what it does then the code is actually harder to understand.
I agree, but that's a misunderstanding or misapplication of the DRY principle. The DRY principle doesn't say that you can't duplicate anything and that you have to chop up everything into small fragmented pieces. One function should do one complete thing. If the duplicated code looks the same, but do different things for different reasons, they shouldn't be in the same function. A function shouldn't rely on another function or some external context to make sense. That is a poorly defined function.
I'd agree in general, but in the specific case used (unauthenticated vs unauthorized user), it's often best -not- to split them out. I don't want people "fishing" for valid usernames! That's why so many login screens give errors like "username & password do not match" or the like.
I see some people complaining about the code extraction. In the example, I would do a similar extraction, but for another reason. Avoiding code duplication is a nice side effect, but the core idea should be: *Keep the abstraction level of a function or class or whatever constant.* An example: You don't want a function dealing with structuring a report page doing regex stuff, this is clearly too low-level. Think about how you would explain your function with a few words to your colleage: "I look if there is a report header, else I use the default, then I add the body, and if there is a footer, I include it as well". And that's what the function should do. If it does some finicky stuff with the body, extract it. If it does some regex, definitely extract it, maybe even a level deeper, below the functions dealing with the header, body or footer. Having one abstraction level is much more important for readability than to avoid code duplication.
That makes it hard to do anything beyond simple changes. I would usually agree that it's important to avoid mixing levels of abstraction, but I never thought someone would ever think that using a regular expression at high level code is a violation of that, regular expressions are not part of the hierarchy of concepts for a page
@@kantancoding This is is. There's a type of "professional" developers out there who complain about TDD (TDD and clean code go hand in hand imo) being so much overhead but wasting so much time debugging. I've seen people in enterprise context preferring to wait for minutes for the app to startup and testing every single iteration of their changes by hand instead of adding a little bit of code and refactoring and automating this process. LiveUnit-Tests give you feedback in seconds and in the long run you're so much faster with TDD. Seriously, I'm doing TDD now for a long time and I hardly need the debugger anymore. Tools like NCruch even show you the code paths or hot spots in your code these days. Using the debugger for me is mostly a sign that I'm working with ugly or untestable code.
It’s worth noting that the first example refactor changes the program behavior- they are not equivalent. It needs additional return statements in the guards .
Actually that’s incorrect. It doesn’t need a return because log.Fatalf() will exit the program. It’s the same behavior as before the refactor. If you're curious: cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/log/log.go;l=417
@@kantancoding Good to know! I’m clearly not familiar with Go 😅 So for a more general refactor recipe one would basically make the (implicit) returns in the "leaves" of the control flow tree explicit before applying inversion and remove redundant ones afterwards.
On one hand it could be expected logging a "Fatal" message should exit(1) but in some systems allowing a logger to end the program could be seen as bad design or dangerous.
Yeah I was also thinking that he committed a much worse coding crime than having some deeply nested code. As someone who troubleshoots other people's code in Prod a lot, never ever do this and log incorrect errors.
TL;DW Use guard clauses to check state for on/off condition code, switch statements are great and can be used for simplifying the if, ifelse, else chains, enums are also useful for conditions that are always going to be of a set type Reduce duplication by moving common code with similar purpose into their own methods. Single lines don't need to be methods, multiple line duplications probably should be moved unless they're inherently dynamic like DB calls. Make clear variable names for everyone to figure out what each variable is, single and double character names aren't helpful for describing what your variable is in most cases.
Great examples! You earned yourself a new subscriber! Back some 30 years ago, I noticed one programmer on the dev team consistently broke our coding conventions. Drop downs in the IDE listed methods in alphabetical order. He prefixed all methods with letter = A to ensure his methods for all devs always showed up first in the IDE. Ego driven! Next, any local variables he created were named Anna, Bertha, Charlotte, Denise, Ellen, . . . I asked why? His answer: To ensure only he would ever want to maintain the could he wrote - to ensure his continued employment. Sometimes developer decisions can have effects way beyond the next code check-in.
lol I don’t understand the argument about making your code obscure so other devs can’t maintain it. There’s definitely always somebody smarter than you who can figure out what the code is doing and refactor it. Also, this guy seems insane 😂
I really need to say this. The advice to never nest your code has been repeated for years now, it's the same thing that I was told 8 years ago when I was reading about how to write VB code in highschool and it is a good concept, but It's important to note something. In the provided example, yes, I think the code is easier to read, but it does not change the conditions you need to keep in your head. If you are doing more complex logic that then relies on the state of those conditions, you still need to know that those conditions are not true for example. Take authentication. If you have middleware the runs in an endpoint, or your function verifies that the user is authenticated before running, you still need to know that condition has been met, otherwise you will end up writing redundant code in your actual logic to ensure that the user is authenticated. Whether your nest or don't nest the condition, you need to be aware of the state your app can be in at that point in the code, and both ways require you to have this awareness. I agree that in most cases, early returns are better, but just wait until you have a function that has a structure like this and tell me it isn't confusing AF if (A) return // logic if (B) return if (C) return // logic if (D) { //logic } else { //logic } if (E) return //logic The code looks readable, but damn, it can get confusing, especially if the code I put as "//logic" can potentially return, therefore adding conditions that aren't even ovbious. Sometimes the real solution in my opinion is to double up your conditions just for understandings sake, or in a complex case, abstract where necessary. The idea of code inversion is very primitive and does not always apply
I'm surprised this video didn't mention comments. At 2:05, couldn't you instead put comments on what the code segment does instead of making it a small, one-use function? I think it would be harder to go and search for function definitions.
could have also put the condition into an aptly named variable if it was so badly needed. whenever i find something like this in a code review i tend to immediately reject it, because stuff like this make the the code as a whole harder to navigate. imagine if every single condition longer than a single word were to be its own function - the whole thing would fill up with one liners that each take up 5 lines instead. i'm not braindead enough to be stumped by a single AND statement, i'd much rather keep the code concise.
Great summary of the most important concepts that everybody should follow in a team. Nested, unreadable code like your example is a perfect breeding ground for bugs, often missing edge cases or special cases. It's always worth taking a closer look at such code.
Saw the video until 2:42 instead of one single function that I can read and understand I’ve got multiple functions, multiple contexts, you forced me to remember what’s going on in all functions, and a simple code becomes unreadable. Bravo
@@kantancodingNo, what you presented is terrible advice. One should only extract logic into its own function if it is used multiple times. There is no benefit to extracting logic, especially if it pertains to the original function.
Sure, we’ll just put everything in main() if it’s only used once. No need to logically structure our applications. And the comment you are responding to is completely unrelated to what you’re saying man. I’m all for opposing arguments but your argument is useless since you don’t give any type of explanation of why or how you came to that conclusion.
Law nr.4: Always comment on what something is doing. Not only will this allow you to remember why it was made in the first place, but it can help anyone else who needs to look over your code. This can also help them find a function, a code block, or array they might need to change.
There are a lot of Videos, on general readable code, but I would like to see one, which gives you tips, for structuring a whole Project, at what point to use Classes or what valid to put in Arguments or write a whole new Function for etc. Good Video tho :)
Please correct me if I am wrong. The inverting looks good but I don't think it is that simple. In the given example, multiple conditions are needed to be checked to execute the nested code. If you are going to make it linear, you will have to use logical operators like &&, || to make sure that the condition remains same. For example: at 0:39, "user not authorized" will be logged if ( ! isMaintainancePeriod && isAuthenticatedUser && ! isAuthorizedUser ). On the other hand, at 1:21, "user not authorized" will be logged just by ( ! isAuthorizedUser ). Here, we have not made sure that isMaintainancePeriod is false and isAuthenticatedUser is true.
You're not at all wrong, the logic is entirely changed. Personally I've come across a lot of these "laws" by folks like Robert Martin and I'm just not a fan. I don't think most of them are that critically helpful.
No... we have made sure that isMaintenancePeriod is false by exiting the function if it's true. Rhe authorized user logic did in fact change but that's not because of the inversion.
Changing my habit of writing code is better for the next developer to continue my work. But what will be more valuable for me is to be able to understand the messy code that someone else has written with bad patterns.
I really like how Verilog has freedom of the switch-like case statement to the point of "reverse case statement". E.g.: case(1'b1) isMaintenacePeriod: log.Fatalf("feature unavailable"); !isAuthenticatedUser: log.Fatalf("invalid user"); !isAuthorizedUser: log.Fatalf("permission denied"); default: doWork(); endcase SystemVerilog even adds features like unique and priority.
Point 1 is good. Point 3 is a bit of a dramatic example IMO but it does happen 😅 and there's no such thing as perfect names, so be sure to write good tests and documentation anyways. Point 2 though - "Avoid duplication" is I think the wrong advice to be giving, it's missing the actual problem. The problem is poor abstraction, or in other words a lack of / unclear / poor separation of concerns. Lots of duplication is simply a symptom of poor abstraction. Your problem is in your design choices, not your duplicated code. Software design boils down to asking the question "should this code be shared or not" over and over again. By saying "avoid duplication" you imply the answer to that question is always "yes" when in fact that's not going to give you good results. Sometimes duplication is the right answer. The key question, where you want to focus your intent, is on where and why code is shared, and whether it should be or not. Don't simply avoid duplication, that's a shortcut that will lead you into poor design choices and will prevent you from improving as an engineer. This is the exact reason why the calculateTaxes extraction from the first example is a bit iffy - I understand it's a bit contrived on purpose, but it clearly illustrates the problem of preferencing "cleaner" code instead of making a thoughtful abstraction, because the latter is what really matters. The developer's intent is in the wrong place, i.e. "perceived cleanliness" instead of thoughtful, intentional, useful abstraction. The most effective way to produce good abstractions is through strong TDD practice. As you improve your practice the best abstractions will start to become obvious, and you'll stop thinking about code duplication and "cleanliness" altogether, because it turns out to be somewhat irrelevant in the quest for "good" software. --- I'll throw in a Point 4 for free, don't use double negatives in conditional logic. This feeds into data design, too, naming a field disableTooltips just means later someone has to write if (!disableTooltips) { enableTooltips() }... It makes logic just enough harder to read to be not worth it. This is a simple example but add a clause or two or a few indentation levels and the cognitive load adds up quickly.
I probably agree everything you said. The point 1 rewrite is good enough and I think the rest of the video is kinda pointless to the original problem. After the first one, like merging conditions into one, everything becomes a readability preference, it doesn't affect the readability and you lose the log difference and the fine details. The point 2 (convert multiple conditions into isValidUser) is purely wrong in my opinion, it's pure premature abstraction without good reasons. Unless you told me that isValidUser is fairly common and used in many spots. Last point is just pure assumption of what your code will be in the future, the reality is that 90% of the time your predication will be wrong. Had seen countless cases where people/colleagues rewrote their pre-imagination refactor/abstraction over and over, which can be avoided at the really beginning if they kinda listen my suggests. Also, reduce my PR review time. So instead of trying to do the abstraction early, leave it like that is totally fine. If you need to use many "what if" for an abstraction, likely you shouldn't do it. I felt like the whole video falls into the trap of premature abstraction after point 1 unless the author didn't include those context in the video.
@@doc8527 Yeah I totally agree with you. I also don't think deep nesting is a sign of an "inexperienced developer" necessarily, but premature abstraction all over the place is definitely a strong sign of inexperience 😅
@@nijolas.wilson Hmmm I feel like you both are applying the wrong argument to what I'm trying to explain. I'm not adding abstractions to the code based on some prediction in the future. I'm simply writing each function so that it does one thing and only one thing. There's a difference between: ``` package main func calculateTaxes(product product) { switch product.Type() { case "alcohol": total += alcoholTax case "electronics": total += electronicsTax default: total += generalTax } } func main() { for _, product := range cart { calculateTaxes(product) } } ``` AND ``` package main type calculator interface { calculateTaxes(product product) } // implements calculator type taxCalculator struct{} func (tc taxCalculator) calculateTaxes(product product) { switch product.Type { case "alcohol": total += alcoholTax case "electronics": total += electronicsTax default: total += generalTax } } // "constructor" func newTaxCalculator() calculator { return taxCalculator{} } func main() { tc := newTaxCalculator() for _, product := range cart { tc.calculateTaxes(product) } } ```
@@kantancoding Sorry, I didn't mean to imply by my response that I think your example was premature abstraction, or that I think you are an inexperienced developer 🙏 that was meant as a separate commentary. I agree, and to me it's very clear the point you were making. I don't think that deeply nested code is a sure sign of an inexperienced developer, but I also don't think what you were showing was premature abstraction. 🙏
yeah and on the extraction rule of thumb: 3 to 5 actions, which produce 1 action when combined. e.g. function is "cook potatoes" actions would be "peel potatoes", "chop them", "choose a way to cook them", "cook them the way you chosen to". if you can split an action into more or equal amount of actions you should do so. e.g. ",cook potatoes the way you chosen to" can split into "get the chosen cooking method", "prepare for cooking" and "do the cooking". you can split actions into more actions into more actions. they only need to result in one concrete thing and be semantically similar. like e.g. when we cook we will do things related to cooking. we will not focus on arbitrary actions like "take the knife in your hand" if it isnt obvious how we will relate that action in the context of splitting another one ("take the knife in your hand" and "make a chicken stock" arent obviously related, imagine if we dont have to chop anything why did we take the knife then?)
why 3 to 5 becsuse human brain ram is approx 5-9 statements depending on the person so youre always within your brain's 1 stack frame (maybe even 3 if youre lucky). thats easier to reason about what
Law zero of readable code is to comment abundantly and always keep in mind that the reader might be... future yourself. I don't know about Python, but Perl, Java and C can be deceptive... Blocks of code? Tell me about looking for a bug in a regular expression just 6 months later in a function called from pretty much everywhere in hundreds of lines of code (i.e. a small program). Also, do abuse functions/procedures and merge them to the main code (with lots of comments!) only at the very end when they're called only once (just get the xref to get the list). The thing about code being unique but called from multiple locations is a well known paradigm of "proper" databases: each piece of information is semantically unique, otherwise you'll pay that in code and data size, and each chunk of data is identified uniquely. These are base rules, but everything else derives from that, you'll think of me when you need recursivity and callbacks: pointers on lambdas are a thing...
Yeah, documenting is a must. When working on multiple projects at once, as one often does when programming as a career, makes it increasingly difficult to remember what a particular piece of code does. I think I was lucky in this regard when I first learned programming. I was born with a disability that among other things has affected my memory. As such, I have always commented my code a lot, out of sheer necessity, because if I don't, I forget what the code does the next day (heck, some days I wouldn't be able to remember over the lunch break...).
I'm a software engineer of 15 years and my main thing is that code is not for the computer. It's for the person reading it. Just think "is this code understandable for person that has no context" you write so much more clean code.
problem in second law is, when trying to avoid duplication, I also need to do transaction in SQL, and every transaction function always differ from other
Every law has its edge case. The point where it divides by zero or reaches infinity or whatever. Just adapt. A law is just a guideline, not something to follow religiously like a robot.
This is exactly why Chris McCord make significant change in elixir 1.12 to 1.13. He change 'Model' in MVC to 'Context', partly because people just keep using the generated Model to talk to db without using proper transaction. Every transaction SQL will different from each other. Race condition is fine if you just want to make small blog, but not in banking app
There is also a "five finger" rule. The origin of this rule was back in the days when code reviews were done with printouts. Reviewers would identify the entry point of the code under review, then use fingers as bookmarks to walk through the program logic to evaluate its correctness. This agrees with the 5+/-2 rule of human short-term memory, which effectively means that if there are more than about 5 nested function calls, the reviewer would lose track of the logic, and the time and accuracy of the review quickly deteriorates. Although there are code inspection tools that can search for symbols and keep a stack of examination points, the 5+/-2 rule is a limitation of the human reader and remains an issue. Executing these three laws also serves to reduce a metric called Cyclomatic Complexity. This is a measure related to the number of branches and loops in a piece of code (including short-circuit operators), and it's interpreted as a measure of the likelihood that a section of code contains a bug. It's often used to guide code reviews and testing. It reinforces the notion that code that lacks complexity is both more correct and easier to inspect.
The first one is mighty if you use it with de morgan's law. It can also be an optimization if "isAuthenticated" and "isAuthorized" are packed in the same register or data in cache. In that example you might want to keep the granularity for log messages. But in machine control you might have a bunch of conditions that would force your machine to stop if one is false. If i had to add one rule it would be keep the same flow control statement for the same type of task. If you keep the number of language features low and the syntax similar it's much easier to understand the problem the code is solving.
The first example does not make the conditions any less relevant, or further out of mind. It’s just easier on the eyes. I hear this incorrectly described too often.
In fact, the first rule is a design pattern named Early return. Which can be refactored to Chain of Responsibility in more complexe situations. Both second and third rules made me think about respectively the third and second rules of the 4 rules of simple design from Kent Beck.
As someone who only got into learning C quite literally last week, I'm shocked and awed at how few comments (pun unintended) there are about commenting, and that it wasn't even mentioned in the video. The amount of times I've searched on stack overflow to try and find a solution to a problem I was having and only understood what it was I was looking at when the poster explained what the code did afterward...comments are an ACTUAL godsend for anyone trying to read your code!
One thing I tend to prefer in my code is avoiding the ! operator in if statements. Sometimes it can be missed by someone who is going through the code quickly and therefore I tend to try and make all my boolean variables describe what the condition is. Of course don't go out of your way. Sometimes a quick ! is better than an extra line of code to flip/rename a boolean.
I was on your side until you talked about the functions. Don't get me wrong, functions are useful, however, functions which only have 1 return line or less than 5 lines altogether, are unnecessary and using those would require the reader to potentially jump around in different files to find what the main code is doing.
This is quite good, and as someone who’s been teaching programming for 40 years, you’d be amazed at how many college instructors will teach exactly the things you’re saying not to do.
2:06 While extraction is good in some cases, in this case that is a step too far. Code is easier to reason about if you keep basic operations inside the main function. You should only extract large chunks of code into their own inline functions. They should be inline to start with because you have no reason to take external code doing the same thing into account until it becomes common enough. Somewhere about 3-5 usages is the minimum amount for this, depends on the complexity of what is being done. In the case of one liner expressions extraction is in fact incredibly poor developer behaviour. If the code spans at most just a few lines (3-5) then an inline function is all you need for it. If it spans 5+ lines then it's a candidate for a shared function to avoid mistakes in logic due to complexity.
The argument that extraction should only be used if the code spans a certain number of lines doesn't make any sense to me. Why some arbitrary number of lines? Better to have a real reason regardless of the number of lines. The reasons are as follows: - separation of concerns - modularity for testing - reduced surface area when debugging - readability - enables us to test individual behaviors in isolation - the above mentioned tests have the added benefit of serving as documentation for how each individual functionality should work
@@kantancoding All those reasons are auto-covered by the line count. The more lines the more context you're trying to keep in mind when looking at the whole function. 1 to 3 are easy enough but it get's a bit unreasonable from there on. However jumping straight 2 a shared function when extracting is not always a good idea which is why I say use a local inline function. Also for many people, both newbs and devs, a line count is easier to understand or even remember than a bunch of more complex rules. You can simply reference those rules as the reason for the line count. At bare minimum the line count helps identify code that should at least be considered for extraction. It's like using if ( line_count >= 3 ) extract = should_extract( lines ); You're simply skippimg the complex stuff until the line quota is met
OMG. I love how your refactoring of code actually changes the logic and you somehow breeze right over that as if that's not kind of important??!! I think I need to make a video about rookie developers doing TH-cam videos. 🤣😂🤣😂
to me one thing i've learned is 1) make sure functions can run without relying on global variables this to improve testability 2) all false or invalid states should be on top most 3) nest as few as possible. 4) don't apply abstraction too early until you actually know you will be repeating it this so you don't end up with 1 time use functions 5) avoid iterating a database rather turn it into a function from the database
Usually it's better to sacrifice performance for readibility. This is where CS programs set up their students for failure when they get to the real world. The thing you really need to optimize for is Mental RAM. Engineering time is expensive. Writting code that's easy to understand and refactor and is tested will save engineering time in the long run.
So true. And increasing computer processing power is generally much cheaper than increasing human processing power. :) It's better to let the machine read one more line or one more byte than having the engineer waste a day trying to understand their colleague's messy code.
It's supposed to be, so you realize it. (Can't tell how much of this is sarcastic, I know I know, always assume someone online is sarcastically wrong...)
You seem to have completely missed the big backlash against DRY. The biggest tell that someone is a junior programmer is when they start creating premature abstractions to avoid code duplication.
I think this was just an easy example of the concepts. If this was the total extent of the program, I would agree with you that extracting the code into separate functions is a bit excessive. DRY is good; the problem isn't that junior programmers abstract prematurely or too much, the problem is that they try to abstract the same literal code, but from multiple contexts, which will cause problems later on when they want to make changes to the extracted code. But if one make sure that the abstraction also is semantically the same, not just lexically the same, the DRY principle should be good. I feel the same way with the inheritance backlash. In my experience, most of the criticism comes from misuse of the technique, not from the technique itself. Same with OOP, a lot of criticism comes from either lack of understanding what it is or plain old misuse.
variable naming has the problem that it dependes of the program model inside the programer's brain , you must choose names that make sense in context of what are you writing. " Future readers of your code " will never have the same brain model until they read enougt code . Java fully qualify names is the solution , and i hate it.
That's why, in my opinion, a variable should be short-lived; preferably only be used once and also be defined as close as possible to the code that uses it. I never reuse variables, I treat variables as write once, read (preferably) once. I hate it when reading someone else's code and they have defined a variable with some dubious name on line 10, and then first use is on line 46, it's redefined with a new value on line 51 and finally used in a totally different context on line 123.
the switch case does not need to be in a separate function, while you can modularize anything, just having a separate function for the validations and the logic is a good amount of modularization with reasonable "locality of cose" so the un-necessary navigations are not needed!!!
The only thing I have a problem with is the if statement in the first code. Ie; ! isAuthenticatedUser || ! isAuthorizedUser First of all I would be really wary of putting the both conditions into one if statement. The reason being clarity. We can only use one error message the way you recode it, which could work but it limits how descriptive the code is. Second thing is, that by having the if statement in its own method, you don't make the code better. It doesn't make it shorter or more readable. I would even argue the opposite for both. The code is longer and you have to search the code for the method which could be just there. The conditions are descriptive so there is no problem there. Of course in such a short code it doesn't change much, but when you are working with longer, more complex code, you would only make it harder for others to understand the code.
One other thing you can do is maintain objects so you can use the 'if' statements wherever you want, to check for the current status of the block of code you are running. Sometimes it won't work but most of the times it will work.
gotta admit, did some of the nesting 'mistakes' you mentioned, noted it under great advice, for naming i find it the best to write the convention used in a comment at the start of the file. not sure how helpful it is to some people but since there are a few i wanted to make it easier for potential other programmers
@@damianjblack I have learned some Pascal (and now remember almost none of it, didn't ever really use), but my first language was Python Also I remember writing begin on a new line smh :) I was generally a terrible coder back then
@@notyourfox I have to admit I love Pascal and my main hobby project is in FreePascal. And yes I still put BEGIN on a new line for some of my code. IFs and loops not so much but usually for procedures and functions.
Me and my buddy like to make fun of naming conventions and terminology while we work together. The first rule would have been termed to 'hire a bouncer/hire a guard' - in order to write the subroutine exits at the top instead of dangling elses. Interface Connectors are called 'Handshakers'. We are writing code that are full of insiders. Also i've learned to better name functions as getOneUser/getManyUsers instead of getUser/getUsers due to quick readability
Not being overly enthused by silly 'guard clauses', I like your functionnaming scheme of and (please note the application of capital(s)). I myself use a simmilar scheme of _ or even _, like 'Get_Object()' or 'Read_Line()' or 'Show_GrandTotal()': all for the benefit of readability...
I’m personally not a fan of some of those function extractions. If I were reading the code, I might not know exactly how calculateTaxes works, but if instead you added a comment saying “// Calculate taxes”, I’d know exactly what you intended the code to do and also exactly how it does it.
With extraction, the general idea is "Either you are working on the program's general functionality and you don't care about the implementation; or you are worrying about the implementation and don't care about the overall flow" so you separate those two things by tucking certain details into its own part of the codebase. With many IDEs you can simply control+click the function you want to know more about and it will jump you right to it!
Wow finally somebody who knows how to go to function definition. Another point, with calculate taxes extracted, you can now write unit tests to test that functionality in isolation which will have the added benefit of providing documentation about what calculate taxes does if you really need to know. Boom, no need for excessive commenting
I would argue you probably don’t *need* to always know what every piece of code does at all times. And if you do, you can easily go to it. If you are debugging, editing or expanding this function you can easily follow what’s going on and where to add/edit the logic without reading the entire implementation line by line
@@zachmanifold Yep. It would be a pain if all code for all standard libraries and such would be inline. A simple Hello world program would take several sheets of paper.
Good video. I don’t agree with extraction though if it’s not to handle duplication, it’s enough to leave a comment to say what is happening, jumping functions in most cases can be confusing as hell if it goes too far off, also it’s not good performance vise to extract code to functions, so just better leave a comment
One word of caution, you should never default to valid with authenticating a user. So, that's one area where allowing the code to drop through when by checking !valid instead of valid will get you into trouble.
Law #4: never right negative properties. The name of your property should be something such that knowing what the default is is very obvious and straightforward. If you find you are putting the word not in your property names then you have it backwards. Example: DontProcessX. In this example in order to process x you would need to pass false. You're much better off with a property called ProcessX
Regarding clean code: placing the opening containers at the end of the line, which is the case for every if() and for() statement of these examples, is NOT clean code and is, therefore, NOT readable code. The nested if() statements are made much harder to track because the formatting is unnecessarily compact. Frankly, the nested if() statements with the open container at the end of the line is less code structure and more ASCII art. Please stop showing young programmers your ASCII art!
I do not entirely agree with the second law. Having code duplicated is completely fine while being wrong at the same time. By that i mean, taking your example, let's say that a new business rule is introduced for the function that iterates over the users list. It's specific to that use case only. Now you have to mix up logic from two different use cases. What if that function needs to be re-used on a third use case? What if it has something specific to that use case? In most of the cases I would say to follow the WET (Write Everything Twice) principle and unless you have the same exact lines of code in more than two places i don't think there's a real need to move something into a shared space. Good video overall :)
You forgot one rule: COMMENT YOUR CODE... No seriously, it takes like 5 seconds to do and makes any and every piece of code much easier to read and understand. So please, for the love of god, comment your code.
I try to use comments to explain "why". If I feel like I should explain "how", it means the code is not readable/explicit enough. Besides, comments tend to "expire" during refactoring and updates, especially when working in teams.
Excessive commenting is useless and actually not good practice in my opinion. Not to mention, if you write unit tests you are actually documenting how the code should work so comments should really only be needed on rare occasions.
No. excessive comments is a smell. Code should be self documenting by being readable, and only in rare cases comment if there is some necessry tricky logic.
The variable naming is rather important, even if you want to read your own code months or years later. The other advice is great for readability, but can slow things down a bit with extra function calls. Most software today probably doesn't need that kind of speed up, but some things could. So just remember, every function call adds a go to subroutine, which will push the current instruction pointer to the stack, then jump to the subroutine, and when done, it will have to pop the previous instruction pointer back off the stack, and then jump back. The key is understanding the requirements, it's easy of maintainability verses number of instruction cycles required.
I checked out this video to see if there was anything off but this was well explained. Nice work. In JS, eslint can be used to check deeply nested conditionals. Maybe go over something like that
My advice is to put lots of boolean statements into one integer, and compare the resulting integer with the combination you need for code execution. Example is calling an integer "status", defining 3 as admin, 2 as trusted, 1 as worker and 0 as unauthorised. Then you can make switch status, case admin, trusted etc
about extraction. in first case with taxes, the extracted function is used only once, and it gains readability only by its name. so why not just to add a comment?
In teaching, i'd recommend mentioning the titled names of the 3 'laws' in the end of the video to help brains retain the information. Second, I recommend after making final adjustments to code explanations that you give a side by side of original and final. This helps brains understand structure, just like the first 'law' is alluding. This would elevate your vids from good to fantastic.
Awesome tips! Thank you, I will try to incorporate them :)
5 หลายเดือนก่อน +2
Your 'calculateTaxes' is not a function. So from the point of view of the main code, it becomes a blackbox that can be full of surprises. So it doesn't make anything clearer to have it.
I'm a junior-ish dev in a semi-low-code integration system and I find myself struggling with 1. Granted, in my context it's not always easy to break things down into steps because declaring a local variable inside e.g. a data mapping isn't really allowed the way I'd want to (the "do" block is supposed to allow that, but it's not _quite_ the same -- especially because it is _itself a block,_ so only worth using if you can break down many steps with many intermediate variables), but still, I often end up with a long chain of function calls that are combined all at once with no easy way to rewrite. I do wish I had the skill to avoid that, but my platform doesn't make it super easy (or maybe I just suck). RE 2, I recently had to work with a codebase that managed to invert DRY's use in places. Things that you would _want_ to extract and unify into one piece of code were duplicated... but some logically different use cases that _just happened_ to share the same code were unified. Having to work on one of those use cases, I almost broke the other. (Happy to say RE 3 that I have not yet encountered single-variable names. Yet.)
![[TH] 2024 PMGC League | Survival Stage วันที่ 1 | PUBG MOBILE Global Championship](http://i.ytimg.com/vi/FVFT-PFwU80/mqdefault.jpg)
For those of you that are confused about me not using a return statement in the first example, it's because `log.Fatalf()` actually exits the program e.g.:
```
// Fatalf is equivalent to [Printf] followed by a call to [os.Exit](1).
func Fatalf(format string, v ...any) {
std.Output(2, fmt.Sprintf(format, v...))
os.Exit(1)
}
```
Sorry if it made the example a bit confusing!
ref: cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/log/log.go;l=417
If you have to explain what "Fatalf" does you have exactly that naming problem you talked about.
"Fatalf" - tells nothing what it does - and why the ending 'f' ?
A name like "LogErrorAndExit" would do the trick.
Using exit() is very questionable, but that depends on the language used.
@guruware8612 😆 I didn't write the Fatalf function. This is part of the log package which is part of Go's standard library. It's a variation of Printf the only difference being that it will exit the program.
Printf is somewhat historically understood from the C Programming language. The "f" is because it prints "formatted" output
I left a link in my original comment. If you don't like the naming you need to talk to google about that one. And also some of the pioneers that created C
@@guruware8612 I prefer Rust's "panic!" :)
I guess "fatal" implies that the program cannot recover, so it makes sense. But the problem is that in many logging libraries, "fatal" is just a log level like any other, and won't exit the app automatically. As for calling exit() on error, I think it depends more on the type of application than on the language. If it is a command line app that only does one thing (which this example kind of is), then it makes sense. But if it is something like a GUI, you probably want to display a message to the user and let them try again or something.
I agree with your point about calling exit() but actually this isn’t a real app, it is just artificial code I wrote to illustrate examples. The reason I used the panic approach is because adding error handling made the code too long to fit on the screen. Not to mention laziness.
@@guruware8612 it's a standard library function dude. Combination of being pedantic and clear unfamiliarity with the language.
And remember: "Potential readers of your code" is more than likely going to be YOU in the future.
Very important point indeed
Future you, is always the one you should feel the sorriest for.. every time.
Haha I wouldn't like to be that guy, poor man
Yes, your future self who will have completely forgotten having ever written that code. You may even switch on Git Blame just to figure out who wrote this garbage, praying that it wasn't you. The easier it is to understand, the less likely it is that your colleagues will start calling you for an explanation. And if they do it anyway, on a morning after you had a horrible night and can barely focus, you'll spare yourself the humiliation and headache of having to decypher your own code quickly and not understanding it either.
Source: Guess.
I am thanking myself of 5 years ago for doing well self-documented code I completely forgot about.
In my opinion avoiding duplicate code is often taken too far. You avoid duplicate code if the code has the same reason to change, not just to make it look prettier. If you try to extract every inch of shared logic you’ll end up with high coupling and your code will be even harder to maintain
I approach it in the same way as normalizing a database (as it's effectively the same issue in the data domain as opposed to the logic domain). Normalization is best for the integrity of your code or data, but there are reasons to have denormalization as well.
DRY only applies to code where the "actor" is the same, as you explained. Code that happens to do the same thing, but has two different responsibilities according to SRP, is ok to do. :) If you end up using it in MANY different components, you likely need to abstract it into a util function, or you may need another business logic interface, depending on what this repeated interface does.
big difference between "accidental" and "real" duplication. I have seen countless time people abstracting "accidental" duplication, which in fact wasn't the same code given its context.
@@Scroapy that’s how you end up with a bunch of parameters that dictate nested branches in a function.
@@TehKarmalizer exactly
I've once seen a variable called qr_t_us_135_uss_t. The variabke name meant "Query result, Table, Users, Using Columns 1, 3 and 5, which are of type unsigned int, string, and string, and the whole variable is temporary".
The codebase was a mess
Hungarian notation taken to the extreme
How would you improve that?
@@ooker777 just don’t, it’s not worth it
@@Jonas-Seiler you mean trying to have that variable at the start is a crime? But then how do you process the data that is that specific?
@@ooker777 write a comment
1. don't deeply nested code and not using inversion;
avoid deep nesting by inverting conditionals, merging related if statements, and extracting complex logic into separate methods or functions
2. organize code;
prevent code duplication by extracting shared logic into its own functions.
3. don't use naming that only you understand;
use clear and descriptive naming conventions to make the code self-explanatory and easier for others to understand.
For me some of the worst issues with #3 is if you try to change a variable name at a later stage and you called it x or y or such. If you "find and replace" it will replace every instance of x and y, and not just the variable names.
based comment
@@perrymaskell3508 IDEs have a language-aware rename feature that only replaces the name when it refers to the variable, so renaming single-letter variables shouldn't be an issue if you use an IDE
@@seanthesheep my IDE is made of bash and vim...
@@RegisMichelLeclerc Then the package you're using for IDE features should have a command to rename variables, like :YcmCompleter RefactorRename for YouCompleteMe
I'm leading a project which consist of migrating old PHP code. I found up to six levels of nesting, functions with bodies of more than 3000 lines (yes, three thousand), copy-paste engineering galore and many, many other issues. They blame PHP, but I blame the programmers, who even today still write very sub-standard code
It is possible to write good code in PHP, but 15 years ago, there were a lot of mediocre programmers producing bad PHP because it was so easy to get going in it and there were far fewer ways to learn what was good and what was not.
I've had to refactor 12 levels of ifs (that's not counting loops/try/class)
Typical for PHP and Python code bases in my experience. Typically the languages novices start with and many never leave…
@@kristianlavigne8270 Do you mean that other languages like Rust or JAVA are inmune to high cyclomatic complexity, copy paste engineering, poor class architecture and poor typing?
At a previous job, we had a guy nicknamed Mr. If. He didn't care and never improved.
In my Software class, we learned the following principles:
Don’t repeat yourself (DRY)
Comment where needed
Fail fast (code should reveal its bugs as early as possible)
Avoid magic numbers
One purpose for each variable
Use good names
Use whitespace and punctuation to help the reader
Don’t use global variables
Functions should return results, not print them
Avoid special-case code
I think this video summarizes many of them!
what is magic number
@hungryhikaru Well, if let's say somewhere in my code, I have
let costOfBag: number = 67.25 * costOfApple
There, the "67.25" is a magic number because we have no information about it on our code (though it could be inferred). This a silly example, though. I remember working on a project that involved geometry and had to write various angles across the code, such as 72. The reason why magic numbers should be avoided is to make the code more readable and to show the relationship between the variables, so that the code is ready for change and easy to understand.
Astonishingly it leaves out the most important one! - documentation(or comments) There is no substitute for it. Even if you have taken an unusual approach to an unusual project, explaining it in plainspeak in the comments will make the code very easy to read(if only to your own self in the future) Documentation, modularity(also great for debugging!) and avoiding nested as opposed to linear thinking are probably the three golden rules
Astonishingly it leaves out the most important one! - documentation(or comments) There is no substitute for it. Even if you have taken an unusual approach to an unusual project, explaining it in plainspeak in the comments will make the code very easy to read(if only to your own self in the future) Documentation, modularity(also great for debugging!) and avoiding nested as opposed to linear thinking are probably the three golden rules
@hungryhikaru "what is magic number"
If 21
Watching this video I remembered collaborating with some friends on a school coding project years ago. One of my friends had a proclivity for writing very messy code, using variable names like "habbababa". I asked her on that occasion why on earth she would name a variable like that - and reportedly, "habbababa" sounded "kind of cute". I still find it hilarious up to this day.
Yeah, I've seen many programmers learning to code use nonsensical names, just because they don't care much about programming. If they got into programming more, they would use much much better variable names.
I used variables like that for a couple years. Well, not usually COMPLETE nonsense, just unrelated to their function. "makeToast", "batman", "spoooon", etc. I eventually got over it.
I did this on a school project once because I was the only programmer and the 3 other team members were "artists" (only 1 of them made all the art, the other two did lord knows what for the whole semester). I was super bored so I started naming things random stuff like "laughingLlama" and "pb_and_j" etc. It was actually a pretty cool platformer game in the end, but tons of copy pasta for the different levels.
someone I know, he uses join at top of the query, and select at the most bottom..
2:46
I feel like the if statement that checks if it is a valid user does not need to be in its own method, as it is literally two lines of code. I get that making it into its own method would allow whoever is reading the code to instantly understand what the if statements are for, but another part of clean readable code is conciseness and being succint. If we are adding another two lines of code that doesn't actually much affect the readability of the code, then it isn't very necessary.
True, but let’s say you’re working with React, Vue of something. It’s better to turn your longer conditionals into computer state for better readability. In terms of executable code, I agree. It’s not necessary to create a small isValidUser func if you’re only using it once.
the point of extraction in this case is to define a new logic property for the entity user: whether it's valid or not. currently user is valid when they're authenticated and authorized, but who knows, maybe there will be some additional protection in future, like user will be required to have some special security ID, or restrictions like no banwords in username, not being deactivated or banned by administrator
At least, it should have a more descriptive name like "isUserAuthenticatedAndAuthorized()" or simply "isUserAuthed()".
You get the idea.
@@zyriab5797 isn't calling a function like that defies the whole purpose? why write isUserAuthenticatedAndAuthorized() when you can write isAuthenticated && isAuthorized
@@semplar2007 It's more so that when new validation method is added such as TFA, modifying the function will automatically apply to the entire codebase. Edit: I misread you comment and wholeheartedly agree with you
The reason math is poorly understandable is not its inherent complexity, but the fact that mathematicians for some reason are all using single letter meaningless variable names
Coming from a math background I actually find having to using explicit variable names makes it hard to calculate more than using Greek letters. There's a topic about this on Math Stack Exchange.
hard disagree on that one, simple variables are way better for the kinds of things math uses, especially in the domains mathematicians would actually be working on. The variable names are short, sure, but the notation is very formal as way and, in general, the formalism plus conventions capture what matters fairly well.
Well, as you can see I don't have any actual argument. I can only provide information and my experience. (Or you can say secondary evidence, supportive argument, or whatever). From your arguments, then a consequence is that computer scientist must understand that meaningful variable names are better than abstract notations. Is that correct? But if you look at how they write math formulae, only notations are used.
This depends a lot on what you’re doing and how complex it is. If I had to work with differential equations using camelCased variable names I think I would end myself. However, it’s usually super helpful with physics or advanced engineering topics to stop, and take a moment to walk through the equation, keeping in mind what each variable actually corresponds to, or even saying it out loud.
Except transport phenomena. Fuck that shit.
they use them because digits are single letters too. So the data is represented by the letters and the operations are represented by other symbols different from letters
Overuse of these principles (except the naming one) can also be a sign of an inexperienced developer, because they treat them like rules instead of guidelines and only applying when it really does make sense to do so. Overuse of extracting methods would be the most common one - creating a function for the sake of it when it's just a few lines of code that's only used in one place. If the name of the function helps to make the code more readable, this is where a comment would be more helpful than a function call. Bear in mind that calls have performance implications too.
You can always put inline for the function, and in many cases the compiler will do it anyway if it is deemed more efficient to do so.
@@danielcreatd872 Yes you are right. I should have said "may have performance implications"
Tbh context matters a lot. Even if it is used only once if it is kind of thing that would make sense to be used again as code develops then it still makes sense to make a function out of it. Otherwise you can get code duplication from seperate Devs rewriting same logic on seperate files.
I will take over functionalization over huge monolithic files but I get your point, striking the perfect balance takes expertise
Avoiding extraction for "a few lines of code" is exactly the wrong thing to do. An ideal function will only have a few lines of code. If you see a few lines of code that perform an encapsulated task, it absolutely should be extracted into a function, even if it is only called once. Readability and maintainability are at least as important as reuse.
In fact only virtual functions do have performance implications. Non-polymorphic function calls are optimized away by every decent compiler.
I agree 100% with rule 1 and 3. For rule 2, you have to be careful to not split things too early. This could lead to premature abstractions that don't work well in the future. Instead, don't be afraid to rewrite/refactor code early and often. So first keep it as simple as possible, and if some logic is user 3 times or more often, extract it. By then it should be clear what a good abstraction should look like. And because you're in the habit of refactoring early and often, you'd be confident to adjust the logic if the abstraction doesn't work nicely anymore later.
So my top priorities for good code are:
1. Write code that's easy to understand
2. Write code that's easy to change
I think what people who abstract prematurely do wrong is they don't understand why they should make abstractions. The real goal of abstractions isn't to avoid code duplication, it's to increase separation of concerns. Instead of focusing on abstracting the HOW, one should focus on abstracting the WHAT. I think a reason for that misunderstanding is that many beginner programmers will only encounter simplified examples of abstractions like those in the video. They learn only that they should avoid duplicated code, but not the reason why.
It reminds me of when schoolchildren first encounter equations in maths class. They could get an example like "2 + x = 3" and they wonder why the heck they should use equations since the answer what x is, is obvious. They don't get that the true power of equations isn't to find the value of x, it's to express a generalisation of an equality.
There is a lot more to it than just 3 simple laws but I can definitely agree that this is a very good start. Well done!
Extracing is a double edged sword. You now need to find the function, understand it, remember it and keep it in your head while youre trying to understand what follows after it. Also if its something simple then the function is pointless. The only reason to extract something is if youre using it multiple times. Also that first example for extraction it would be much better just having bool isValidUser = ...; if(!isValidUser){} right next to each other.
As long as the interface (func name and param names) are clear and meaningful, then this is not a problem because each lower level function should have been verified and tested, so you treat it as a trusted black box when debugging the higher level function.
Well yeah, if you do it wrong. A well written function with good documentation that serves one purpose is extremely easy to understand and debug.
You don't need to have intricate knowledge _how_ a function works as long as it's clear _what_ it does. (E.g. no one thinks how the regex method works in detail or how file read eventually makes a sys call, or what happens so that you get electricity out of the wall socket)
Creating good abstractions is hard, and all are leaky, but they can still help to reduce cognitive load.
Usually only when you’re debugging something, if it’s working fine you can just ignore what the function does in the background
@@piff57paff _"Creating good abstractions is hard"_
I think that's exactly the point. It's very hard to consider all use cases and edge cases in advance. That's why abstraction shouldn't be a kneejerk reaction to make code prettier. It should be born out of necessity. When you know _why_ you need it it can be better tailored for its purpose.
Imagine someone writing a program that converted well-written code to badly-written, by nesting, duplicating, and renaming.
comment for the algorithm
As your requirement doesn't include runnable, I suggest ChatGPT
so a badly written compiler.
yeah, its called obfuscator, or minimizer for other purpose
So basically ChatGPT
4:33 Bro gave us time to discuss the topic in pairs.💀
Groups of 3 😉
lmfao 😭
Separation of Concerns is good but I feel like people sometimes take it too far. So you'll find a function which calls 3 other functions and each of those functions perform only 1 line operations. A rule of thumb I like is that I only extract functionality to another function if it's used in multiple places.
the overly nesting problem often comes (at least where i live) from the way coding is taught at university. i was taught that a function should have a single return, and early returns where a bad thing, so that resulted in a deeply nested code
Same here in Uruguay, having múltiple returns was a bad practice at university.
There was a reason for having one return: cleanup code;
Imagine the following
function() {
setupStuff1(); //Remember to cleanupStuff1();
setupStuff2(); //Remember to cleanupStuff2();
// now bury this into logic
if (badPleaseStopNow) {
return shitHittingFan; // Forgot to cleanStuff1(); and cleanStuff2();
}
setupStuff3(); // Remember to cleanupStuff3();
cleanupStuff3();
cleanupStuff2();
cleanupStuff1();
return result
}
The single return makes it so you only have to check one "cleanup path";
I've actually found a bug in a friends' assignment "it just hangs"...
It was a mutex locking... and unlocking if the function went to the end... but it had an early return inside a loop... which didn't unlock the mutex.
If you are using a modern language with a modern library that usually isn't a problem because there are tools to deal with that:
C++ has destructor methods
Java has "finally" block
Python has "with" block
Go has the "defer"
such bs
The hate for goto also can bloat nesting
At my first job we were doing the next version of a project for a customer. The old code had been written in ADA i believe, and ADA has some sort of facility where you jump to some end code, kind of like a finally or ensure section, and inside that section there was one return statement (I have never programed in ADA, this i what i was told). In the new version of the project it was begin coded in C and C did not have that feature, but the requirements to have one and only one return from a function remained, it led heavy nesting.
the logic inversion is so valuable specially for beginners, usually you create a lot of weird situations and temporal dependencies by not structuring conditons properly
I agree, can get out of hand pretty quickly
Its personal preference, but i probably wouldnt create those tinty functions isValidUser and calculateTaxes. The first one you can clearly guess what its doing without having to create the isValidUser func. The second one, the file is probably named tax calculator service or the function instead of "main" is already probably named "calculateTaxes" or something like that.
The base function is already short enough and easy to understand, i prefer reading it sequentially than having different layers of abstraction by creating tiny functions like those. You already know what the base function will probably do because u previously read the filename or the base function name. If the base function was larger or more complex, then yes, i would probably extract some functions tho.
Great advices btw!
All great points! Thank you! 😊
I’m not inexperienced but I stayed for your animations
The "merge related if statements" example you gave is in fact a bad advice. Authentication and authorization are different things. You are not just "losing granularity", you are shooting your foot with a wrong log message. You'll waste time trying to solve an authentication issue thinking it's an authorization issue.
Nevertheless I agree with all rest, its very well presented animated and edited.
Hey, I see your point. I mentioned you’d lose that granularity in the video. But yeah, next time I’ll spend more time on the example code 🙂
Yeah! Additionally, it makes it difficult for the programmer because if they have to change the authorization code, they would also have to touch the authentication code (or at least, it would look like it for someone doing a diff on the code). A function should ideally just be responsible for doing a single thing.
@@kantancoding The user validation could actually be extracted functions like "requireAuthenticated" and "requireAuthorized" that both throw their own errors. The authorized function calls the authenticated function, and the main function function only needs to call the authorized function.
I’m luke warm about the DRY principal. It only makes sence if the spun out code can be understood in isolation. If you need to see where a function is called to understand what it does then the code is actually harder to understand.
I agree, but that's a misunderstanding or misapplication of the DRY principle. The DRY principle doesn't say that you can't duplicate anything and that you have to chop up everything into small fragmented pieces. One function should do one complete thing. If the duplicated code looks the same, but do different things for different reasons, they shouldn't be in the same function. A function shouldn't rely on another function or some external context to make sense. That is a poorly defined function.
Very good tips, but I like to keep the conditionals more granular. It's easier to write better log messages or throw more specific error codes
I'd agree in general, but in the specific case used (unauthenticated vs unauthorized user), it's often best -not- to split them out. I don't want people "fishing" for valid usernames! That's why so many login screens give errors like "username & password do not match" or the like.
@@hoi-polloi1863 ohh i never even considered this!
I see some people complaining about the code extraction. In the example, I would do a similar extraction, but for another reason. Avoiding code duplication is a nice side effect, but the core idea should be: *Keep the abstraction level of a function or class or whatever constant.* An example: You don't want a function dealing with structuring a report page doing regex stuff, this is clearly too low-level. Think about how you would explain your function with a few words to your colleage: "I look if there is a report header, else I use the default, then I add the body, and if there is a footer, I include it as well". And that's what the function should do. If it does some finicky stuff with the body, extract it. If it does some regex, definitely extract it, maybe even a level deeper, below the functions dealing with the header, body or footer. Having one abstraction level is much more important for readability than to avoid code duplication.
People who complain about code extraction don’t write unit tests.
@@kantancoding and they write unreadable spaghetti code :/
That makes it hard to do anything beyond simple changes.
I would usually agree that it's important to avoid mixing levels of abstraction, but I never thought someone would ever think that using a regular expression at high level code is a violation of that, regular expressions are not part of the hierarchy of concepts for a page
@@kantancoding This is is. There's a type of "professional" developers out there who complain about TDD (TDD and clean code go hand in hand imo) being so much overhead but wasting so much time debugging. I've seen people in enterprise context preferring to wait for minutes for the app to startup and testing every single iteration of their changes by hand instead of adding a little bit of code and refactoring and automating this process. LiveUnit-Tests give you feedback in seconds and in the long run you're so much faster with TDD.
Seriously, I'm doing TDD now for a long time and I hardly need the debugger anymore. Tools like NCruch even show you the code paths or hot spots in your code these days.
Using the debugger for me is mostly a sign that I'm working with ugly or untestable code.
best comment by far.
It’s worth noting that the first example refactor changes the program behavior- they are not equivalent. It needs additional return statements in the guards .
Actually that’s incorrect. It doesn’t need a return because log.Fatalf() will exit the program. It’s the same behavior as before the refactor.
If you're curious: cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/log/log.go;l=417
@@kantancoding Good to know! I’m clearly not familiar with Go 😅
So for a more general refactor recipe one would basically make the (implicit) returns in the "leaves" of the control flow tree explicit before applying inversion and remove redundant ones afterwards.
@@RegrinderAlert Yeah, that's my bad. I shouldn't have used such implicit code in the example. I didn't really think about it at the time 🙏
On one hand it could be expected logging a "Fatal" message should exit(1) but in some systems allowing a logger to end the program could be seen as bad design or dangerous.
It's a weird behaviour when a logging function exits program
1:46 All fine and good until you run into an auth error and can't figure out if it's an authentication or authorization problem. Don't do this!
you *should* do that, he just gave a wrong example.
Yeah I was also thinking that he committed a much worse coding crime than having some deeply nested code. As someone who troubleshoots other people's code in Prod a lot, never ever do this and log incorrect errors.
TL;DW
Use guard clauses to check state for on/off condition code, switch statements are great and can be used for simplifying the if, ifelse, else chains, enums are also useful for conditions that are always going to be of a set type
Reduce duplication by moving common code with similar purpose into their own methods. Single lines don't need to be methods, multiple line duplications probably should be moved unless they're inherently dynamic like DB calls.
Make clear variable names for everyone to figure out what each variable is, single and double character names aren't helpful for describing what your variable is in most cases.
Great examples! You earned yourself a new subscriber!
Back some 30 years ago, I noticed one programmer on the dev team consistently broke our coding conventions.
Drop downs in the IDE listed methods in alphabetical order. He prefixed all methods with letter = A to ensure his methods for all devs always showed up first in the IDE. Ego driven!
Next, any local variables he created were named Anna, Bertha, Charlotte, Denise, Ellen, . . .
I asked why? His answer: To ensure only he would ever want to maintain the could he wrote - to ensure his continued employment.
Sometimes developer decisions can have effects way beyond the next code check-in.
lol I don’t understand the argument about making your code obscure so other devs can’t maintain it. There’s definitely always somebody smarter than you who can figure out what the code is doing and refactor it.
Also, this guy seems insane 😂
I really need to say this. The advice to never nest your code has been repeated for years now, it's the same thing that I was told 8 years ago when I was reading about how to write VB code in highschool and it is a good concept, but It's important to note something.
In the provided example, yes, I think the code is easier to read, but it does not change the conditions you need to keep in your head. If you are doing more complex logic that then relies on the state of those conditions, you still need to know that those conditions are not true for example.
Take authentication. If you have middleware the runs in an endpoint, or your function verifies that the user is authenticated before running, you still need to know that condition has been met, otherwise you will end up writing redundant code in your actual logic to ensure that the user is authenticated. Whether your nest or don't nest the condition, you need to be aware of the state your app can be in at that point in the code, and both ways require you to have this awareness.
I agree that in most cases, early returns are better, but just wait until you have a function that has a structure like this and tell me it isn't confusing AF
if (A) return
// logic
if (B) return
if (C) return
// logic
if (D) {
//logic
} else {
//logic
}
if (E) return
//logic
The code looks readable, but damn, it can get confusing, especially if the code I put as "//logic" can potentially return, therefore adding conditions that aren't even ovbious. Sometimes the real solution in my opinion is to double up your conditions just for understandings sake, or in a complex case, abstract where necessary. The idea of code inversion is very primitive and does not always apply
These are very good points. Thanks for the insight!
When you check all your preconditions at the start you're fine. If the logic is complex, by all means, use nesting.
Great brief explanation. As someone once said, bad coders know what to write, whereas good coders know what not to write.
I call it "The indian arrow." Once our externist (guess from where) gave us a 10x (I kid you not) nested code. Truly amazing.
Writing readable code is an art and a great skill. I also think there is a lot more that could be covered here.
I'm surprised this video didn't mention comments. At 2:05, couldn't you instead put comments on what the code segment does instead of making it a small, one-use function? I think it would be harder to go and search for function definitions.
could have also put the condition into an aptly named variable if it was so badly needed. whenever i find something like this in a code review i tend to immediately reject it, because stuff like this make the the code as a whole harder to navigate. imagine if every single condition longer than a single word were to be its own function - the whole thing would fill up with one liners that each take up 5 lines instead. i'm not braindead enough to be stumped by a single AND statement, i'd much rather keep the code concise.
Great summary of the most important concepts that everybody should follow in a team.
Nested, unreadable code like your example is a perfect breeding ground for bugs, often missing edge cases or special cases. It's always worth taking a closer look at such code.
Saw the video until 2:42 instead of one single function that I can read and understand I’ve got multiple functions, multiple contexts, you forced me to remember what’s going on in all functions, and a simple code becomes unreadable. Bravo
Skill issue brother
@@kantancodingNo, what you presented is terrible advice. One should only extract logic into its own function if it is used multiple times. There is no benefit to extracting logic, especially if it pertains to the original function.
Sure, we’ll just put everything in main() if it’s only used once. No need to logically structure our applications.
And the comment you are responding to is completely unrelated to what you’re saying man.
I’m all for opposing arguments but your argument is useless since you don’t give any type of explanation of why or how you came to that conclusion.
Law nr.4: Always comment on what something is doing. Not only will this allow you to remember why it was made in the first place, but it can help anyone else who needs to look over your code. This can also help them find a function, a code block, or array they might need to change.
0:32 flutter devs in shambles rn
There are a lot of Videos, on general readable code, but I would like to see one, which gives you tips, for structuring a whole Project, at what point to use Classes or what valid to put in Arguments or write a whole new Function for etc.
Good Video tho :)
Please correct me if I am wrong.
The inverting looks good but I don't think it is that simple. In the given example, multiple conditions are needed to be checked to execute the nested code. If you are going to make it linear, you will have to use logical operators like &&, || to make sure that the condition remains same.
For example: at 0:39, "user not authorized" will be logged if ( ! isMaintainancePeriod && isAuthenticatedUser && ! isAuthorizedUser ).
On the other hand, at 1:21, "user not authorized" will be logged just by ( ! isAuthorizedUser ). Here, we have not made sure that isMaintainancePeriod is false and isAuthenticatedUser is true.
You're not at all wrong, the logic is entirely changed. Personally I've come across a lot of these "laws" by folks like Robert Martin and I'm just not a fan. I don't think most of them are that critically helpful.
No... we have made sure that isMaintenancePeriod is false by exiting the function if it's true.
Rhe authorized user logic did in fact change but that's not because of the inversion.
Changing my habit of writing code is better for the next developer to continue my work. But what will be more valuable for me is to be able to understand the messy code that someone else has written with bad patterns.
Im not sure why but somehow lots of Go code using single / double character names which annoys the hell out of me.
Good stuff, keep it up!
Thank you! Yeah that has always annoyed me too! It seems some Go people would rather be terse but I always get irritated reading that type of code.
I really like how Verilog has freedom of the switch-like case statement to the point of "reverse case statement".
E.g.:
case(1'b1)
isMaintenacePeriod: log.Fatalf("feature unavailable");
!isAuthenticatedUser: log.Fatalf("invalid user");
!isAuthorizedUser: log.Fatalf("permission denied");
default: doWork();
endcase
SystemVerilog even adds features like unique and priority.
Point 1 is good. Point 3 is a bit of a dramatic example IMO but it does happen 😅 and there's no such thing as perfect names, so be sure to write good tests and documentation anyways.
Point 2 though - "Avoid duplication" is I think the wrong advice to be giving, it's missing the actual problem.
The problem is poor abstraction, or in other words a lack of / unclear / poor separation of concerns. Lots of duplication is simply a symptom of poor abstraction. Your problem is in your design choices, not your duplicated code.
Software design boils down to asking the question "should this code be shared or not" over and over again. By saying "avoid duplication" you imply the answer to that question is always "yes" when in fact that's not going to give you good results. Sometimes duplication is the right answer.
The key question, where you want to focus your intent, is on where and why code is shared, and whether it should be or not. Don't simply avoid duplication, that's a shortcut that will lead you into poor design choices and will prevent you from improving as an engineer. This is the exact reason why the calculateTaxes extraction from the first example is a bit iffy - I understand it's a bit contrived on purpose, but it clearly illustrates the problem of preferencing "cleaner" code instead of making a thoughtful abstraction, because the latter is what really matters. The developer's intent is in the wrong place, i.e. "perceived cleanliness" instead of thoughtful, intentional, useful abstraction.
The most effective way to produce good abstractions is through strong TDD practice. As you improve your practice the best abstractions will start to become obvious, and you'll stop thinking about code duplication and "cleanliness" altogether, because it turns out to be somewhat irrelevant in the quest for "good" software.
---
I'll throw in a Point 4 for free, don't use double negatives in conditional logic. This feeds into data design, too, naming a field disableTooltips just means later someone has to write if (!disableTooltips) { enableTooltips() }... It makes logic just enough harder to read to be not worth it. This is a simple example but add a clause or two or a few indentation levels and the cognitive load adds up quickly.
I probably agree everything you said.
The point 1 rewrite is good enough and I think the rest of the video is kinda pointless to the original problem.
After the first one, like merging conditions into one, everything becomes a readability preference, it doesn't affect the readability and you lose the log difference and the fine details. The point 2 (convert multiple conditions into isValidUser) is purely wrong in my opinion, it's pure premature abstraction without good reasons. Unless you told me that isValidUser is fairly common and used in many spots.
Last point is just pure assumption of what your code will be in the future, the reality is that 90% of the time your predication will be wrong. Had seen countless cases where people/colleagues rewrote their pre-imagination refactor/abstraction over and over, which can be avoided at the really beginning if they kinda listen my suggests. Also, reduce my PR review time.
So instead of trying to do the abstraction early, leave it like that is totally fine. If you need to use many "what if" for an abstraction, likely you shouldn't do it. I felt like the whole video falls into the trap of premature abstraction after point 1 unless the author didn't include those context in the video.
@@doc8527 Yeah I totally agree with you. I also don't think deep nesting is a sign of an "inexperienced developer" necessarily, but premature abstraction all over the place is definitely a strong sign of inexperience 😅
@@nijolas.wilson Hmmm I feel like you both are applying the wrong argument to what I'm trying to explain. I'm not adding abstractions to the code based on some prediction in the future. I'm simply writing each function so that it does one thing and only one thing. There's a difference between:
```
package main
func calculateTaxes(product product) {
switch product.Type() {
case "alcohol":
total += alcoholTax
case "electronics":
total += electronicsTax
default:
total += generalTax
}
}
func main() {
for _, product := range cart {
calculateTaxes(product)
}
}
```
AND
```
package main
type calculator interface {
calculateTaxes(product product)
}
// implements calculator
type taxCalculator struct{}
func (tc taxCalculator) calculateTaxes(product product) {
switch product.Type {
case "alcohol":
total += alcoholTax
case "electronics":
total += electronicsTax
default:
total += generalTax
}
}
// "constructor"
func newTaxCalculator() calculator {
return taxCalculator{}
}
func main() {
tc := newTaxCalculator()
for _, product := range cart {
tc.calculateTaxes(product)
}
}
```
@@kantancoding Sorry, I didn't mean to imply by my response that I think your example was premature abstraction, or that I think you are an inexperienced developer 🙏 that was meant as a separate commentary. I agree, and to me it's very clear the point you were making.
I don't think that deeply nested code is a sure sign of an inexperienced developer, but I also don't think what you were showing was premature abstraction. 🙏
😂 the part about deeply nested code being a sign of an “inexperienced” dev was likely a bit of an oversimplification/overgeneralization on my part.
yeah and on the extraction
rule of thumb: 3 to 5 actions, which produce 1 action when combined.
e.g. function is "cook potatoes"
actions would be "peel potatoes", "chop them", "choose a way to cook them", "cook them the way you chosen to".
if you can split an action into more or equal
amount of actions you should do so.
e.g. ",cook potatoes the way you chosen to" can split into "get the chosen cooking method", "prepare for cooking" and "do the cooking".
you can split actions into more actions into more actions. they only need to result in one concrete thing and be semantically similar.
like e.g. when we cook we will do things related to cooking. we will not focus on arbitrary actions like "take the knife in your hand" if it isnt obvious how we will relate that action in the context of splitting another one ("take the knife in your hand" and "make a chicken stock" arent obviously related, imagine if we dont have to chop anything why did we take the knife then?)
why 3 to 5
becsuse human brain ram is approx 5-9 statements depending on the person
so youre always within your brain's 1 stack frame (maybe even 3 if youre lucky). thats easier to reason about what
Law zero of readable code is to comment abundantly and always keep in mind that the reader might be... future yourself. I don't know about Python, but Perl, Java and C can be deceptive... Blocks of code? Tell me about looking for a bug in a regular expression just 6 months later in a function called from pretty much everywhere in hundreds of lines of code (i.e. a small program).
Also, do abuse functions/procedures and merge them to the main code (with lots of comments!) only at the very end when they're called only once (just get the xref to get the list). The thing about code being unique but called from multiple locations is a well known paradigm of "proper" databases: each piece of information is semantically unique, otherwise you'll pay that in code and data size, and each chunk of data is identified uniquely. These are base rules, but everything else derives from that, you'll think of me when you need recursivity and callbacks: pointers on lambdas are a thing...
Yeah, documenting is a must. When working on multiple projects at once, as one often does when programming as a career, makes it increasingly difficult to remember what a particular piece of code does. I think I was lucky in this regard when I first learned programming. I was born with a disability that among other things has affected my memory. As such, I have always commented my code a lot, out of sheer necessity, because if I don't, I forget what the code does the next day (heck, some days I wouldn't be able to remember over the lunch break...).
@@oliver_twistor have you ever tried coming back a month later to a Perl script full of uncommented RegExes? ;-)
I'm a software engineer of 15 years and my main thing is that code is not for the computer. It's for the person reading it. Just think "is this code understandable for person that has no context" you write so much more clean code.
problem in second law is, when trying to avoid duplication, I also need to do transaction in SQL, and every transaction function always differ from other
Every law has its edge case. The point where it divides by zero or reaches infinity or whatever. Just adapt. A law is just a guideline, not something to follow religiously like a robot.
This is exactly why Chris McCord make significant change in elixir 1.12 to 1.13. He change 'Model' in MVC to 'Context', partly because people just keep using the generated Model to talk to db without using proper transaction. Every transaction SQL will different from each other. Race condition is fine if you just want to make small blog, but not in banking app
There is also a "five finger" rule. The origin of this rule was back in the days when code reviews were done with printouts. Reviewers would identify the entry point of the code under review, then use fingers as bookmarks to walk through the program logic to evaluate its correctness. This agrees with the 5+/-2 rule of human short-term memory, which effectively means that if there are more than about 5 nested function calls, the reviewer would lose track of the logic, and the time and accuracy of the review quickly deteriorates.
Although there are code inspection tools that can search for symbols and keep a stack of examination points, the 5+/-2 rule is a limitation of the human reader and remains an issue.
Executing these three laws also serves to reduce a metric called Cyclomatic Complexity. This is a measure related to the number of branches and loops in a piece of code (including short-circuit operators), and it's interpreted as a measure of the likelihood that a section of code contains a bug. It's often used to guide code reviews and testing. It reinforces the notion that code that lacks complexity is both more correct and easier to inspect.
Also, User not authenticated and use not authorized are very different things, its just wrong to have a common response to both!
yea, he did point it out tho. Soo like maybe we should do his approach just to make it "readable" in exchange for information
The first one is mighty if you use it with de morgan's law. It can also be an optimization if "isAuthenticated" and "isAuthorized" are packed in the same register or data in cache. In that example you might want to keep the granularity for log messages. But in machine control you might have a bunch of conditions that would force your machine to stop if one is false.
If i had to add one rule it would be keep the same flow control statement for the same type of task. If you keep the number of language features low and the syntax similar it's much easier to understand the problem the code is solving.
The first example does not make the conditions any less relevant, or further out of mind. It’s just easier on the eyes. I hear this incorrectly described too often.
In fact, the first rule is a design pattern named Early return. Which can be refactored to Chain of Responsibility in more complexe situations.
Both second and third rules made me think about respectively the third and second rules of the 4 rules of simple design from Kent Beck.
This is actually very helpful. After looking into these I’m thinking of expanding the series. Thank you!
It's also called a guard clause
i was gonna say make sure you use comments. much easier imo than searching through code again
As someone who only got into learning C quite literally last week, I'm shocked and awed at how few comments (pun unintended) there are about commenting, and that it wasn't even mentioned in the video. The amount of times I've searched on stack overflow to try and find a solution to a problem I was having and only understood what it was I was looking at when the poster explained what the code did afterward...comments are an ACTUAL godsend for anyone trying to read your code!
One thing I tend to prefer in my code is avoiding the ! operator in if statements. Sometimes it can be missed by someone who is going through the code quickly and therefore I tend to try and make all my boolean variables describe what the condition is. Of course don't go out of your way. Sometimes a quick ! is better than an extra line of code to flip/rename a boolean.
Agreed. I don't like that tools like clang-tidy advocate for less readability rather than more. I always spell out my boolean comparisons.
I was on your side until you talked about the functions. Don't get me wrong, functions are useful, however, functions which only have 1 return line or less than 5 lines altogether, are unnecessary and using those would require the reader to potentially jump around in different files to find what the main code is doing.
This is quite good, and as someone who’s been teaching programming for 40 years, you’d be amazed at how many college instructors will teach exactly the things you’re saying not to do.
Thanks for watching! Great to see so much experience in the comments section 🙂
2:06 While extraction is good in some cases, in this case that is a step too far. Code is easier to reason about if you keep basic operations inside the main function. You should only extract large chunks of code into their own inline functions. They should be inline to start with because you have no reason to take external code doing the same thing into account until it becomes common enough.
Somewhere about 3-5 usages is the minimum amount for this, depends on the complexity of what is being done. In the case of one liner expressions extraction is in fact incredibly poor developer behaviour. If the code spans at most just a few lines (3-5) then an inline function is all you need for it. If it spans 5+ lines then it's a candidate for a shared function to avoid mistakes in logic due to complexity.
The argument that extraction should only be used if the code spans a certain number of lines doesn't make any sense to me. Why some arbitrary number of lines?
Better to have a real reason regardless of the number of lines. The reasons are as follows:
- separation of concerns
- modularity for testing
- reduced surface area when debugging
- readability
- enables us to test individual behaviors in isolation
- the above mentioned tests have the added benefit of serving as documentation for how each individual functionality should work
@@kantancoding All those reasons are auto-covered by the line count. The more lines the more context you're trying to keep in mind when looking at the whole function. 1 to 3 are easy enough but it get's a bit unreasonable from there on. However jumping straight 2 a shared function when extracting is not always a good idea which is why I say use a local inline function. Also for many people, both newbs and devs, a line count is easier to understand or even remember than a bunch of more complex rules. You can simply reference those rules as the reason for the line count. At bare minimum the line count helps identify code that should at least be considered for extraction. It's like using if ( line_count >= 3 ) extract = should_extract( lines ); You're simply skippimg the complex stuff until the line quota is met
OMG. I love how your refactoring of code actually changes the logic and you somehow breeze right over that as if that's not kind of important??!!
I think I need to make a video about rookie developers doing TH-cam videos. 🤣😂🤣😂
to me one thing i've learned is
1) make sure functions can run without relying on global variables this to improve testability
2) all false or invalid states should be on top most
3) nest as few as possible.
4) don't apply abstraction too early until you actually know you will be repeating it this so you don't end up with 1 time use functions
5) avoid iterating a database rather turn it into a function from the database
Good points! 🙂
Usually it's better to sacrifice performance for readibility. This is where CS programs set up their students for failure when they get to the real world. The thing you really need to optimize for is Mental RAM. Engineering time is expensive. Writting code that's easy to understand and refactor and is tested will save engineering time in the long run.
So true. And increasing computer processing power is generally much cheaper than increasing human processing power. :) It's better to let the machine read one more line or one more byte than having the engineer waste a day trying to understand their colleague's messy code.
Kind of ironic that a video about readability flashes words onto the screen instead of displaying the full text and animating it... ;P
Absolutely brother!
i had zero issues following along with the video, maybe just, use your eyes?
@@chersymale "use your eyes?" lol
It seems you missed the ";P". My comment was meant to be tongue in cheek.
It's supposed to be, so you realize it.
(Can't tell how much of this is sarcastic, I know I know, always assume someone online is sarcastically wrong...)
the results are so readable you only need a split second.
As someone who taught many programming students, I'm glad to say I quickly understood the bad code in the last example :D
😂🤣I too have had lots of exposure to that one
Thanks for the suggestions 😊
30 seconds into this video and I liked and subscribed. The animations and simplicity are great
Thank you for the support and the kind words!
You seem to have completely missed the big backlash against DRY. The biggest tell that someone is a junior programmer is when they start creating premature abstractions to avoid code duplication.
I think this was just an easy example of the concepts. If this was the total extent of the program, I would agree with you that extracting the code into separate functions is a bit excessive. DRY is good; the problem isn't that junior programmers abstract prematurely or too much, the problem is that they try to abstract the same literal code, but from multiple contexts, which will cause problems later on when they want to make changes to the extracted code. But if one make sure that the abstraction also is semantically the same, not just lexically the same, the DRY principle should be good.
I feel the same way with the inheritance backlash. In my experience, most of the criticism comes from misuse of the technique, not from the technique itself. Same with OOP, a lot of criticism comes from either lack of understanding what it is or plain old misuse.
Quite late, but this video is amazing, bug thanks on the tips. Also, what programming language is that?
variable naming has the problem that it dependes of the program model inside the programer's brain , you must choose names that make sense in context of what are you writing.
" Future readers of your code " will never have the same brain model until they read enougt code .
Java fully qualify names is the solution , and i hate it.
Hmm, this is a good point actually 🤔
"The two hardest problems in software engineering:
1. Naming things
2. Invalidating cache
3. Off-by-one errors"
That's why, in my opinion, a variable should be short-lived; preferably only be used once and also be defined as close as possible to the code that uses it. I never reuse variables, I treat variables as write once, read (preferably) once. I hate it when reading someone else's code and they have defined a variable with some dubious name on line 10, and then first use is on line 46, it's redefined with a new value on line 51 and finally used in a totally different context on line 123.
the switch case does not need to be in a separate function, while you can modularize anything, just having a separate function for the validations and the logic is a good amount of modularization with reasonable "locality of cose" so the un-necessary navigations are not needed!!!
The only thing I have a problem with is the if statement in the first code. Ie; ! isAuthenticatedUser || ! isAuthorizedUser
First of all I would be really wary of putting the both conditions into one if statement. The reason being clarity. We can only use one error message the way you recode it, which could work but it limits how descriptive the code is.
Second thing is, that by having the if statement in its own method, you don't make the code better. It doesn't make it shorter or more readable. I would even argue the opposite for both. The code is longer and you have to search the code for the method which could be just there. The conditions are descriptive so there is no problem there. Of course in such a short code it doesn't change much, but when you are working with longer, more complex code, you would only make it harder for others to understand the code.
One other thing you can do is maintain objects so you can use the 'if' statements wherever you want, to check for the current status of the block of code you are running. Sometimes it won't work but most of the times it will work.
1:12 Well, the code kinda needs a “return;” after each log.Fatalf()…
I think Fatalf throws error
Great, thanks to you I no longer get to know if it's my email or password I typed in incorrectly.
Good stuff! This is the type of stuff that doesn't go out of style :)
Thank you! I’m glad you liked it 🙂
gotta admit, did some of the nesting 'mistakes' you mentioned, noted it under great advice, for naming i find it the best to write the convention used in a comment at the start of the file. not sure how helpful it is to some people but since there are a few i wanted to make it easier for potential other programmers
My first law of readable code: opening and closing curly brackets has to be in the same column.
100%
const char* this_is_also_acceptable() {
if (closing_bracket_matches_statement_start) {
return "ok";
}
return NULL;
}
@@notyourfox I have a hypothesis that a lot of people who use this style learned Pascal first and wrote "IF x >= 10 THEN BEGIN" ;-)
@@damianjblack I have learned some Pascal (and now remember almost none of it, didn't ever really use), but my first language was Python
Also I remember writing begin on a new line smh :)
I was generally a terrible coder back then
@@notyourfox I have to admit I love Pascal and my main hobby project is in FreePascal. And yes I still put BEGIN on a new line for some of my code. IFs and loops not so much but usually for procedures and functions.
Me and my buddy like to make fun of naming conventions and terminology while we work together. The first rule would have been termed to 'hire a bouncer/hire a guard' - in order to write the subroutine exits at the top instead of dangling elses. Interface Connectors are called 'Handshakers'. We are writing code that are full of insiders.
Also i've learned to better name functions as getOneUser/getManyUsers instead of getUser/getUsers due to quick readability
Not being overly enthused by silly 'guard clauses', I like your functionnaming scheme of and (please note the application of capital(s)). I myself use a simmilar scheme of _ or even _, like 'Get_Object()' or 'Read_Line()' or 'Show_GrandTotal()': all for the benefit of readability...
more like this please
Anything specific? 🙂
If a piece of code isn’t significant, I feel like it isn’t a good idea to give it its own method. Comments exist for this exact purpose.
I’m personally not a fan of some of those function extractions. If I were reading the code, I might not know exactly how calculateTaxes works, but if instead you added a comment saying “// Calculate taxes”, I’d know exactly what you intended the code to do and also exactly how it does it.
With extraction, the general idea is "Either you are working on the program's general functionality and you don't care about the implementation; or you are worrying about the implementation and don't care about the overall flow" so you separate those two things by tucking certain details into its own part of the codebase. With many IDEs you can simply control+click the function you want to know more about and it will jump you right to it!
Wow finally somebody who knows how to go to function definition.
Another point, with calculate taxes extracted, you can now write unit tests to test that functionality in isolation which will have the added benefit of providing documentation about what calculate taxes does if you really need to know. Boom, no need for excessive commenting
I would argue you probably don’t *need* to always know what every piece of code does at all times. And if you do, you can easily go to it. If you are debugging, editing or expanding this function you can easily follow what’s going on and where to add/edit the logic without reading the entire implementation line by line
@@zachmanifold Yep. It would be a pain if all code for all standard libraries and such would be inline. A simple Hello world program would take several sheets of paper.
Good video.
I don’t agree with extraction though if it’s not to handle duplication, it’s enough to leave a comment to say what is happening, jumping functions in most cases can be confusing as hell if it goes too far off, also it’s not good performance vise to extract code to functions, so just better leave a comment
Just add a comment instead of converting it into a function, adding too many functions can also cause confusion when they are only being used once.
One word of caution, you should never default to valid with authenticating a user. So, that's one area where allowing the code to drop through when by checking !valid instead of valid will get you into trouble.
First law: Use Go, Second law: ???, Third law: Profit
😂🤣
Law #4: never right negative properties. The name of your property should be something such that knowing what the default is is very obvious and straightforward. If you find you are putting the word not in your property names then you have it backwards. Example: DontProcessX. In this example in order to process x you would need to pass false. You're much better off with a property called ProcessX
First rule for me - you need to remember about Clean Code
Yes!
Thirst rule - don't forget to hydrate!
Silly programmer. That's not what 'thirst' means!
@@CartoonyPirate thanks))))
Regarding clean code: placing the opening containers at the end of the line, which is the case for every if() and for() statement of these examples, is NOT clean code and is, therefore, NOT readable code. The nested if() statements are made much harder to track because the formatting is unnecessarily compact. Frankly, the nested if() statements with the open container at the end of the line is less code structure and more ASCII art. Please stop showing young programmers your ASCII art!
I do not entirely agree with the second law.
Having code duplicated is completely fine while being wrong at the same time.
By that i mean, taking your example, let's say that a new business rule is introduced for the function that iterates over the users list. It's specific to that use case only.
Now you have to mix up logic from two different use cases.
What if that function needs to be re-used on a third use case? What if it has something specific to that use case?
In most of the cases I would say to follow the WET (Write Everything Twice) principle and unless you have the same exact lines of code in more than two places i don't think there's a real need to move something into a shared space.
Good video overall :)
You forgot one rule: COMMENT YOUR CODE... No seriously, it takes like 5 seconds to do and makes any and every piece of code much easier to read and understand. So please, for the love of god, comment your code.
Good code doesn't need comments tho
I try to use comments to explain "why".
If I feel like I should explain "how", it means the code is not readable/explicit enough.
Besides, comments tend to "expire" during refactoring and updates, especially when working in teams.
Excessive commenting is useless and actually not good practice in my opinion.
Not to mention, if you write unit tests you are actually documenting how the code should work so comments should really only be needed on rare occasions.
No. excessive comments is a smell. Code should be self documenting by being readable, and only in rare cases comment if there is some necessry tricky logic.
Well written code with well chosen names should be self explanatory.
The variable naming is rather important, even if you want to read your own code months or years later. The other advice is great for readability, but can slow things down a bit with extra function calls. Most software today probably doesn't need that kind of speed up, but some things could. So just remember, every function call adds a go to subroutine, which will push the current instruction pointer to the stack, then jump to the subroutine, and when done, it will have to pop the previous instruction pointer back off the stack, and then jump back. The key is understanding the requirements, it's easy of maintainability verses number of instruction cycles required.
I often wonder if modern developers work on solving actual problems, or just moving text in their IDEs.
I checked out this video to see if there was anything off but this was well explained. Nice work. In JS, eslint can be used to check deeply nested conditionals. Maybe go over something like that
Thanks for the info!
My advice is to put lots of boolean statements into one integer, and compare the resulting integer with the combination you need for code execution. Example is calling an integer "status", defining 3 as admin, 2 as trusted, 1 as worker and 0 as unauthorised. Then you can make switch status, case admin, trusted etc
this save me in job, i took hours to find a error in a script that i made. after refactory, using functions was very much easier to find
Love your content! Subbed and thank you! 🎉
Hey! Thank you! That means a lot 😊
about extraction. in first case with taxes, the extracted function is used only once, and it gains readability only by its name. so why not just to add a comment?
1:30 Infact you can have a separate method for validations there, which is what most people would write this code as anyways.
Using this advice, I have now obtained the ability to understand my own code after looking away from it for more than 3 seconds.
😂 I remember those days
In teaching, i'd recommend mentioning the titled names of the 3 'laws' in the end of the video to help brains retain the information. Second, I recommend after making final adjustments to code explanations that you give a side by side of original and final. This helps brains understand structure, just like the first 'law' is alluding. This would elevate your vids from good to fantastic.
Awesome tips! Thank you, I will try to incorporate them :)
Your 'calculateTaxes' is not a function. So from the point of view of the main code, it becomes a blackbox that can be full of surprises. So it doesn't make anything clearer to have it.
I'm a junior-ish dev in a semi-low-code integration system and I find myself struggling with 1. Granted, in my context it's not always easy to break things down into steps because declaring a local variable inside e.g. a data mapping isn't really allowed the way I'd want to (the "do" block is supposed to allow that, but it's not _quite_ the same -- especially because it is _itself a block,_ so only worth using if you can break down many steps with many intermediate variables), but still, I often end up with a long chain of function calls that are combined all at once with no easy way to rewrite. I do wish I had the skill to avoid that, but my platform doesn't make it super easy (or maybe I just suck).
RE 2, I recently had to work with a codebase that managed to invert DRY's use in places. Things that you would _want_ to extract and unify into one piece of code were duplicated... but some logically different use cases that _just happened_ to share the same code were unified. Having to work on one of those use cases, I almost broke the other.
(Happy to say RE 3 that I have not yet encountered single-variable names. Yet.)