9 "rules" for cleaner code | Object Calisthenics

"A crocodile?!?"🤣🤣

DynamoDdSection

Although it is about OOP , the very first point's solution is to use functions - irony !!

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
lov this type of content 69 :3

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<T> / 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.

I am curious to know how you feel about abbreviations in lambdas, and linq?

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).

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.

I dont agree with many of the things surrounding rule number 7.
So [the author] wants you to have one line per actual operation, as many methods as the code can feasibly be split into and then also spit classes by line length?

I dont know what you guys write as code, but i absolutely hate writing a 280 LOC/250 SLOC algorithm in a class over maybe 4 or 5 methods, which all do well defined things, with a clear program flow and no black magic - and then returning to find it refactored into 6 classes and 8 interfaces 1200+ LOC, dependency injection from outside everywhere to keep the complex net of interpependent stuff alive and super extremely loosely coupled

for something that will only ever actually be used as instantiating the object with a parameter and then using one of two methods
and if you ever want to change it again you need someone to spend 3 hours trying to familiarize themselves with the structure instead of 10 minutes reading the original class.

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 😜

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.

Personally I don't fall into the trap of "one level" only. I've seen it done, Delphi VCL being a prime example, and it just leads to dozens of layers of code to do simple tasks. It's such a bad idea I can't believe people still push this concept.

If you pay attention to the greatest programmers of all time, they'll pretty much all say to keep your function simple: with a rule of thumb being less than McCabe Complexity of 10. That's a level that's been proven to be (1) understandable by the reader and (2) within the acceptable limits of testability. Your basically advocating for a McCabe complexity of 1-2, which is frankly ridiculous.

I mean seriously, what is easier to understand; A single function with a McCabe of 10, or 10 functions with a McCabe of 1?

Crocodile? haha

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.

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 🤨

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:
<=25 lines is generally fine, quite often just breaking the lines will put you over 15. This said, it doesn't mean a 20-25 line method is always SRP-compliant, but most probably it's acceptable.
25-50 is shady, so it requires the reviewer to pay extra attention to SRP - it may simply be a database transaction that takes 2-3 EF calls to complete, or somebody has just broken SRP in 3 places.
50> 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.

Rule 1: Learn Clojure
Rule 2: Stop using OO languages
Rule 3: Regain your sanity

The absolute best part of this video. "... what do you think I am adding here? A crocodile?" rofl

Developers always want their code clean until deadline and customers forces them to break "clean" rules :D lol

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!

I don't think you should ever abbreviate, I don't see why C++ or Go could be an exception.

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

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.

"What you think I'm editing? A crocodile? No." Lol nice one. Great video on some good disciplines.

honestly... nobody ever taught me what clean code is. and i never read any book. all i'm doing is using my common sense and see what i find out? i'm following most of these guidelines.
however, while i do agree to all of them, they also all sound very harsh. "not more than 2" are you kidding? the consens is, kepp it as low as possible. and you could even say "if you have more than 2 it indicates you should rethink your code-cleaning" but as a rule this sounds way too harsh. especially when you follow a proper class splitting.

for me code quality is what cooking is for asians. just use feeling ;) (love ya uncle roger)

oh and writing a dictionary class wrapper is just stupid. i'd actually even like to add some principles to even prevent that, honestly. the more files you include in your code the less overview you have even if you have a proper folder structure, if you create an object wrapper whenever you have a dictionary, you will end with so many classes that you end up using full solution search... and this isprobably the first sign that your clean code fucked up XD

an additional class is used to making logic clearer, structure it, split it. this thing here does not reduce code lines, it increases them

I disagree with the AddUser/EditUser example in UserServices, sometimes it is better to have it be more explicit to avoid confusing a list<x> for a instace of x. Never step on native names unless it's very clear.

Very good video, I'm really enjoying your content and it's helping me a lot to take the next step in programming!

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!

The main goal is to make the code readable, how you approach depends on you

most of these are good when you slightly lax them, for example, 2 levels of indentation is fine, any more is a warning sign.

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.

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.

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.

"calisthenics" sounds like exercises you do to get better at your job and not necessarily techniques at your job

I am loving this!

I don't agree with any of these :D

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.

One day, unit tests will become dinosaur that every developer is prohibited to even try.

Referring out to other videos is fine... but don't just outright skip it in this one.
It comes across in a bad way. Don't have to give a 20+ minute spill about all the details but at least cover the very basic concept and then refer to your other video.

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

This is trash tier, low quality programming advice.

Id is also abbreviated. It's really Identifier.

Most of these are basically derivatives of SRP rule :)

lol.. Soccer.. 🤣🤣🤣