Taking the code examples first. You favour:
if (isApplicationInProduction(headers)) {
phoneNumber = headers.resourceId;
} else {
phoneNumber = DEV_PHONE_NUMBER;
}
function isApplicationInProduction(headers) {
return _.has(headers, 'resourceId');
}
And your boss would write it as:
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
In my view, both have problems. As I read your code, my immediate thought was "you can replace that if with a ternary expression". Then I read your boss' code and thought "why's he replaced your function with a comment?".
I'd suggest the optimal code is between the two:
phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;
function isApplicationInProduction(headers) {
return _.has(headers, 'resourceId');
}
That gives you the best of both worlds: a simplified test expression and the comment is replaced with testable code.
Regarding your boss' views on code design though:
Writing small functions is a pain because it forces you to move into each small functions to see what the code is doing.
If the function is well-named, this isn't the case.
isApplicationInProduction is self-evident and it should not be necessary to examine the code to see what it does. In fact the opposite is true: examining the code reveals less as to the intention than the function name does (which is why your boss has to resort to comments).
Put everything in a main big loop even if the main loop is more than 300 lines, it is faster to read
It may be faster to scan through, but to truly "read" the code, you need to be able to effectively execute it in your head. That's easy with small functions and is really, really hard with methods that are 100's of lines long.
Write only small functions if you have to duplicate code
I disagree. As your code example shows, small, well-named functions improve readability of code and should be used whenever eg you aren't interested in the "how", only the "what" of a piece of functionality.
Don't write a function with the name of the comment, put your complex line of code (3-4 lines) with a comment above. Like this you can modify the failing code directly
I really can't understand the reasoning behind this one, assuming it really is serious. It's the sort of thing I'd expect to see written in parody by The Expert Beginner twitter account. Comments have a fundamental flaw: they aren't compiled/interpreted and so can't be unit tested. The code gets modified and the comment gets left alone and you end up not knowing which is right.
Writing self-documenting code is hard, and supplementary docs (even in the form of comments) are sometimes needed. But "Uncle Bob"'s view that comments are a coding failure holds true all too often.
Get your boss to read the Clean Code book and try to resist making your code less readable just to satisfy him. Ultimately though, if you can't persuade him to change, you have to either fall in line or find a new boss that can code better.
"Clean code" is not an end in itself, it is a means to an end. The main purpose of refactoring bigger functions into smaller ones and cleaning up the code in other ways is to keep the code evolvable and maintainable.
When picking such a very specific mathematical algorithm like the "Miller-Rabin" prime test from a text book, most programmers do not want to evolve it. Their standard goal is to transfer it from the pseudo code of the text book correctly into the programming language of their environment. For this purpose, I would recommend to follow the text book as close as possible, which typically means not to refactor.
However, for someone working as mathematician in that field who is trying to work on that algorithm and change or improve it, IMHO splitting up this function into smaller, well named ones, or replacing the bunch of "magic numbers" by named constants, can help to make changes to the code easier like for any other kind of code as well.
Best Answer
You have to take everything in Clean Code together. I've read the book more than once. I frequently lend it to new developers on my team.
Yes, Martin does say you should prefer long, descriptive names over comments. However, he also advocates for the Single Responsibility Principle.
I've seen SRP defined a few ways, usually "a class or method should only have one reason to change", or "a class or method should do only one thing".
So, I tell my developers to write very descriptive names. I also frequently tell them that if the name of the method is getting too long, it is probably doing too many things.
If the name of the method is becoming unmanageable, consider if something needs refactoring.
I also tell my team to avoid the Incredibles Pattern. As we learned from Syndrome, "when everybody's special, nobody's special."
If every property or method of you class starts or ends with the same word(s), you can probably omit that word.
Assuming these are both members of one class, would the following make sense?
It looks like you have many PageReloaders, so we make a base class for the things all PageReloaders do.
Next, the name indicates that being "used in the editor" is significant, so there must be other kinds of VectorGraphicsReloaders.
Finally, we get to the EditorGraphicsReloader, which is a VectorGraphicsReloader that does something specifically for the editor.
Within one of these class we should have two properties:
The class these properties belong to depends on whether they are unique to the editor, vector graphics, or common to all page reloaders.