We already know the way but probably out of laziness we don’t follow it. It’s the most underestimated way of writing clean code and there is a misconception around it as well which is totally not true, I will come to that in a while. So, Its breaking down code into functions!
I agree, we do break down code into functions but not as often as needed. Let’s take this snippet below as our example which was literally a state of code in my previous company. Let’s go through each of the points below.
- It looks like it is restoring some kind of group. Even though its just 2 lines, it make sense to extract a method out of it.
- Initially this must have started with one flag. Developer might have thought why bother having a separate method for just 1 line of code! Then later another flag was added and then yet another.
- Seems like we are creating a drawer for a view. Why the heck high level code should care how it is getting initialised?
- Only after going through 4 lines of code, I am able to understand that we are notifying device information. I know 4 lines aren’t lot but such things can span 10s or even 100s of lines. Better extract out a method.
- Logging in a reading flow is very unproductive. Get a room!
- All listeners should be set separately in a method.
After doing some refactoring.
See how beautiful the high level code looks now. Its easy to scroll through without getting bogged down in unnecessary detail. A well defined method name doesn’t require any comment as well!!
Tips to follow
- Don't pollute the reading flow with irky initialisation or complex conditions.
- Define short methods. They are easier to reason, flow is more obvious, scope is shorter, side effects are better visible.
- Make most read parts of your code declarative.
Its always easy to add two lines in an existing method. But do pay attention if the code above and below the added changes require its own space, if yes, then extract a method out of it. Every set of logic must be bundled properly in a method, that way future contributors will obligingly add changes in there respective methods.
Conclusion
Does an increase in the number of methods hurt performance, as many people claim? It’s a misconception and in almost all cases the impact is so negligible that it's not even worth worrying about. If you work on JVM based languages then let me tell you JVM is a wonderful piece of software (it will probably outlast Java!) that has many truly amazing runtime optimizations built-in. So, break it down...
Code is like humor. When you have to explain it, it’s bad. — Cory House
Top comments (19)
Personally, I do not like the kind of method in your improved version. They neither take arguments nor do they produce values. This means they rely on state set by someone else and they produce side effects that others (might) rely on. In the initial version it is easier to reason why things are ordered as they are, because I can see which usage of a variable depends on which write to this variable. However, I do support the extraction of methods to give them a proper name, but with two additions:
Let's not argue about procedural programming style, where avoiding side effects is not always easy or even possible. But there are a lot of occasions where following these principles leads to code which is easier to understand and to reason about. That is because from a method's call site, I can not only see what it is intended to do (=method name) but also what it needs to do this (=parameters) and what is the result of doing it (=return value).
I totally agree that side effects cannot be avoided as far as procedural programming is concerned. But I would still prefer the improved version (with or without parameters) because either way there are side effects. Certainly, the code example I used is somewhat trivial (no inter-dependent methods, no return values) and I am sure we can break down our code even in that case. My only concern is that the high level code should be made declarative.
I like it. It indicates that six different steps take place and what they are. Compared to the original version, now you can actually tell what the code does. When you have to modify it, it's much easier to find the part you're looking for. It's also easy to tell if the code in any of those steps affects any of the other steps.
In effect it's like a table of contents for the code. We don't pick up a product manual and read the whole thing to find one detail. We read the table of contents.
I think extracting to methods is good, but I think it's better to not hide initialization. For example, I would have preferred it like
mView.setTabLayout(createLayout())because more clearly conveys what's being done (and has less side effects and so on).Also, having methods indicates that you plan to reuse the code inside it. I think immediately invoked functions is a great way to handle that (although not all languages support that):
You might also use lambdas or similar code grouping elements.
"having methods indicates that you plan to reuse the code inside it"
That's what really bothers me about having all those small functions. I don't need them anywhere else and my IntelliSense is soon full of garbage.
Separation of concern is another reason why we define methods and if we define proper method names then we need not worry about IntelliSense drop down list size.
No shit Sherlock 😂😂
I don't think everyone knows how to refactor or write readable code, especially if they're a beginner. Let's not assume, yeah?
Fair enough. But you should take into account that this kind of click bait titles do not help your web. I expect a bit more.
I do wrote in the title that its the simplest way. I guess you are the only prey ;)
I know 😃 but mostly its not followed through.
In my experience it's followed most of the times, when it's necessary of course not blindly. In your example the original code is simple and very well structured. Putting a couple of lines of code in a method that is not reused outside this class is a bit extreme. But that's just me :-)
Example I used is just a snippet. The init() method earlier actually spanned 1000s of lines. And the extra line I have put between each set of logic is so that its easy to follow.
If a method has 1000s of lines then it's definitely a nightmare. But moving every 2/3 lines to a method will only make it worse. You'll end up with hundreds of methods. You need another kind of refactoring, perhaps creating multiple classes with different responsibilities.
Yeah we actually did the refactoring and created multiple classes and methods.
Disagree, code is for human to read. A good code comment will enable following developer easy to pickup. For instance
A meaning comment always help developer to maintain the code without misunderstanding.
If we can make a comment as follow
See also github.com/Microsoft/vscode/blob/m...
They both have tradeoffs, so my default position is to be cool with either, until I have some reason to prefer one or the other. One tip for reading code that looks like the former is to read the LHS of assignments and ignore the RHS, unless you are specifically interested in it (eg in the same way a method name abstracts its body, a variable name abstracts its assignment).
I wouldn't say by defining methods we are making any trade-off and at some abstraction level we need to assign some values, just that assignment shouldn't be done at high level code. I like your LHS/RHS tip :)
There are tradeoffs for every decision :) Eg while the initialization method was ugly, all the code within it was scoped to it. Now the code is in methods, even though they are only used by a single method. So while the constructor gets more declarative, the class gets more cluttered.
I often write objects that could basically be functions, the constructor receives their arguments, and then they have a
callmethod to do whatever it is that they do, and all other methods are private. Within that frequent pattern, I often find it useful to restrict variable setting and getting toinitializeandcall. This forces the integration to happen at the top, which reduces coupling. It means I only have to look in those two places to see everything that can access that variable I'm trying to debug. By pushing the assignment into other methods, I can't glance at one spot and know all the candidates. Do note that you can mitigate this by having the method initialize the object and return it, and then performing the assignment in the constructor.Also note that the methods here are nice declarative names, but they are implicitly depending on something. I can't tell from the code sample, but it's common for one step to need the other to have been performed already, so methods allow an implicit dependency on order of invocation. Note that you can also fix this while still using methods: pass the dependencies as arguments. This also allows you, again, to see everything that accesses some variable of interest.
Basically, instance variables are like global variables, but for a much smaller globe. So, if you're not careful, your instance variables can have the same pains as global variables, and methods operate on implicit state are the primary way to do that. So, I try to keep ivar access at the top... but, that's just a heuristic, I also write code like your pre and post refactored examples :)