71–80 of 104 posts
When I coee, somewhere a method needs to initiate this execution flow and will therefore contain "and". Even if all it does is call two other methods where this principle is followed. How do I avoid this? I mean, somewhere in the code it makes sense to execute the methods together.
I want to testify of this great death spell caster. This great man helped me cast a death spell on my wicked step father and just within 48hours the wicked man had a motor crash and died. All thanks to this great death spell caster called instant death spell. You too can contact him now for an urgent death spell cast on anyone, quickrevengespell@yahoo.com,
Most code reviews seem to focus on:
1. Examining what the change does 2. Finding ways to make the change in a nicer way. E.g. Refactoring etc.
This leaves out the key step 0 - what is actually trying to be achieved, does it need to be done and is there a better (maybe completely different) way to do it.
This leads to a focus on relatively trivial matters such as naming conventions and method lengths.
I think that the underlying reason for this is laziness. Talking about clever refactoring is an easier/faster process than understanding the 'why'.
While I agree with all the points listed in the article, it highlights for me a major problem of a lot of code reviews. Most code reviews seem to focus on: 1. Examining what the change does 2. Finding ways to make the change in a nicer way. E.g. Refactoring etc. This leaves out the key step 0 - what is actually trying to be achieved, does it need to be done and is there a better (maybe completely different) way to do…
That's why we discuss these high-level design and architectural decisions before-hand so that they are known to the reviewer at the time of the review. We're a small team so it works well. But I'm not sure how this scales up as the team gets bigger. I would like to know how bigger teams approach this problem!
setting thresholds on method / class sizes seems quite arbitrary and potentially harmful. Splitting a method into n different ones, none of which is called more than once is setting up the code for opportunistic reuse and obscures it's true function. It's especially wrong if the code that was split is mutating / non pure functional. see http://number-none.com/blow/john_carmack_on_inlined_code.htm...
If you name the methods correctly (what they do) splitting actually helps readability of the code. It is quite hard to read and remember what was in the beginning of 500 line method.
While I agree with all the points listed in the article, it highlights for me a major problem of a lot of code reviews. Most code reviews seem to focus on: 1. Examining what the change does 2. Finding ways to make the change in a nicer way. E.g. Refactoring etc. This leaves out the key step 0 - what is actually trying to be achieved, does it need to be done and is there a better (maybe completely different) way to do…
One thing we did on the current project is pre-commit reviews in pairs. This ensures at least two people in the team knows about the changes, let us talk about the why and how of the changes, and possibly teach a coworker new things in the process.
What it ended up doing is that every programmer now self-reviews their own changes prior to the actual review knowing they'll soon share it all face to face with a coworker. Turns out the talks are now about the design of the code, not how it looks.
Earlier quoted context omitted.
I agree with you in principle but find that code editors let me down in that regard. Say I split a function up into a few sub-functions because it's getting big. Now I have the problem that I'm jumping forwards and backwards through the code when I want to explore what that function does: SomeMassiveFunction() { SubfunctionA(); SubfunctionB(); SubfunctionC(); SubfunctionD(); } SubfunctionA() { } .... In this case, Su…
I think you've hit upon a good measure of function quality. If you find you have to jump around a lot when reading, that would be a sign that it's poorly organized and needs to be refactored. On the other hand, if you find you don't have to jump around and can trust the sub functions by their names, then it's been broken up well. In the best case you should be able to follow the logic without diving into the other fu…
It all depends at which stage of development you are at. If the piece of code we're talking about is mature and rarely changing then yes it seems like a reasonable thing to do. If however this section is still under development then I would opt for another way of dealing with readability issue.
I think if what you want is to optimise the readability of the code then a simple comment above each section, combined with block scoping is a good set up for splitting.
As the true nature of the code emerges one can decide to turn the block into local lambdas then later into functions.
Again that's because of the first point I've made here in this comment. If you are still developing the functionality, splitting early means that subsequent reviews&development may miss potential interactions + potential refactorings/simplifications that would have been quite clear if the function was still "messy"
Earlier quoted context omitted.
I disagree. Breaking down a function into several one time functions that all sit at the same mental model of abstraction make code much easier to reason about.
A named one-time-use function has a name that describes what it does. This seems obvious, but bears repeating: instead of having to figure out what a chunk of code does, you can guess from the name and have a series of steps naturally described.
Earlier quoted context omitted.
I think you've hit upon a good measure of function quality. If you find you have to jump around a lot when reading, that would be a sign that it's poorly organized and needs to be refactored. On the other hand, if you find you don't have to jump around and can trust the sub functions by their names, then it's been broken up well. In the best case you should be able to follow the logic without diving into the other fu…
From a static point of view this is true, however splitting the functionality into sub functions might later obscure commonalities that would have been obvious if they were still part of the upper function. It all depends at which stage of development you are at. If the piece of code we're talking about is mature and rarely changing then yes it seems like a reasonable thing to do. If however this section is still und…