9 "rules" for cleaner code | Object Calisthenics
Vložit
- čas přidán 15. 06. 2024
- Become a Patreon and get source code access: / nickchapsas
Check out my courses: dometrain.com
Hello everybody I'm Nick and in this video I am going to cover a ruleset called Object Calisthenics. Object Calisthenics was defined by Jeff Bay in his The Thoughtworks Anthology book and it defines 9 different rules that you can use to write and keep your code clean.
Don’t Use The Else Keyword video: • Why I don't use the "e...
Wrap All Primitives And Strings video: • Treating Primitive Obs...
Timestamps
Intro - 0:00
Only One Level Of Indentation Per Method - 1:34
Don’t Use The ELSE Keyword - 5:14
Wrap All Primitives And Strings - 5:53
First Class Collections - 6:39
One Dot Per Line - 9:32
Don’t Abbreviate - 13:40
Keep All Entities Small - 15:56
No Classes With More Than Two Instance Variables - 18:12
No Getters/Setters/Properties - 20:36
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: bit.ly/ChapsasGitHub
Follow me on Twitter: bit.ly/ChapsasTwitter
Connect on LinkedIn: bit.ly/ChapsasLinkedIn
Keep coding merch: keepcoding.shop
#cleancode #cleancoder #objectcalisthenics
the most important thing about rules is that you know you're breaking them and why you're breaking them. The rules exist for people who can't yet recognize the problems they're inviting when they do.
i'd thumb this up, but it says 69.
Sean, I totally agree. I like the way you think.
There's an FP koan that basically teaches this. A student is trying to learn how to program without side effects, so the teacher tells him not to loop over a list by incrementing, but to use fold, that sort of thing. The student is learning all of this, and decides to study the teacher's code, and finds out that the teacher is still using incrementers and while loops in some cases instead of using things like fold, map, etc. When he asks why the teacher is using them when he's the one who told the student not to, he explains "but i already know how to write code without side effects."
The point being that the important part is the conceptual approach, not which keywords you happen to be using
1. I follow this with a caveat - it's only a problem if nested indentation is more than a few lines long.
2. I don't do this, it often makes code less clear.
3. I do this sometimes, really depends on what kind of code I'm writing and how I use primitives
4. Heck no. If the collection doesn't have its own invariants, it shouldn't be newtyped. Most of the time the invariants are the owning class's, not the collection's.
5. Most of the time I do this, not always though.
6. This is good. If you can't find a reasonably short name w/o abbreviation, you're probably doing something wrong.
7. This is a pure antipattern, also known as "how to produce an unreadable codebase". Even simple behaviors must span multiple files with this approach, and that's terrible for readability.
8. This is pretty silly. Doesn't do anything good, just bloats the codebase. Keep classes small, but split them at meaningful points.
9. If you have both a getter and a setter that do nothing, it should probably be public. If it shouldn't be public, it probably should just have a setter.
In general: clean code comes from care and attention, deliberate thought. Guidelines like these can help, but they won't give you clean code automatically.
I agreed 100%
I may add, ELSE?, NO ELSE,
It is as basic as 123, abc, let a =1;
What does it say about the coder, "8. This is pretty silly." (RTFL)
Edited, Feb 26,2021
HAHAHA.
ClearLY, TEXTBOOK expert(s). (WHO????)
NESTED "IF ELSE" is ABC, problem with that ->ABC => Need I spell this out?. ha.
Should read more "REAL world" project(s) from Github!!!!
One more,
"Two Instance Variables", hahahahahaha. (0.00000001% chance for a real-world program)
(hello, 888)
Related to #1 I would say - if you’re breaking it, you need a reason. And your reason is perfectly valid. In the code I read I would say one main reason I get lost is because some coders have zero respect for nesting depth and vertical height.
Totally agree, and most of these things arent easy for new devs wich makes it only worse if they start doing the wrong ways for the wrong reasons.
According to many psychiatrists, clean coders and other types of perfectionists are sick people and they should seek help instead of "coding": greatergood.berkeley.edu/article/item/perfectionism_is_a_disease
I agree with #1. As long as I don't have to scroll the read the whole method then It's usually fine.
These 'rules' and 'guidelines' are getting more and more scholastic through the years. One dot per line, one indentation per method, 5 methods per class. I was really interested what was going to be next: one IDE per computer or maybe one rubber duck per debug session? Really interesting.
Jokes aside, I watched few of your videos and you have good content dude. The level of knowledge and skill is really impressive. Thank you!
One level of indentation sounds like it was brought to you by the same people who want to limit code to 80 characters per line. I can see where it would be useful in an extremely restricted environment (VIM terminal), but it just feels stupid when you're working in a real IDE. Mostly I find that those hyper-compressed format rules just makes code extremely long vertically, making it harder to read and understand.
First class collections is a thing I've tried on occasion. Sometimes it works, and sometimes it just creates more of a headache (particularly if you end up with 'using' imports).
One dot per line starts failing when you're trying to do complicated linq queries on nested dictionaries or the domain objects you wrap primitives in, such that the objects you're working with aren't directly connected to each other. It's an ideal that doesn't match the reality of difficult algorithms.
Don't abbreviate, but be careful about dumbing things down _too_ much. AddUser() is a lot more informative than just Add() (depending on how the class is used). Basically, name things so that they make sense no matter the context, both inside and outside the class.
Small entities, against a rule made by people who never had to deal with a switch statement, or a state machine, or anything where the complexity is inherent to the algorithm. Extracting methods is often a good rule of thumb, but the risk is losing context in the extracted method vs keeping it with the rest of the code.
"No class with more than two instance variables" - This makes me laugh a bit. Even with your example, what happens when you have to keep track of the game character's stats? Standard D&D has 6 attributes, and tons of derived stats. Maybe you encapsulate that in a 'Stats' class, but now the stats class itself has multiple instance variables. Even if I'm just writing a simple wrapper class or record, I almost always need more than two properties. It doesn't seem like it would ever be workable.
On the last item, I can definitely see not allowing setters, but removing getters/properties is just silly. You put in an API that controls how the fields can be manipulated, but you shouldn't add obstacles to how the properties can be _used_ .
I love how the example of "Don't abbreviate" contains "Id" (abbreviation of identification)
Sometimes an abbreviation is used so often, it effectively becomes it's own word that is a synonym of what it abbreviates, id is such a case. I wouldn't use this rule to replace "HTTP" with "HyperTextTransferProtocol"
Really good video - I appreciated that you didn't just regurgitate a list of rules, but give your opinion on how they face up to reality. Because seriously, how the hell can you fit a useful class into 50 lines of code?
Yeah that one is a bit nuts. No matter how good you are and how much you are not violating single responsibility, it is really really hard to honor that one.
lines of codes mean nothing.
you can fit everything on one line but it isn't readable at all
@@Isitar09 100% agreed with that statement
So the question becomes - what is your limit on class size? I feel like these rules are overly restrictive on purpose. So that when you break it by half or twice, you think about why you’re breaking it and what you just did to your code base. If the rule were 300 lines of code (400 to 500 if you add white space) that’s a recipe for disaster when the rule actually breaks.
@@SixOThree I don't think we should put a number on it, but rather change the rule to "Keep the class as small as possible while making sure it still performs its responsibility and minimizes the violation of rules regarding readability and maintainability.".
I'm so happy that I found your channel! Amazing video as always!
Been waiting for this the moment I've watched your "not using else" video. Keep it up.
Nick your video is totally killing! Thanks for sharing it! Personally I like all this practices (including even 7 and 8) but with numbers adjusted to the team needs (maybe even several tresholds / levels) would nice to have configurable local linter for all the rules (integrated with IDE)
_"A CROCODILE?"_ - Nick Chapsas 2021
Just think what will happen when you add a crocodile to a user.
Hmmmm, that would be like a delete. So, just rename Delete to AddCrocodile. Sounds great to me.
LoL!
@@frankroos1167I think it should be AddCrocodileAsync() because that would be more efficient.
I just LOL'd when I heard him say this.😀
Very good video, I'm really enjoying your content and it's helping me a lot to take the next step in programming!
I sometimes find it strange to name a method "ParseRows" where internally it calls a method "ParseColumns". Someone might want to use the ParseRows method, and then try to ParseColumns after that, not knowing it already does it internally. Maybe it doesn't make sense in some cases..
Yeah thinking about it, I should have written the example in a more real-world applicable way because this one is very generic and I agree, it looks like the original version was better.
Hi Nick, nice tips i will try to keep them in mind, thank you from Portugal
Awesome, awesome!! Learned a lot
And went straight away to one of my applications, to have a look at improving it
9:54 I'm actually a fan of the .Get() method that maps the IConfiguration to a POCO class.
A nice overview. Nick clearly shows that things in practice often are not as clear-cut as theory would like to have it. When it comes to method count and SRP, I would recommend only counting the *public* methods. If the public API of a class does too much, it probably violates SRP. How many methods are used internally to achieve this goal - I don't care. This allows free application of the extract method refactoring, provided that your extracted methods are private (and in 99% of all cases, extracted methods should be private).
Nick seems to get the bigger picture. Respect! The dogma / rules all boil down to DEP / DIP (dependencies are dangerous; limit and protect your code accordingly). All the rules are approaches to dependency management.
Excellent video Nick! I really like the structure and the way you shared you opinion on the subject. :)
Number 9 is a big one people don't talk about enough. In OOP you not only want objects to have separation of responsibility but also *self reliance*. You generally shouldnt ask for data to do a job, you should tell the object with that data to do the job for you.
Sometimes a job requires some data to do, when possible it should be through a function parameter, not setting public variables. Sometimes you have to get some data, ideally it should be through a function that returns an interface with the correct "do your job" methods inside it.
The ultimate goal for maintainable OOP code is to pass around explicit data (especially mutatable state data) as little as possible.
Only about 1% of programmers realise this.
Some really good stuff, I suppose the key is know which rules you really /should/ follow, and which are mainly relevant as warning signs to make you reflect on your choices.
Another rule that I use personally, avoid creating collections with unnecessary (complex) hierarchy; flatten in order to simplify filtering and selection. I suppose this is very similar to #1 & #2 in terms of preferring flatter, simpler logic structures 🤨
Good one bro, will u add more on refactoring series?? I love that.
Awesome video! Well done! My two cents behind this for some of the "rules":
1. Ehhh. not really worth the effort. A couple nested for loops (for { for { foreach { } } }) or something between those lines could bring more readability for debugging, that going into and into and into different functions. I should be able to understand the basic logic of a function when I'm debugging on my screen, not go on an adventure (insert hobbit meme "going on an adventure") between functions and try to understand what is happening and who calls what. My advice on this is: * Keep a complicated method within screen lines *. If you have 2-3-4 loops or IFs, the happily write them in a single method, but extract the body of the last to a seperate one. For example:
It's easier to understand:
for(...)
{
for(...)
{
for(...)
{
DoSomething(foo[i][j][k]);
}
}
}
because I can read (ok, it's going to iterate through these 3, and DoSomething() for each of the objects in the collection). * Basically, in my opinion, it doesn't matter how many indentation levels there are, as long as I can see all of them ending in my screen and figure out the code without going up and down a class). If the "DoSomething()" code is getting big, the extract that to a separate method to keep the loops within a screen.
8. You can't do that in real life scenarios, unless you're coding something magical. In case I have a SubscriptionService, the I must have the CurrentUserService, the PaymentService, the BalanceService, the PermissionsService, the Logger and god knows what else (example from experience). It would take more time to refactor all of them to "keep only two of them", and in the end, it wouldn't make sense (from Service model perspective).
Remember guys, any and all refactoring/coding you do, it should be to serve your needs. You should write clean code with guidelines so it's easier for you and others to understand in the future. If a pattern/rule/whatever is bringing more trouble that profit (now or in the long run), don't do it, it's not worth it. You're not writing a program for the sake of the code, for the code "to be beautiful". You write it because it fixes a problem you have. There were many times I found myself thinking "I spend 2 hours on these classes, breaking them into smaller ones, writing neat descriptions/names etc, when I could finish the implementation in 10 minutes with perfectly fine code".
If you're gonna write some complicated logic that needs to be extendable and you're gonna support/implement things in the distant future, sure, spend some time getting things neat in there. Don't "flex your patterns/rules" when talking about simple stuff. It's not worth your time.
PS: to all other rules, I agree for the most part (not really much to go into, personal preference in this one), really good points, ESPECIALLY number 9. Number 9 should be a holy grail. It's a "Score class responsibility to know how a goal is scored, and not the programs'. It makes the code more readable etc. Win from all sides!
In real scenarios when the logic isn't as microscopic, I find nesting like this impossible to read. Even in the example you shared, I have to see from which for loop am I using indices and try to do a lot of scanning up and down to make sure I am in the right point. It's usually not DoSomething(foo[i][j][k]); but rather a bunch of array lookups on different levels in the nesting. It's just not a good experience. If you find it easier to read then that's perfect. In situations like this I also talk to the team and we agree on an approach. It just happens that every team I worked with prefers extraction over loop nesting.
Regarding the rule 5, one other situation which it helps (which is why I follow this rule) is when you have a stacktrace for a production error, if you have multiple statements with one single pain point each, the line number can express exactly the error you have
Nick, I really like a format of your videos! They are short enough and nails a problem at a time. Easy & clear explanations. Well done! I mostly agree with most of what you are talking about in this video. However rule 7 & 8 a bit weird for me:) I never followed them hard. I agree that they are not rules really rather recommendations to do it if possible and meaningful in a context. My opinion the rule 9 has a bit misleading description. It looks it should name something related to ‘do not break encapsulation OOP principal’ providing access to a private logic by getters/setters/properties which you were talking about. Thank you for all the work you are doing. I am happy to support you in patreon
Yeah 7 & 8 for me are of those rules that I get why they were written but I don't think they can actually be applied in a real world situation.
All principles in this rules list (which I would better call recommendations) are good and applicable in many cases. But numbers in this rules are to hardcore and in most cases can be loosened, especially about 2 instance variables, this is really crazy))
Love. As others say, the critical approach instead of just reading the list aloud.
7: I’d love you to elaborate on balance. Applying ddd, I tend to bloat my aggregate roots “because that’s where the business logic goes”. How do we avoid/ balance that?
Excelent! I always said that's in my crew. I love the auto descriptive code
I propose a rephrasing of the brevity rule: keep functions/methods under 20 lines. This nudges you towards more understandable code, code that is easier to test, and SRP. It also works for code that is more functional than object-oriented. Often you have a mix of both in a project, and it's useful to have a single rule for both.
I tell this always to my members and students that clean codes makes you like doing storytelling rather than following complex instructions.
SetupGame ()
ScoreTo (team)
FinishGame ()
GetMvp ()
CloseGame ()
End ()
Oh yeah, your explanations is really great. Some of them are still violating because i dont have choice but to make it work and readable.
That's the Template Method design pattern from the original GoF.
15:23 I liked the way how you laid out the crocodile here :P
Thanks - I will work to a couple of those that are new to me. Some I'd toss out of the window as things can become unreadable where "nesting" is very deep (say 6 levels). If they are small-ish then I'd let 2 or 3 rather than only 1.
Amazing, Nick! I'm learning a lot here!
Made a web-hook to share this videos to my students! Very useful things!
I want to add a crocodile to the user service.
On indentation. You may also use LINQ to lower indendation. E.g. foreach (var row in rows.Where(x => !x.HasErrors)). Extracting a method here and there may let your code become less cohesive. Also you may end up passing your whole stack to another function which is not good for code modification and clarity.
First class collection. Hard to agree with this advise. Wrapping a collection as a mean of providing API feels like an overkill and erronous usage of object-oriented paradigm. Being overly protective about how someone may or may not work with the data is just wrong way of communication via code in software development. Plain collection and a number of functions to work with it is more than enough.
One dot per line. Very personal in my opinion. Matter of code style and convention rather then useful coding guideline.
Don't abbreviate. Totally agree. Very good and important point. Especially regarding idea of "context" in code.
No class more than... I think whole idea of talking about complexity of code in terms of number of lines of code is outdated. It's just plain wrong.
No Getters\Setters... At some point in my career I realized that binding such methods to some specific class (like Score.ScoreGoal()) is not a good idea. Too much incapsulation leads to dirty hacks when your business domain actually requires you implement complex logic. Incapsulation protects you and your colleagues against mistakes but at cost of making harder to add non standard logic.
Yeah but even though I am using C# I am trying to not use C# specific features to make the video applicable to any OOP language. The HasErrors thing wouldn't actually fix anything since I would still have the nested foreach nesting that I would need to fix. This is why I said that it's not a violation either.
I am loving this!
thank you. i just needed to research this.
Thanks Nick. Could you do a video with your opinion on adding to your code( class/method) comments, or having good variable naming would be enough nowadays? I assume you already have a video about this topic.
Also, could you do a video about LINQ(lambda) queries and the performance measurement of some advanced examples (EntityFramework would be ok). Because I didn't seem to find any video about such topic (DB get/update/create). SQL would be great.
And last but not least, what do you think about moving WPF based applications to webbased, in terms of: will WPF be popular in the coming few years or is better to focus on .NET Core 5/6 to be more future proof?
Re: "tell me what you use..." I'm lucky if I even get to use SOLID at work (where the motto is get code out fast and don't worry about if we need to change business logic later)...
In my *personal* projects, I try to use several of these rules on a regular basis. 1 line of indentation, 1 dot per line, and don't abbreviate (both don't be redundant and don't use actual abbreviations). I also try to avoid using the else keyword, but I don't think I would ever set it as a firm rule for my code in any environment. Yes, use return, continue, or break if you're doing validation or something similar, but in other cases, I may end up using "else" when there is a good reason to do so. In my personal projects, because they're typically smaller projects and I almost never have any tables with 15 or so columns and typically fewer than 10-15 tables, I can keep small entities, but I kind of feel like that's cheating at that point. I would just get annoyed with a lack of getters and setters in my personal projects... Just a personal preference though, I guess. I understand the point of that section, but... I've just never really seen a point in using them in personal projects. Although I agree with several of the other for larger projects, I don't personally have a reason to use them in my personal projects.
If I were working in an enterprise environment where code cleanliness is actually valued, I would probably pay attention to "No Getters/Setters/Properties" and "First Class Collections" more, but I'm not sure how often I would really think about "No classes with more than two instance variables". I get it for the most part, but... like you said, in the case of a service class or something similar, inject whatever other classes you need. Keep the class small enough that it's manageable, but if you have to break it up so that, what was 100 lines of code with 6 or so methods is now 6 different classes with a combined 200 lines of code... that doesn't make much sense, especially if the methods are all related. I really enjoyed the primitive obsession topic and I would probably use that if I ever ran into a scenario where validating something like that would be an issue. I just don't run into something like that very often.
Great content, as always! Looking forward to your next video.
For any instance of a class where the class is a "leaf" class. (No dependencies to injected behaviour in the constructor)
If your method has a return type: It can only have the purpose of returning said type. (No (side) effects)
If your method is a "void" return type, it can only have 1 (side) effect.
This forces you to divide your class methods into easily testable "chunks", and ensures your methods stay small.
Now for anything that isn't a leaf, try to adhere to the same principles, but of course, when you need to do multiple actions, like, CalculateScore, CheckWinCondition, CheckLossCondition, etc.
But each of these examples will have adhere to the rule, so there will only ever be 1 action associated with a method call. This makes the code readable, because you can easily anticipate what a method does, and method complexity and size, will always remain low.
It makes your "abstraction layers" or "paragraph levels" easier to read through.
Following this rule of thumb, makes your code read like words on a page, rather than "code".
And, it doesn't specify an arbitrary "line count" or min/max setting, merely, that the code has to be cohesive, as a "single effect" is the purpose. (Single responsibility pattern)
Forgive the pseudo code here!
Say:
public class Player{
private int _score;
public void method ScoreIncrease()
{
_score++:
CheckLoss();
CheckWin();
}
private void CheckLoss(){
//whatever
}
private void CheckWin(){
Whatever;
}
}
Violates this pattern, in the public method, it has 3 actions-> increase score, check for a win, check for a loss.
Now you must do all three.
So you have to extract each private method into a separate leaf class, That controls how to do whatever you private methods are doing.
So it becomes:
public class Player{
private int _score;
private readonly IWinChecker _winChecker;
private readonly ILossChecker _lossChecker;
public Player(IWinChecker winChecker, ILossChecker lossChecker)
{
_winChecker = winChecker;
_lossChecker = _lossChecker;
}
public void method ScoreIncrease()
{
_score++:
_lossChecker.CheckLoss(_score);
_winChecker.CheckWin(_score);
}
Now Player is NOT a leaf class, and can have multiple actions, but the effects, are split out into separate classes, and the behaviour is maintained separate from the Player class.
Checking for a win or a loss, may also well use variables, that do not belong in the Player class, like a limit, that may be map specific? or difficulty specific, etc.
now these values can be DI'ed in where they belong. It forces you to separate into Single responsibility, without actually thinking about code design.
Also the naming convention:
All interfaces, abstracts, classes, etc. MUST BE NOUNS.
All method names MUST BE VERBS.
This makes everything make way more sense, when reading.
I started coding 7 months ago with a game project. And it's amazing to see me applying half of these guidelines without even knowing they were being taught; I just applied them because I thought my previous ways were inefficient. It really reassures me to know that I'm pursuing what suits me, an efficiency sorta-freak.
Great video. Thanks!
Crocodile joke got my like 😅
Thanks for sharing!
re small entities, I use the 30- guideline: Methods under 30 lines. Classes under 30 methods. Modules under 30 classes.
are those small entities any more? I think 30 methods are way above a small class
Thanks, Nick. The big prob, in all points you mentioned, is that every programmer in team has to follow them or at least be near the idea. In my team I have 50+ yo guy. He dont want/afraid? open something new for skills... He codes "helper class", if you know what I mean, and "hellper" has 52 methods. I have to support this code somehow... So in real life this is mostly the dream refactoring. Thanks again, and keeeep coding!
Yeah I totally get that. Old habits die hard. It’s a matter of showing with examples why one way is better than the other. At the end of the day the biggest thing is for everyone in the team to be aligned and happy
I'm currently refactoring a collogues code. He has multiple nested functions that make it hard to understand what is going on. That can be a multitude of factors, but I find that he uses naming conventions inside the public function that has a nested private function called "getWorkDay" and another private function nested in that function called "getNextOpenWorkday". Since I'm unfamiliar with the code and logic I have to jump into multiple functions. I forget what I had just seen on the previous functions therefore I have to navigate back and I quickly forget what was on the previous function. I don't really like this breadcrumb approach if it isn't your code. It makes it more readable yes. But, I've found that when it comes to refactoring it becomes more of a hassle when it isn't your code.
When split method code into submethods, the main thing is not to overdo it, since the code becomes unreadable, you have to jump from one method to another and try to keep the context in your head.
The last 3 rules are far too absolute to be followed. It is more interesting to understand what is the idea behind those rules (like you explained perfectly in this great video).
For instance, I have a specific class which is more than 2000 lines... BUT... It is still super readable and easy to debug/modify (not just by me, but by everyone, according to the feedback they provided to me), just because it is very well organised, commented smartly and makes a lot of sense regarding the business issue the class is trying to solve. Of course I can split it in 20 different classes... but it will not make it more readable or esay to use... quite the opposite actually
SRP prevails over everything else.... this is the only rule I strictly follow :) all the other ones can be adjusted
Regarding "No Classes With More Than Two Instance Variables":
I worked for a shipping company. I built a model around their business processes. These processes used to be broken down into multiple stored procedures, middle tier logic, and front end logic... when combined, managed their shipments. I was able to consolidate this code into a collection of classes.
Shipment
- Items
- Documents
- Clauses
- Bonds
- Charges
- Payments
This was a single shipment class with multiple collections attached to it. I was able to load it in a single call to a stored procedure. The best part was that all of the logic for the shipment could be tied to this object... including unit tests.
This was the best design - even though it violated the "2 instance variable" nonsense. That's because the system had to consider the types of documents, clauses, bonds, against the items... then determine charges & payments as a "single entity". From this single object, you could tell
- Did it have an "invoice" or "BOL" document?
- Hazardous cargo? If so was the correct clause associated with it?
- Was the shipment paid for?
- Was it bonded correctly?
- Was it ready to load, manifest, ship, receive, etc.?
The best part, 100% of the business logic related to a shipment was consolidated and easily tested.
Sounds like you didn't need the OOP/SOLID nonsense to begin with.
"What you think I'm editing? A crocodile? No." Lol nice one. Great video on some good disciplines.
Absolutely agree with not abbreviating or adding the name of the class on methods.. something that all Jr. Devs tend to do (including myself);
Funny that on rule 8 Nick has property PlayerName and PlayerId 😜
I disagree with the AddUser/EditUser example in UserServices, sometimes it is better to have it be more explicit to avoid confusing a list for a instace of x. Never step on native names unless it's very clear.
8:45 "And what does create mean?" 😂 That happens to me too everytime. Btw, create does not do the Guid.NewGuid() which might inadvertently fix a bug if the client already autogenerated a guid on creation.
The absolute best part of this video. "... what do you think I am adding here? A crocodile?" rofl
Not watched the whole video yet, but on the first point (Indentation levels), this is ok for memory rich targets (i.e. PC, servers, etc), but when coding embedded systems with limited RAM, this 'guidance' creates more code space, uses more stack and has reduced performance (due to the extra stack push, pop context saves and jump calls). Not an issue if your target has the power and resources, but not all code is written for resource rich resources ... just wanted to highlight that for any coder that moves from PC to embedded systems. 'Readability' of code is not always the #1 consideration when writing code. Don't get me wrong ... I find you videos very interesting and good food for thought with some good guidance and explanation.
Completely agree. This is very hard to apply on lower level systems. I’ve only seen them all applied effectively in microservices and higher level systems
Correct me if I am wrong. When coding for embedded systems, you write in C, C++ or Assembly language and then use the compiler or another tool to compile to machine specific binary instructions. Your original code should still be readable, those spaces or identation will not be in the binary instructions so don't have any effect on RAM or memory.
@@awmy3109 The removal of indentation in this example generates more functions. As you say, whitespace is disregarded in any complier/interpreter. The code generated by smaller granular functions depends on the optimisation of the compiler. If optimisation is low, then jumps to functions are preserved, unless you specify that the function is to be explicitly 'inline'. Not all code compiles down to what you expect it to unless you force it to by your coding structure and pre-compiler directives (or language keywords). In systems like windows it doesn't matter (unless you specifically code for performance). The point was, that not all code structure at a higher level is suitable for every target. If you write game engines, making the code readable is not necessarily a move in the right direction (unless you want to extensively re-use code in other projects)! You can program in C++ and make things object orientated for embedded systems, but it comes a cost. Just want the young players here to be aware that one coding style/methodology doesn't fit all end use applications and that readability sometimes comes at the cost of performance. I have written unmanaged code in c# and written static libraries in C and assembler to get performance hits in the past. Not all code you write may be at the higher level ... after all, someone has to write the low level optimised stuff!
As an exercise, compile Nicks original example to native code (rather than JIT), then do the same once you make it more 'readable', see if the .exe is bigger, smaller or the same. Put it in a disassembler and see what extra has been added.
@@simonbaxter8001 I get it now. Interesting.
there's an eternal battle in OOP between conciseness and strict adherence to principles... I generally tend to err on the side of conciseness. the principles are something you need to be aware of but not necessarily devote your programming life to.
Hi Nick, great video! Sepaking of "one level of indentation per method" - how it can be applied to all kinds of try-catch mechanisms? having something like main method, which contains try-catch, and calling some "internal" method in try which contains dangerous code seems kinda ugly, or is it just tradeoff and we cant really do anything about it?
Yeah so to me things like try and using that force intendation are one the "grey" area where if I have them then I might as well violate this rule because the alternative is to extract them which is good for some cases but not so good for some others.
Robert Martin lists many useful guidelines in his book "Clean Code". One of the rules is: "Each method should do one thing only. Exception handling is one thing."
This guideline has worked well for me. But as always, there are always some cases when it doesn't make sense.
1. It's rather the matter of good refactoring. If you have many levels of indentations you should rather figure out if something can be improved. However sometimes it is more clear and easier to debug if you let e.g. 2-3 levels of loop instead of creating many sub-functions. Too many classes and functions can be also a headache when you try to understand someone's code.
2. In general agree, but not treat it so strict
3. "It depends"
4. In general no reason to do so. I saw only a few cases justifying collection class. Most of the time you should probably create child class having additional Guid property, instead of moving dictionary to another file or just think about using another collection type. In my opinion, in case of collections it is better to have them as close to the build-in collection types as it is possible, because it minimizes errors and keeps code clean.
5. 100% agree
6. 100% agree
7. "it depends". For large classes/parts of code you should rather be reasonable and follow KISS principle. Too many small classes can be also devastating both for clearity of code and IDE performance.
8. "It depends"
9. "It depends"
For me such "laws" and "principles" are rather guidelines, and most of them you can break, BUT you should be aware why you doing so and have a very good justification to do so.
I really love this guy video's, Keep it up Chapsas
Another great reason to minimize your indentation is for clarity of pull requests. I can't tell you how many times I've PR'ed deeply nested code where the level of nesting changed, causing the PR to be super verbose and confusing. Has this led to bugs creeping in to production? Absolutely.
In your outro, you sound like a cross between a rapper and the terms and conditions voice over actor on an advert!
Calisthenics is a term I never thought I'd find in programming 😂😂
At 3:37, the *if(row.HasErrors)* is a _filter_ - it would be better to replace that with *dataTable.Rows.Where(r => !r.HasErrors)* in your *foreach* loop.
It actually isn’t. It is less performant for no real readability benefit.
Wonder about your opinion on nested classes. For example, I have UserPaymentService and inside this class, I have 10 private methods and one public which accept a primitive value. To easily operate with those methods I could create some custom class to represent the structure of a user which makes sense ONLY WITHIN this class and has structure adopted to work with those private methods and never will be populated outside. So I will create this nested User class instance from primitive values and operate with it inside the UserPaymentService to simplify work with this service and private methods and keep some decisions from internal methods inside my nested User structure. Is it good practice? Should I anyway move such classes to separate files and keep them internal?
I must admit for me the first example, I preferred the method as one, I hate having to jump through multiple methods when reading code, maybe that’s just me but I’d rather having nesting (maybe extract the inner loop) but definitely not the outer loop too
About the first guideline: Only use 1 indentation. What about nested for loops that you need to navigate multiple dimensions? For example a Matrice, or something 3- or more dimensional in maths or engineering. I think then it's more readable to leave them at the same place, do you agree?
I would just keep the nested for loops for that, then again i am just a random math major with no cs education who learned c# from tutorial videos for unity, but hey i mean thats what i would do
Guilty as charged, regarding the entity size.
I have so much functionality in some classes, that I exceed 250 lines a couple of times over.
To be honest, I tried to reduce it, but I don't know how to reduce/extract functionality in a meaningful and logical way.
And then the time constraints... :)
One day, unit tests will become dinosaur that every developer is prohibited to even try.
What's clean and not is a personal opinion I guess. I think several of the examples are relative to the language in use and not generic for programming. And I believe some of the rules can be seen a little bit different.
Some examples, personal opinions I might add.
Rule 1: intendation is not bad in a context where the code is divided properly. The proposed solution to do procedural abstraction is the key here. Not the intendation itself. My rule of thumb is not to have code that spans "pages". Divide with procedures and classes if needed. In this way, you probably end up with less intendation as a result of the way you divide code. Not as a rule itself. So I guess we're talking a little bit about the same thing - just from different perspectives. A five or ten line long snippet of code with perhaps level 2 or 3 intendation is not unreadable. But if it's pages long...
Rule 2. As described in your separate video, removing else and using return statement is less readable as far as I'm concerned. Else can be misused and is often overused (especially when code becomes too long and is not divided, see rule 1. I believe, again, that the right amount of procedural abstraction and use of language features is the key here. Not the existence of the else (or lack of).
Instead of banning the use of else, I try to think "extensability". Ask myself: if I where to extend this with more values or more use cases, will I have to add more cumbersome and awkward elses (or other code that makes things hard to overlook). If so, then I try to think how to code it so that extension is less of a burden and the risk of extending with unclean code is as minimal as possible.
Rule 8. Agreed. Two seems way too low. Again, use common sense and make sure code is not too long horizontally. Divide the code using different approaches. But I have seen projects where many small classes in many files etc etc are way too far taken. It gets hard to overview. Everything is so spread out that it's hard to overview and get a grasp of.
Rule 9. Domain driven design. It's a good thing (and domain driven security for that matter). That has to be the thing to think about, not the getters or setters themselves. But I agree, it's easy to misuse them and expose too much and giving the wrong level of control to the "user". Getters and setters also can give the impression that it's "just" about that. Get and Set. But it's easy to hide tests, logging, side effects of all kinds to a get or set of a property. It can be a good thing too, but it is very important to document what a property setter och getter might do under the good in terms of performance impact or side effects. As with any method I guess - again - it's not about the getters and setters themselves but how they're used (or misused/designed).
Great video, but i will do a little nitpicking right now: In 9 rule, the Main should not know about "AwayTeamScore". Goal happens inside a game and without a specific game, you can't have a score. So, i think that last line should be game.ScoreGoal(awayTeam) and the Game class should have the internals about how that score is recorded.
Many ways to skin the cat. I could argue that it's the score's ValueObject's responsibility to know how to increase itself and not the game since the game is a host fo the score. You would have a ScoreGoal forward method but it would still be a wrapper around the Score's object ScoreGoal method.
"What are you editing? A Crocodile?!?! NO! It's the User" 😆
Regarding length of methods and classes: you make a very good point - don't violate SRP, and any length goes. The problem is, defining SRP is actually pretty hard. The original principle is vague at best, and understanding it requires you to go a bit deeper into stuff like levels of abstraction. Most online guidelines, as far as I remember, are just as vague as the original principle. I personally follow these rules:
is most probably not SRP-compliant and should be reviewed by a senior dev - again, it may be a large transaction or something similar, but most likely the developer has misunderstood SRP. As you said, c'mon 50 lines is a ton.
But those are for me and myself only. I won't force this on others, contrary to SRP.
I definitely disagree with 2 fields per class. You will violate that by injecting AutoMapper and a Logger. 4 is more reasonable, and even then it again boils down to SRP. Let's not go nuts tho', anything like 6 is most definitely an SRP violation. On the other hand, between having 6 levels of abstraction instead of 4-5, and a single class slightly breaking SRP... I'll choose the latter. SRP is great and I love it, but having to delve 6 layers deep to understand the entire flow is just too much. Then again, this can all be solved by a better design and use of Mediators and Facades.
I am curious to know how you feel about abbreviations in lambdas, and linq?
I tend to abbriviate on simple/obvious types and use better names on more complicated ones
14:10 - Before watching further: my prediction is that CPM may mean Clicks Per Minute - we are talking about CZcams channels after all.
But if that would be a quantum physics - I wouldn't know.
But why would I try to develop a program for quantum physics?
But I agree - writhing something like RSTM_2 doesn't help ;-)
P.S. And CPM mean "Cost Per Mill" - well... yeah. That proves the point!
Developers always want their code clean until deadline and customers forces them to break "clean" rules :D lol
by also saying when you reach 250 lines , you have to reconsider , isn't the same thing that you are dismissing in that same chain of thought?
yes, I am editing a crcodile. Awesome content thanks!
11:00 Why would you pollute the local scope with new variables? Such code is fine for debugging purposes, but i find it too obfuscated for production.
The main goal is to make the code readable, how you approach depends on you
1) I don't do this also I would remove the continue by rewriting as if (!row.HasErrors) ParseColumns(dataTable, row, cache);
2) I do this but only if I am able to exit from within the if. I would still use the else for everything else.
3) I agree with others I've seen in the comments. This is very situational.
4) I would do this only if it will be used elsewhere, if its not what is the point in making a new class for it. I would just leave it in the service and use access modifiers appropriately.
5) For debugging I can see the appeal but I tend to do that after the fact. I write code with multiple dots and when debugging if needed I separate things out.
6) I agree with this but out of habit I tend to do things like crntTime instead of currentTime but only for private or local members oh and parameters.
7) I don't agree with this at all, but I do agree with single responsibility principal. Why create a limitation based on number of lines?
8) I don't do this. As long as it follow single responsibility I don't really think about how many I have.
9) I agree with this or at least with the example you provided.
When people say things like "classes should be fifty lines" I look at a 4000 line class and wonder what is going on in the world.
Yeah, for example WPF source contains few thousands lines of code for each widget class... And it requires you to do the same thing if you are creating your own widget!
Great. I never heard of these before. Btw i want to point out you violated one dot per line rule in the last example
Well the interesting thing is that this happens in the Main method of an executable so there is no ruleset really applied there. It's just for demonstration purposes. You are right tough It would be a violation if this was something like a GameService class.
@@nickchapsas thanks for responding in such a short notice
You should try putting "{" on the same line, NOT on a separate line. Try it, and after a bit of a culture shock, you might find your code is much more readable. { and } replaced by "begin" and "end" also greatly improved readability. Juts for fun reformat some code using thes two thing (I know it wont compile), and you mind might be blown away at how hugely more readable you code becomes. The reason is { } requires your brain to work hard to interpret it is it requires context,. "Begin" and "end" stand alone, so your brain does not have to struggle with context, ...MUCH faster and less effort to decode. This is why prof. Niklaus Wirth, and interestingly enough, Anders Hejlsberg, advocated it for the same reason, ...that is until M$ made him do otherwise for marketing reasons. Remember Anders Hejlsberg also created Delphi, which is more productive for various reason, and begin/end readability is one of them. And yest I coded in C++ for several decades, and at first hated begin/end, but I am glad I got over it because now I code very fast and clear!
I agree with you abut nesting, too deep is VERY BAD. As for abreviation, not good, but long names are EVIL because they 1) take long for your brain to process, and 2) easy to mistake is similar names used elswhere!
NoClassesWithMoreThanTwoVariables: Injects IMapper and ILogger, ok I'm full :D Though, I've used facades over some areas where I actually needed 5-6 services. By extracting it all behind a facade it was more manageable to unit test.
I learned coding when memory was tight. Abbreviating was a very hard habit to break. Totally agree though, and never do it any more.
@@matiasmiller6119 BASIC on a VIC-20, then BASIC on a ZX Spectrum before getting a PC as a teenager and learning Turbo Pascal. I use C# mostly these days.
The rule for classes not having more than two instance variables makes much more sense if you exempt the fields that hold injected services. Even then, two is fairly strict even if you're talking about true "state" fields of an instance. Are you going to make an extra class just to avoid having three fields? Maybe not.
Id is also abbreviated. It's really Identifier.
17:20 the number of lines limit is soooo weird, because depending of how you code, you can have a portion of a code made in 10s of lines or in 1 line. And it is much worse if someone tries to cram as much as he can in one line. I saw people who make one line monsters, for example in property:
int X {get {check one; check two; do something; calculate value; return x;} set {check one, validate; something; x = value;}} //all in one line of code
So any role that encourage this, is not too wise.
czcams.com/video/gyrSiY4SHxI/video.html ("One Dot Per Line")
Although true during debugging a single line doesn't allow for individual segments to be evaluated independently,
by creating 3 variables (local scope) this is 2 additional allocations as well that persist in memory until the method ends.
Since in this example those 3 local scope variables are essentially static those really should be instantiated once elsewhere and used repeatedly since those aren't changing,
something better could be an underlining framework that injects an object containing those static values async or on another thread or some other way to share the object-asset data.
And notably, properties are comparatively slower than direct variable usage so those should be minimized for speed. Why classes? just use structs, also a performance bottleneck.
All of those passed strings should also be defined in a centralized area, to make modifications easier without scanning through methods, and ideally by reference not by value but y'know that can introduce issues, particularly when declaring a local variable with a readonly string by reference (and passing that variable), or such, with threads and/or object-data sharing.
This is what I hate about javascript particularly, yeah you just pass a string value to call a method but you must know what that string is to call,
ideally those strings should be enums to more effectively determine what the method is named to call. This is the term: 'magic numbers' 'magic strings' 'magic uh-erm-references' to which you don't know what those are without explicit behind-the-scenes knowledge
I think you are confused and I see a lot of misunderstanding in your response. Firstly, the jitter will inline those operations during runtime so there isn't a speed performance implication neither in speed or memory. Also just because you don't assign the result of a method on a variable doesn't mean that the memory isn't allocated. It is. You can actually try that yourself. Check out this pastebin and run it with BenchmarkDotNet to see: pastebin.com/7mhqxAzF . Also memory gets disposed when out of stack frame only for value types. Reference types will stay allocated until GC pressure makes a collection kick in. Also structs don't have the same properties as classes. They can actually end up being slower depending on the usecase. Strings are being interned anyway so they aren't a problem. I think you have quite a bit of misunderstanding on how memory works in .NET and I do have several videos talking on the subject so you might wanna take a look at them.
great
lov this type of content 69 :3
I think another way to to describe rule 8 is to place your classes in 3+ Normal Form.
Are you using Ryder? I’ve used resharper for years is it worth?
It totally does
@5min: I don't like "continue" - The For-Next-Construction in C# still needs too many lines. object.foreach() cannot do the job (can it?)
I like how you warn not to take these dogmas too seriously. Too much code gets even worse when people hold on to a dogma without proper understanding (and frankly, people creating these rules probably know better that their own warnings of not making it a dogma will be ignored). I will take on point 1 (only 1 level of indentation), how this is harmful:
By looking at the original code, its a matrix traversal. Maybe the row lengths differ, but lets assume they don't for the sake of naming it "matrix traversal". Anyone who dealt with 2d data structure at least once knows, that to do it, you need a nested cycle. If you extract nested cycle into another method you:
a/ lose direct sight of that it is a nested cycle/matrix traversal,
b/ have to take the scope of the method and expose it as parameters (here only one, but it can easily be much more, all things you precalculate for your iterations)
If you just extract what happens for every cell via a Consumer / delegate, you encapsulate the matrix traversal into one method that is perfectly readable, maybe still under 10 lines (that contain meaningful code) and don't have to propagate the scope. You have a method you can reuse, which does not need to know about cache or anything from outside. That is much more useful than what this dogma forced you to do. Of course, nested cycles can always be flattened using flatMap, but readability can be argued then.
One thing I would say is, in the end rule, you implemented ScoreGoal in an "AwayTeamScore" object? Surely the ScoreGoal would actually be better and more logical in an AwayTeam object. The score wouldn't score a goal, the team object would itself.
An away team isn’t necessarily playing a game or any team
For that matter. Why would the team know how to increase the score? Maybe the game object would be a good fit as well but definitely not the team itself
Gold!
From a performance perspective, how negative does clean code like this affect performance? In general extra method/function calls result in worse performance. I'm not sure how it works in the landscape of C# though given an assembly language background.
I actually have a video coming just for that topic. Long story short, the benefit of cleaner code outweighs the nanosecond that you will lose. On top of that in C# the JIT compiler will actually optimise the execution during runtime and completely remove those methods and treat them as code that was invoked inline instead.
@@nickchapsas This was what I suspected and I do agree about the benefits. My thoughts where purely scientific curiosity :) Looking forward to your video
What was the reason behind not using "else"? I found that SharpLab shows that any code block where you remove the use of the "else" statement the lowering process just added it back in, I am a little confused. Is the use of "else" just to ensure more readability and clarity in the code?
Yeah there is no performance reason. It's just for readability which is subjective to begin with.
Here is why I avoid else:
if (someCondition && otherCondition || thirdCondition) {
doSomething();
}
else {
doSomethingElse();
}
It's hard to reason about the code above. When is doSomethingElse() called?
You can always right click find if you need to see the code.
16:11 "...you should not have a class woth more than 50 lines of code..."
*Meanwhile my 800 line Matrix class*
To be fair,its for a shitty math library i made,and operator overloading and other functionalities require those many lines.
I find advice number 1 slightly situational. I mean it is definitely not bad, but, if you find yourself overextracting to too many functions in a file or a class then you have to start asking yourself if you are missing some other abstraction, because your code seems to do too much.
But even then there come corner cases, like methods that capture a specific workflow that might be an overkill to separate, like e.g. constructing an XML file to send via network. The nesting there if all would be done to one wrapping method is unavoidable. Probably if you make some fluent builder or service level class and start really elaborating your solution the resulting code would be better but sometimes.. you just want to get the work done.
And then there is JavaScript.. I recently faced JS code I didn't write myself which was 25+ functions with no apparent who-calls-who relationship. In order to make sense I started to localize them via -excessive- nesting and indeed it evolved to 3 main methods that trigger the call to all the rest. As expected every one of main methods is quite long, but all localized functions are inside with their declarations and their applications - they are not inlined. I have to admit, so far the code stays like this. The problem is the lack of namespacing in JavaScript. Functions shouldn't be long but even if I turn them to classes with rich dependency hierarchy, that doesn't solve anything. If you want to have precise scoping, that a function is only useful exactly here where you call it, you can do that only via nesting. Importing, requiring.. well they are ok, but do we really need another file? I am not sure. That is where I would probably had to skip the first advice. Since closure is a predominant mechanism in JavaScript I wouldn't mind excessive nesting there.
I guess what I am driving at is, personally speaking if a method captures a complete workflow that all interior parts are non necessary for the rest of the code then, although it feels a little bit guilty, I could live without following this advice. Especially with code folding of an IDE:P
Yeah I "violate" rule one several times especailly when I need to have try clauses or using statements that force intendation due to their syntax. Ultimately this is about adapting those originally defined rules into your own needs and usecase. The level to which language support them varies quite a bit.