Uncle Bob's insistence that functions should be 2-4 lines long is baffling to me. I don't understand how he can be taken seriously. Is there a single application in the entire world with substantial functionality that conforms to this rule?
Eventually, your code has to do something. Get that thing in one place where you can look at it in its whole.
My own experience is that with an IDE that can collapse a new scope in the middle of a function, you can make large functions that accomplish a lot and are very clear by writing a comment and starting a new scope.
If something is going to be called multiple times a new function makes sense, but this idea that anything that can eventually return a single value needs to be it's own function is a giant pain that creates more problems than it solves.
Just makeing a new scope in the middle of the functions lets you use all the variables in the outer scope, do transformations without introducing new variables and ultimately "return" a new variable to the outer scope.
I've never understood why polluting namespaces with dozens or hundreds of names (most of which may not be very descriptive since naming a hundred small things is already going to be confusing) is seen as a good idea. You look at a list and you have no idea what is important and what was being shoved in there to satisfy some do nothing public speaker's arbitrary rules.
Too many arguments can become a problem. Nested functions can help here, because they allow you to move their implementation “out of the way” while still having access to shared variables. And sometimes a collection of parameters can sensibly become its own class.
IDE affordances are fine, but I’m opposed to requiring reliance on them for reading and understanding code, as opposed to writing.
What does independent mean? I would just collapse them all because they were meant to be collapsed in the first place.
you have to analyze the unfamiliar code first in order to know which sub-scopes you might want to collapse
Unfamiliar? I wasn't refactoring and collapsed them all.
The benefit of extracted functions is that you know that they are independent of the implementation details of the caller (i.e. can’t possibly depend on or modify local variables of the caller other the ones passed as arguments).
That's true to an extent, but this is more of a way to make monolithic functions simple, which then makes the program simpler over all because you can avoid lots of tiny little functions. What you can end up with is programs that do non trivial things but don't have tons of functions confusing the issue.
Pragmatically this isn't really a problem. The whole "it isn't exactly a 1:1 replacement" isn't the point. You can still put in comments and const references if you really want to.
IDE affordances are fine, but I’m opposed to requiring reliance on them for reading and understanding code, as opposed to writing.
Why would it be required? The alternative is that you still have these commented sections with their own scope but they aren't collapsed. You can always work on it without the IDE and when you go back to the IDE it still works.
The reality of it is that you can see a broad overview of a function then see details one section at a time and you don't even have to go skipping around to other parts of the file or other files to do it.
Uncle bob doesn’t mention this.
createspecialString(y) =
Capitalizefirstletter .
MakealllowerCase .
AddNumberSuffix .
removeLetterA .
removeLetterB .
ConcatwithWord(x)
CapitalizeFirstLetter(a) = a[0].upper() + a[1:]
MakeAllLowercase(a) = map(a, (t) => t.lower())
Addnumbersuffix(a) a + 3.toString()
RemoveLetterA(t) = filter(t, (s) => s.lower() == “a”)
RemoveLetterB(t) = filter(t, (s) => s.lower() == “b”)
ConcatenateWithWord(x) = (y) => y + x
F . P = (x) => F(P(x))
People who complain about this style tend to be unfamiliar with it. If you had knowledge about procedural coding styles and a function composition approach like this then usually this style is easier as the high level function literally reads like English. You don’t need to even look at the definitions you already know what this complicated string formatting function does.
No comments needed. And neither author tells you about this super modular approach. They don’t mention the critical thing in that this style requires functions to be pure.
Thus to get most of your code following this extremely modular and readable approach… much of your code must be minimizing IO and state changes and segregating it away as much as possible.
The Haskell type system, the IO monad is pushing programmers in this direction.
Again neither author talks about this.
createSpecialString = createFormattedString . createNewString
createFormattedString =
Capitalizefirstletter .
MakealllowerCase .
AddNumberSuffix .
createNewString(y) =
removeLetterA .
removeLetterB .
ConcatwithWord(x)
createspecialString(y) = Capitalizefirstletter . MakealllowerCase . AddNumberSuffix . removeLetterA . removeLetterB . ConcatwithWord(x)
Lines doesn't makes sense for pure non imperative functions. Lines ONLY make sense for imperative functions because each line represents an instruction.
In the first example there is one layer. In the second there is 2 layers of abstraction formed by composing the primitives into a tree.
It is all as bad as you imagine. Functionality is spread out all of the place so it is very difficult to reason about how it all hangs together.
Or worse, I've seen people break functions apart in such a way that you now need to maintain some sort of class-level state between the function calls in order to get the correct behaviour. This is almost impossible to meaningfully test because of the complex possible states and orders between those states - you might correctly test individual cases, but you'll never cover all possible behaviours with that sort of system.
I think that's actually fine. Particularly if you're doing a testing pyramid style approach where you do a lot of tests of some piece of low level logic and then a few tests of the higher level piece that makes use of that lower level logic, I don't see any problem with the higher level test covering a codepath that's also used in the lower level test. If anything I find it makes it easier to understand and debug failures - you know that the behaviour of the lower level piece hasn't changed because otherwise the lower level test would have failed, so the bug can only be in the higher level component itself.
When you have small expressions that have incremental results stored in variables, you can see the result in a debugger so you can see each stage.
However, a friend of mine was a professional Smalltalk programmer. He claims that his median line count of methods, over his 17 year career, was 4.
It is harder to do in other languages--it seems that C would be on the order of 10.
Clearly it is a rule that can lead to complexity of too many methods, compromising whatever gain smaller methods give you.
Auto-generate getters and setters for every instance variable and that will drag the average down. (Maybe a lot of those getters and setters should not have existed.)
Name: MyClass
Superclass: Core.Object
Instance Variables: anInstVar anotherVar
Create methods:
Accessors
Initializer
Subclass responsibilities
~
page 113 get method
page 117 set method
https://rmod-files.lille.inria.fr/FreeBooks/WithStyle/Smallt...
~
http://www.hpi.uni-potsdam.de/hirschfeld/seaside/tutorial?ch...
etc etc
He did an example in the article of:
void concurrentOperation() { lock() criticalSection(); unlock() }
So if you subsitute criticalSection with a lot of operations, such as open file, read lines, find something, close file. I think you have a better representation of an over bloated function.
Scala has the langauge support to show what this could look like.
What you're doing in that method is starting a critical section, doing something, and then ending a critical section. It's a good suggestion to break that with:
def criticalSection(f: () => Unit) { lock() f() unlock() }
How you have a single method that does one thing and is easy to understand. Also it's reusable.
The original code would be used as:
criticalSection { _ => doSomething() }
That replacement is now longer dependent on locking. Locking is layered in.