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 misc...
For further actions, you may consider blocking this person and/or reporting abuse
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 :)