Okay, our first actual speaker today is Frederico, who is a maintainer of Metal LB, which I personally
use, thank you, over at RedHeads and he'll be talking to us about cognitive loads.
So a round of applause for Frederico.
Yeah, it works.
Yeah, so today I'm going to talk about cognitive load and how it affects our code base, why
it matters and how we can reduce it.
And the reason why I put together this talk is because over the past, I would say, two
years, I started contributing first and then maintaining the Metal LB project.
Is anyone using it?
Okay, so if it's gotten less stable, that's because of me.
But by doing that, I started reviewing a good amount of PRs and over this period, I kind
of identified the recurring patterns that I was keeping asking and asking over.
And those recurring patterns, those scattered suggestions that I try to give in code reviews
are what this talk is about.
In terms of code, Metal LB is a nicely sized project, not too big, not too small, and I
think it's worth keeping alive.
So some quick words about me, I'm Frederico, I work for RedHead, I'm part of a networking
team in charge of making the OpenShift platform suitable for telco workloads.
That means that I touch and contribute a lot of these different network-related projects,
but that doesn't mean that I'm a network expert because I'm not.
So don't come asking to fix your router, as my parents do because I won't.
All of these are my handles.
Probably the most annoying one needs to be adjusted, but you can find me there.
If you have questions to ask, if you need to provide feedback, I'll try to reply.
So let's start with cognitive load, and this is the Wikipedia definition.
Cognitive load is meant to be the extra energy, the amount of effort that we need to put in
place to understand something that applies perfectly to our codebase.
It might be because we are reading something that we wrote years ago where we were less
expert.
It might be because we are trying to review some code that somebody else is trying to
push to our project.
It might be because we got a bug report and we need to correlate the behavior that we
get from the reality and what we understand from our code.
And the less energy we spend, we are able to spend the better because it might be evening
and we might be tired and we might have some urgency about that.
That's why it's so important.
And sometimes, this complexity is proportional.
This extra energy is proportional to the complexity of our code.
Think about cryptography.
Think about ultra-optimized code that runs in embedded systems.
But some other times, it's not.
Take this example and take the same run through another skater.
This takes a lot more energy to understand that this function prints hello world.
So this is to say that we need to put an effort because that effort gets our reward in terms
of speed of development and speed of understanding.
So say that a disclaimer, not everything is black and white.
Of course, there might be exceptions to the suggestions that I'm going to say.
And this talk is more or less a collection of scattered robots that I collected from
sources that I trust.
So in case you don't like them, blame the sources.
In general, I think that the stuff that we write should take care of two sites.
One is, of course, the implementation, and this implementation is pretty clear, I guess.
This function is just doing the sum of two numbers.
It's easy to understand.
We can't argue with that.
But what if we land on a code base that is doing something like this, and this takes
more energy compared to a better version of it, where the function is named nicely.
So we understand what it's doing.
This is to say, and this is something that is going to be a requirement in this talk,
that what matters is not only how we care about the implementation, but also how we
care about the users of our packages, of our functions, of our objects.
So let's start with the first item, which is the line of sight.
And this is something that I believe every good and idiomatic code base should try to
foster.
Basically, we have this leftmost indented line where all the happy path leaves, and we have
this indented one where we handle all the exceptions.
And I expect every code base, which is well written where I land to, to respect this rule.
And there are a few tips to do this.
It wasn't me.
So these are just tips to do that, to implement this.
And let's see why it matters, how it will make our code base better.
This was more or less a real example that I got from a real PR, and it was really hard
to follow all the special cases.
And so I tried to give feedback and try to hammer it with suggestions in order to leverage
early returns and flipping errors, removing else, when a CNL is something that I try to
get rid of.
Like, it's a red flag, and I think three times before allowing it to go through.
And then leverage more returns and then, yeah, leverage more returns and then, sorry, yeah,
trapping into a function so we can leverage even more returns because now we have a smaller
scope.
So we got to something from something which looked like this to something that looked
like this.
And I dare you to say that this is easier to understand.
And remember, like, this is understandable, but this requires a lot of energy.
It's clear.
It's better because of all the reasons that I already said before.
There is this nice blog post from Matt Ryder about this very same topic.
He more or less gives the same set of advices.
Linocyte is not a nice exercise.
It's a rule of thumb that allows us to untangle our code and to make it slicker and easier
to understand.
Next I'm going to talk about package names.
This is another favorite of mine.
We know that naming is hard, and that is particularly true in case of package names.
We know that the name of a package should be small enough because that is consuming
screen space, but should be also good enough to let us understand the purpose of the package.
In Go there is even more because when we use an object, the name of the package is part
of the name.
So that is an opportunity for us to put some value in that part that the reader can consume.
And again, I'm starting with a bad example.
We have this utility package, and we have this copy node function that is totally fictional,
but that utility part is a wasted opportunity.
It's part of the name that doesn't add any value.
So it's better to take and split our package, smaller scoped packages that do and explain
what to do.
And in this case, from the colon side, you have node.copy, which still explains the purpose
of the function, and it's not wasting space.
And this was taken from the official Go blog, and it says basically the same thing.
There is no need to have these gigantic kitchen sink packages where we throw everything because
in Go packages are free.
So it's fine to split them in a better way.
Next one is going to be about errors, and I see also this happening very frequently.
In Go, errors are types, and let's say that the developer wants to handle a special error.
And the problem with these approaches is that we are giving away the fact that errors are
types, and we are converting them to a string, and we are treating them as a string.
And since Go 1.13, we have, like, and there are, like, that's legacy.
So there are no excuses not to use this.
There are two ways, one is to assert that the error that we are checking is an instance
of a given object that we have somewhere, and there is another, sorry, this is new because
the other one wasn't working.
And there is another one, which is about asserting that the error that we want to handle implements
the error interface against a specific real type.
But there is more.
So in this way, you can have wraps of errors, and you can assert that the error that you
are checking not only equals the one that you are handling, but also any error inside
this wrap.
And this is how you wrap them.
You can either use errors.wrap, so the return error from this function will contain the
value returned by this, but will also return true if we assert against the wrapped one.
And also there is the way suggested by the standard library, which is using the %w formators.
So both of them will return you a wrapped error.
So now let's talk about pure functions and why they are important.
So a pure function has two properties.
One is the fact that no matter how time, when you call it, no matter how many times you
call it, with a given set of input parameters, it will return always the same output.
And the other property is the fact that it shouldn't rely on the state of your system.
It shouldn't modify the state of the system.
Should it be global variables or static variables or your input parameters or anything that
is external to the function.
And why it matters.
This is an example where the behavior of this function depends on the state of an external
system that is accessed through a client.
And then you have the business logic after that.
And why this is bad.
I would say that mostly because this is hard to test or we can mock the external system,
we can do tricks to replace the client, but moving away the statefulness part of the function
away and having the business logic implemented as a pure function will allow us to be quicker
in writing the implementation and to write our tests.
And how about the second part.
So we have a function that accepts a pointer and in some random cases it changes the object.
And what's the problem with that?
The problem with that is now on the reading side because you don't know that it's not
clear enough that this function is changing the node.
So you get your bug report and you look at the code and you know that somewhere the name
of the node changed, but you don't know why.
And that's because it's not clear from outside that is what this function is doing and it's
harder to reason about it.
So a better way is to change the name of the function so it's clear, but I think that
and this comes quite often, a better way to do that is to delegate the responsibility
of changing the object outside and changing the function to be a pure function.
Again this version is easier to understand, it's easier to reason about, it's clear when
you will have something to change.
And this can also say about environment variables.
In the world of pods and containers, adding a new knob as an environment variable is so
convenient.
Just add an environment variable, you consume it from the function where you need it and
you are done.
But the problem with that is that you then don't have control anymore on all the knobs
on all the parameters that your program is consuming because they are all scattered across
the code base and that is bad because you can't foresee what a given function is doing
by reading its calling site.
So again, this is something that should be avoided, environment variables should be read
in your main functions and then be propagated through all the stacks.
So another topic that I care about is function arguments.
And the first one is Booleans.
So you start with something like this where you have a simple setup function that is easy
enough and then with all the good intentions of the world, thanks, with all the good intentions
of the world, the developer starts adding a parameter but then we need another one and
then we need another one.
And how does it look on the calling site?
Something like this and you think, true, false, true, true, false, what the hell?
And then you need to stop, you need to enter into this function, you need to understand
was it, where was the enable webbook parameter?
It was the first one and then you get back here and this works but adds friction and getting
a better version of it is so cheap that we should do that because we are doing a favor
to our future selves, we are doing a favor to the maintainer and it's going to be easier
to understand.
Another option might be to pass a structure to the function that also works but not this.
Now I want to talk about function overloading or the fact that God doesn't have, so it's
more or less the same as the other one.
God doesn't have function overloading so it's easy to have this full variety of the same
function where we need to slightly change the behavior.
So you start with creating a service, then you need one with a backend, then you need
one with an IP and then you need one with a backend and with an IP and it's clear that
can get easily out of hand.
So an approach that I really like is using a variety of the argument with some modifiers
that accept the parameter and do what they have to do and this is how it looks from the
calling site, again, it's clear, it's easy to understand, your future self will thank
you for this.
And there is also another version where you can have these generator functions.
I think it's on the borderline of being too magic for me but, again, this one is easy
to read.
So next one, I see this happening a lot in the world of controllers where you have one
file that basically implements all the methods related to a controller.
So you have this file and you need to add an utility function and then all the other
functions are methods and what do you do?
You add a new method, even if it doesn't have to be a method.
So you look at something like this and you think, hmm, why is this a method?
Is there something wrong with that?
And this, again, is adding friction that could be avoided.
So if a function is a function, just make it a function and not a method because also
testing is easier.
You don't have to have the instance that you are not using for anything just in order to
test this function.
And then a word about pointers.
Go has pointers, like not all other languages, so people might find them hard to reason about.
And when I see two functions like this, my first thing, thought is, like, this one is
not changing the object and the second one is doing that.
So this is the rule of thumb that I'm trying to apply.
If a function is not changing the object, then pass the object with a value, otherwise
pass the pointer.
But there are also exceptions.
There are some kind of objects that can be passed by value or they can, but they will
give you a bad afternoon.
But so mutex is file descriptors.
We need to pass them by reference because that's the way it works.
We have linters that help us in that and we have this rule of thumb that says if you look
at the object, if all the methods associated with the pointer, then use a pointer.
One might argue how about performances.
We are passing the whole object instead of passing just the reference.
Yeah, passing the reference is cheaper, but this is not see, this is go, and that's not
always clear.
So what we should care about is the readability.
And we have a lot of toolery that will help us to understand if that can be optimized
if it's in the hot path.
And then we need to sacrifice a bit the reliability of our program in order to have better performances.
So now I'm going to talk about something that was advocated in clean code where it says
that our code base should read like a newspaper, which means that you open a file, you should
have all the high level concepts on the top of the file, and then start to find all the
nitty details of the implementation in the bottom of the file.
And these applies pretty well to go.
So what I expect from a well-written go file is to have all the public methods, all the
public objects in the top of the file, because when I open the file, I see what this package
has to offer to the external world.
And so those are our high level concepts by definition.
And another thing that I think is sometimes underestimated is the fact that we can have
our packages split into files.
So again, in order to have a better navigability of our code base, we can split it into files,
have a main file related to the package that is named after the package, and then have
these smaller entities where we put the different logics.
And this is basically what I'm trying to say here.
So try to have the public fields on the top, try to remove or to move the utility functions
in the bottom, split the package into file, because again, it's free.
It won't cost any energy to you or to the executable, and have a main package file that
is named after the package.
Next item is about asynchronous functions.
And I saw this many times.
It's one of the nice things about Go, right?
It's so easy, so convenient to implement concurrent code.
You can just implement Go routines, you can pass channels and have fan in, fan out.
But the problem with that is that something like this has some flaws.
And I think that is way better to, again, take the business logic, move it to a synchronous
function that is easier to test without all the infrastructure that you need to put in
place with channels, with weight groups in order to reverse the synchronousness of your
function just in order to test it.
So if you can, move the business logic into a synchronous function and let the calling
site handle the life cycle of the Go routine.
So again, that part has to be delegated on the client code, and that will make our function
easier to test and our code base easier to reason about.
And again, I didn't invent this as everything else.
This is from the code review Go wiki, and it's basically saying the same thing, like
try to use synchronous functions as much as you can.
Next item is about functions that lie, and what I mean by that.
You have something that is, what would you expect this function to do?
Clear the node.
Exactly.
That's what I would expect.
But the developer found a very edgy corner case where if the name of the node is do not
clean, then do not clean.
And he was doing that with the all good faith of the word.
He's trying to solve a problem here.
But the problem is that, again, this is going to give us a bad afternoon because we'll see
that the node is not being cleared and we'll have to put a lot of printfs in our code or
to do a lot of debugging in order to understand why is this happening.
So again, this is done with good intentions, but the result is not so good.
So again, as I said multiple times today, we should defer this responsibility to the
calling site because that will result in a code base that requires less energy and less
effort to understand.
What if we have this function called 100 times in our code base, then I don't know.
Just call it clear the node, but do not clean one or have one filter function, whatever,
but not lie to the reader.
So wrapping up, there is no much to wrap up.
I mean, it was just a list of no related items.
Maybe the only take away that is globally is to say that we should be smart and let
our readers, the calling site over the code base do a bit more because that will give
us a better day in the future.
I'm a strong believer of the Pareto principle, most often when it's on the bad side of it,
but in this case, I think that by applying these set of rules that will take very less
to implement, those will improve the quality of the code base a lot.
And then I want to finish with this quote from Rob Pike, simplicity is complicated,
but the clarity is worth the fight.
And with that, I'm finished.
Sorry?
Are there any questions, I'll try to come with a microphone, if it doesn't work, we'll
have to repeat it.
Hi, thanks for the talk.
I was wondering, do you see any room for automating some of these rules and wisdom that you share
today, maybe something else as well?
I don't know, I should think about that.
Probably some of them, yes, like avoiding having functions or raising a flag if a function
is accepting a channel, for example, but there are exceptions to that, so that shouldn't
be blocking.
There are some others, like the function that is lying to the user is something that depends
on the implementation, or for example, having a function that accepts five booleans should
be flagged.
So, I see that, I think that it depends on the case, but some of them might be automated.
Any more questions?
No?
Thank you very much.
How was it?