Releasing the tension between "deep modules" and "small function" practices

[Roman Leventov]

The contrast between John's "modules should be deep" approach and "extract a lot of small functions" approach advocated by Bob Martin, Kent Beck, and Martin Fowler, among others (see, for example, https://sites.google.com/site/unclebobconsultingllc/one-thing-extract-till-you-drop) has led me to an idea of immediately executed inner functions which, with some support from IDEs and editors, may resolve this contradiction.

I've presented the idea here: https://medium.com/@leventov/semi-function-a-missing-tool-to-handle-complexity-in-imperative-code-bdefe82d8cf6

[John Ousterhout]

First, I am bewildered and horrified by Uncle Bob's "Extract till you Drop" page. He has taken 25 lines of code that are pretty straightforward and easy to understand, and turned them into 38 lines with 9 methods, none of which has a stitch of documentation. What was the point of this?

Uncle Bob says that his goal is to have each function "do one thing", then he takes that to the extreme. But why? Does this make the code easier to read, overall? Personally, I think Uncle Bob's starting code is simpler and easier to read than the result. One of the biggest mistakes people make in design is to take an idea that has some value ("do one thing" falls in the category), and take it to the extreme. This is almost always a bad idea, regardless of the idea. For example, it has been pointed out that "classes should be deep" could be taken to an extreme by putting an entire application in a single class; obviously that would be a bad idea. If Uncle Bob heard someone say that rearview mirrors are a good idea, I fear that he would design a car completely encrusted in rear-view mirrors, to the point where the driver can't see the road ahead.

Let's not lose focus on the overall goal, which is to minimize overall design complexity. Tips like "do one thing" can help with that goal, but we should only use them to the degree that they really do help.

Is there anyone out there who would argue that Uncle Bob's result code really is easier to understand, overall? If so, I'd be interested to hear the reasoning.

Now, on to Roman's semi-functions proposal. This does appear to be a midway point between Uncle Bob's starting and ending code. But, again, I'd ask "why do this?" It's not obvious to me that the code with semi-functions is easier to read than Uncle Bob's starting code, and the semi-function mechanism creates its own additional complexity that must be understood.

Also, it's not obvious to me that functions that can only be called in one place are a good idea. Isn't the whole idea behind functions that we want to create code that can be reused in many different situations? Use-once functions work against this. Again, I'd ask "how does this help to reduce overall system complexity?"

-John-

On Mon, Apr 22, 2019 at 6:24 AM Roman Leventov <leven...@gmail.com> wrote:

The contrast between John's "modules should be deep" approach and "extract a lot of small functions" approach advocated by Bob Martin, Kent Beck, and Martin Fowler, among others (see, for example, https://sites.google.com/site/unclebobconsultingllc/one-thing-extract-till-you-drop) has led me to an idea of immediately executed inner functions which, with some support from IDEs and editors, may resolve this contradiction.

I've presented the idea here: https://medium.com/@leventov/semi-function-a-missing-tool-to-handle-complexity-in-imperative-code-bdefe82d8cf6

--
You received this message because you are subscribed to the Google Groups "software-design-book" group.
To unsubscribe from this group and stop receiving emails from it, send an email to software-design-...@googlegroups.com.
To post to this group, send email to software-d...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/software-design-book/CAAMLo%3DaQZEG-PutHJMf5XMJzCThNxEGZM8r030vx9S-3mrS5KQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Kent R. Spillner]

Hi, John-

> Let's not lose focus on the overall goal, which is to minimize overall
> design complexity. Tips like "do one thing" can help with that goal,
> but we should only use them to the degree that they really do help.

Readability, understandability, minimizing complexity, etc. are of
paramount importance, but there are other aspects of good code/design
that sometimes still take priority (e.g. performance, reliability,
security, modifiability, testability, etc.). Sometimes you need to
sacrifice on simplicity to benefit them (and sometimes you don't).

I'm reminded of something Kent Beck wrote a few years ago along the
lines of "sometimes you have to make a really big mess before you can
make things cleaner."

Which is not to disagree with you but just to point out that sometimes
we *do* need to lose focus on the overall goal.

> Is there anyone out there who would argue that Uncle Bob's result code
> really is easier to understand, overall? If so, I'd be interested to
> hear the reasoning.

In my opinion his final version is more difficult to understand if you
try to read the whole class from start to finish and keep it all in your
head. It's harder to read because you have to jump back and forth
between method definitions, some of the methods are written at one level
of abstraction and others at a different level, you need to read the
code two or three times (or I do at least) in order to grok the full
context of each step in the process, etc.

BUT...

In my experience when I encounter new code on a project in the course of
a specific task I prefer code written in the final style. For example,
imagine that you're asked to change the behavior of SymbolReplacer so
that it only replaces the first occurrence of each symbol. If your
code spelunking leads you to replace() I think the final version is
easier to understand and modify because you don't need to read the
entire class. You can jump straight from replace() to
replaceAllSymbols(), skim the for loop to see we're iterating over
each symbol, jump to replaceAllInstances(), skim the conditional to see
we don't care about its criteria, jump to replaceSymbol() and see that
it's calling String::replace, and you're done. There's no need to look
at nextSymbol(), shouldReplaceSymbol(), symbolExpression() or
translate().

That's the approach I personally take when making changes in large
codebases I'm not familiar with: start at some easily identifiable
entry point like main() or a Servlet or whatever, then start skimming
& jumping around to try and hone in on where some particular behavior
is implemented. In such cases I find the heavily extracted style
easier to read & understand.

My bug hunting approach is a little different, though. If I'm trying
to find & fix a bug in some unknown code like SymbolReplacer I'll take
the same skim & jump approach until I end up in replace(). At that
point, however, I'll start reading the entire class from top to bottom
and try to understand it all. Only after I think I understand the
whole thing will I attempt to hone in on the actual bug via tests or
debugging or printfs or whatever. In that case I would prefer the
original version of the code because I find it easier to read that way.

Obviously, I can't have it both ways, so which would I prefer overall?
I think I'm with Uncle Bob on this one and would prefer the final
version in general. When starting a new project from scratch I would
start writing code in the original style but over time I would expect
to refactor it towards the final style. I just think it eases
maintenance on large, mature codebases if they are written that way.
Especially if the expectation is that every developer on the team can
work on any part of it at any time.

I wonder if that's what Uncle Bob is getting at when he writes: "after
programming for over 40+ years, I’m beginning to come to the conclusion
that this level of extraction is not taking things too far at all. In
fact, to me, it looks just about right." ?

Best,
Kent

[Ralf Westphal]

Even though in general I like splitting functions up into smaller and smaller functions the questions is valid: when to stop? Or when to start in the first place? Should Robert C. Martin have started?

And in this case, I guess, I agree with John: What's the point?

String replace() {
  replaceAllSymbols();
  return stringToReplace;
}

I don't mind functions which get just called once. Re-use is highly overrated in my view. It borders on an anti-pattern to me. But that's a topic for another day ;-)

So, what's the point of refactoring the original replace() to the above?

I don't get it. 1 method is calling just 1 other method? How does that help understandability or testability?

That's my main criteria when assessing the cleanliness of a code base:

* Is the code easy to understand, easy to reason about?

* Are critical/interesting ranges of logic easy to test?

The above refactored method does not add anything in terms of understanding or testability.

Understandability - for my taste - is not optimal in the original code. Mainly however, because it's lacking tests. What is it supposed to do? I find the explanation not really enlightening. (Or maybe I'm lacking some Java skills?)

From a testability point of view, on the other hand, the original code was ok for me. Maybe I don't agree with its overall structure, ie. how OOP was used, but never mind.

There is not much logic in the solution and it's all testable through the one method replace().

(What I'm missing mostly is a definition of getSymbole().)

That said, I too sense there are 2 responsibility which could be separated. But then to me it probably would be sufficient to refactor into 3 methods:

String replace() {

  var x = Responsibility1(...);

  return Responsibility2(...);

}

or maybe

String replace() {

  foreach(var x in Responsibility1())

    Responsiblity2(x);

  return stringToReplace;

}

(Please excuse my C# syntax.)

This kind of refactoring would  lead to more understandability from my point of view because in the top level method it would be very clear how the overall problem is structured: it consist of two pretty distinct sub-problems whose solutions are complementary and a integrated by the top method replace().

Also testability would be increased because the logic to solve the sub-problems can be tested in isolation. The integration itself (replace()) then is a no-brainer.

[Ralf Westphal]

Oh, one more thing: Robert C. Martin is asking at the beginning " What the hell does 'one thing' mean?"

To me that's decisions and insights. The SRP is about encapsulating (major) decisions I make during development and/or insights I gain while reading code.

In the example a major decision to me would be using regular expressions. Looking for a pattern could be done in different ways, but Robert C. Martin has chosen this particular one. This should be documented by wrapping it in at least a method. Or you can say it should be encapsulated in at least a method to make it easy to spot and easy to change.

An insight would maybe be that replacement is done only once. That too then should be documented/encapsulated in a method of its own.

Decisions should be separately encapsulated for documentation and easy revision and also testing.

Insights should be separately encapsulated for documentation and testing.

[John Ousterhout]

On Wed, Apr 24, 2019 at 6:36 PM Kent R. Spillner <kspi...@acm.org> wrote:

...

BUT...

In my experience when I encounter new code on a project in the course of
a specific task I prefer code written in the final style.  For example,
imagine that you're asked to change the behavior of SymbolReplacer so
that it only replaces the first occurrence of each symbol.  If your
code spelunking leads you to replace() I think the final version is
easier to understand and modify because you don't need to read the
entire class.  You can jump straight from replace() to
replaceAllSymbols(), skim the for loop to see we're iterating over
each symbol, jump to replaceAllInstances(), skim the conditional to see
we don't care about its criteria, jump to replaceSymbol() and see that
it's calling String::replace, and you're done.  There's no need to look
at nextSymbol(), shouldReplaceSymbol(), symbolExpression() or
translate().

Hmmm, this sounds like exactly what I would do when reading the original code; haven't you had to look at just as much code, but you also had to jump around in the file among several different methods? For example, the jump from replace() to replaceAllSymbols() is pure overhead, because there is no actual functionality in replace; I learned nothing by looking at replace() except that it delegates its entire functionality to another method.

By the way, when I first read replace() I wondered "why is this procedure performing an operation and then returning the original unmodified string?" I eventually had to figure out, without the help of documentation, that this collection of methods actually overwrites stringToReplace with the result (which, by the way, is another bad design decision, because by the time replace() returns, the variable name no longer describes its contents). This is typical of the super-decomposed style: it tends to create subtle and non-obvious dependencies between the zillions of tiny methods.

-John-

[John Ousterhout]

On Thu, Apr 25, 2019 at 12:41 AM Ralf Westphal <ral...@gmail.com> wrote:

Oh, one more thing: Robert C. Martin is asking at the beginning " What the hell does 'one thing' mean?"

This might be the crux of my disagreement with Uncle Bob. To me, "do one thing" refers to the abstraction visible to callers: the method appears to have a single "tight" function (something that can be described and understood very simply). The original code meets this definition.

Perhaps Uncle Bob thinks "do one thing" means that a method only has one operation in its body? I don't see any particular value in that definition.

-John-

[Ralf Westphal]

Hmmm, this sounds like exactly what I would do when reading the original code; haven't you had to look at just as much code, but you also had to jump around in the file among several different methods? For example, the jump from replace() to replaceAllSymbols() is pure overhead, because there is no actual functionality in replace; I learned nothing by looking at replace() except that it delegates its entire functionality to another method.

I agree about the overhead in this example. But my feeling is, that's because the decomposition is done, hm, badly. The basic idea is good, but he's gone overboard with it and it's based on a bad OOP design in the first place. To solve the whole problem like it was done in the original version is, let me say, "well-meant but misused" OOP ;-) There is absolutely no reason for state in this example.

And the only thing that might suggest state - the dependency on the unexplained getSymbol() method - is not treated as state. OOP might have helped with ctor dependency injection to get getSymbol() in there as a strategy. But then... maybe getSymbol() should not be a method at all; maybe a map would suffice?

 

By the way, when I first read replace() I wondered "why is this procedure performing an operation and then returning the original unmodified string?" I eventually had to figure out, without the help of documentation, that this collection of methods actually overwrites stringToReplace with the result (which, by the way, is another bad design decision, because by the time replace() returns, the variable name no longer describes its contents). This is typical of the super-decomposed style: it tends to create subtle and non-obvious dependencies between the zillions of tiny methods.

I don't think this is a problem of a super decomposed style, but of the bad basic design - and bad naming in the first place: the "stringToReplace" was never meant to be replaced in its entirety. But that's what the name suggests. The string just contained stuff that should be replaced: placeholders for symbols. The whole code reeks of "anaemic domain" ;-)

And on top of that: no test in the first place to tangibly show what it does.

 

-John-

[Ralf Westphal]

This might be the crux of my disagreement with Uncle Bob. To me, "do one thing" refers to the abstraction visible to callers: the method appears to have a single "tight" function (something that can be described and understood very simply). The original code meets this definition.

Perhaps Uncle Bob thinks "do one thing" means that a method only has one operation in its body? I don't see any particular value in that definition.

I don't think that's what Robert C. Martin meant. But this example can be mistaken for such a notion of "one thing" (or "single responsibility"). 

Still, "one thing" is begging a definition. The SRP is defined like this: https://en.wikipedia.org/wiki/Single_responsibility_principle But I still find that fuzzy.

In the end we all might justifiably have different needs for granularity depending on our knowledge of domain and technology.

But I'd like to still strive for a bit more clarity in this. Because it seems to be a fundamental principle.

That's why I said, "single decision" or "single insight" is what I'd wrap in a function. Or a "single aspect to test". Because we can only test what's accessible through a function.

In this example to me there are three decisions entangled:

* how to parse the string

* how to replace placeholders

* the "just once" decision Martin refers to

The first two decisions I'd wrap in functions to be called by replace(). The third I'd represent as a map<string,string>, ie as a data structure.

[Roman Leventov]

On Thu, 25 Apr 2019 at 00:40, John Ousterhout <ous...@cs.stanford.edu> wrote:

Also, it's not obvious to me that functions that can only be called in one place are a good idea. Isn't the whole idea behind functions that we want to create code that can be reused in many different situations? Use-once functions work against this. Again, I'd ask "how does this help to reduce overall system complexity?"

Personally, I'm not with Uncle Bob on his example, I would leave the original function as a single function in his example. I used his example and embedded a semi-function in it in my post just to make the code in my post comparable with his code.

However, I do think that functions must be split at some point. In my project, some developers apparently didn't think so, and there were many multi-hundred LOC functions, sometimes up to 800 LOC. They are a nightmare to work with until refactored into smaller blocks of approximately 50 lines most (my preference is that a function entirely fits the screen vertically).

When there is a giant function with sub-blocks, its internal interface (number of variables, execution paths) is exploding. Structural information, such as "if these variables are used (not used) in this block accidentally or not accidentally?", is missing.

Research back this feeling: cyclomatic complexity correlates with defect density. See M. Schroeder, "A Practical Guide to Object-Oriented Metrics" (https://pdfs.semanticscholar.org/e3d6/6c47ee0ddb37868c51ca30840084263ee1f1.pdf), Figure 4. 

[Roman Leventov]

I realized that "semi-functions", as proposed, can be a feature of IDE interface (like a presentation mode) entirely rather than a programming language construct. I've described the idea in detail here: https://youtrack.jetbrains.com/issue/IDEA-211820.

[Ralf Westphal]

I took the discussion here to heart and wrote a blog post about refactoring to functions: https://ralfw.de/2019/04/the-srp-is-not-about-extract-till-you-drop/

It results in quite some different code than Robert C. Martin's ;-)

[Indrit Selimi]

Hello Ralf,

Thank you for sharing your blog. I didn’t understood though why you appear not to be happy about ‘only one reason to change “ definition of the SRP and I would like to understand better from you. Indeed, to me, it provides a very valuable, specific and contextual guidance on how to apply it. The SRP is not about thinking abstractly what/why it can change using the developer free imagination but focusing on domain specific reasons (and hence roles) that could request a change. Why? Because that’s the software design about, trade-off decision about current goals and future soft-ness of the software. There are some axis of change that the designer will enable, not all, and it is better that those axis corresponds to the roles (reason of change) in the operating domain and context.
In reality, I’m afraid I didn’t understood much also the connection between the SRP and the ‘extract till you drop’ method that you was trying to make. Indeed, I think Bob’s code was illustrative of a concept, that’s why it is pushed to the extreme. Isn’t this the same also with the book that we are commenting here? For example, can anyone tell me when exactly a module is ‘deep’, according to this book? I think in similar way a concept is being expressed without really proving it (not sufficiently enough code and examples to really make the point) since I think the real dilemma is not about lines of code but about abstractions and no one will tell you exactly what to do because abstractions and modeling depends upon your very specific context.

To close, I think that before commenting Uncle Bob ‘s software school (be aware that there are many, all equally valid) one should first learn and understand it.

Have a nice day,

Indrit

[Ralf Westphal]

Since you seem to have understood already Robert C. Martin's SRP definition based on "one reason to change" and I haven't you might be able to explain to me how it applies to this function:

This is taken from Robert C. Martin's bowling kata implementation: https://www.slideshare.net/lalitkale/bowling-game-kata-by-robert-c-martin

I assume Robert C. Martin has followed his own principles and definitions. So this function in his view will conform to the SRP.

Now consider this:

* Which function would need to change if bowling was to be extended to more than 10 frames?

* Which function would need to change if knocking down 9 out of 10 pins with two rolls should be rewarded? Call it a "near miss" instead of a "strike".

* Which function would need to change if frames were to be kept in a linked list instead of an array?

It's always the same function: score()

To me that's looking like 3 reasons to change. Or at least 2: changes to game rules and technical matters.

You might say, I don't get how to define a reason for change. Ok. But I guess I'm probably an average developer. So if I don't get it as an average developer then it does not seem to be a helpful explanation.

Looking at score() from the perspective of decisions I immediately see that there are many encoded in it and that it's not following the SRP:

* Technical decision: usage of an array instead of a linked list to store frames.

* Domain decision: Which special situations to check

* Domain decision: What to do if a certain situation arises

In addition: Very formally it's mixing integration (calling functions) with doing stuff itself (logic). That's violating at least the SLA; and to me it's violating the SRP, too.

I'm happy for you, that you feel well guided by "one reason to change". However, I don't. I've tried, but it does not work for me. So I'm trying to find other explanations.

Sure, in the end there will always be room for individual interpretations. But the SRP is so important, I guess we should try a bit harder. And even Robert C. Martin does not seem to be happy with his own definition. He revised it at least 2 times already.

In the end what counts is:

1. does my fellow developer easily understand my code?

2. can my fellow developer easily test stretches of logic in isolation if she needs to?

Maybe question 1 can be answered with Yes regarding the above code snipped.

But question 2 can only be answered with No. The logic for handling e.g. a strike cannot be easily tested in isolation.

To me that means the code is lacking cleanness despite (or because?) of all the TDD effort and function extraction.

But that's my personal opinion. I would not accept it in a PR.

If you think otherwise that's fine with me.

[Indrit Selimi]

Hello Ralf,

To begin with the SRP (in SOLID) has nothing to do with what you are mentioning (‘a function do one thing’). Indeed it appears that you are confusing the two concepts and my suggestion would be to review your article from this prospective.

I’m not sure if I understood completely your answer (thank you very much for replying btw) I’ll read again carefully and in case be back to you.

Many thanks,

Indrit

[Indrit Selimi]

Hello Ralf,

Ok, I had some time to go through your message. In addition to my previous note,
I think you brought a very good point about the score() method. Interestingly though, your point seems backing what Uncle Bob predicates, indeed I would assume that according to the book the score() method is quite ‘deep’ (or let say that as minimum it resonates with the concept reported in the book, I don’t think [and do not want to think] it is the same concept though). But as you just showed it is quite fragile to change: whatever change comes in the method will require review and refactoring, potentially to a different model and/or sliced/changed, hence what previously seemed ’deep’ in reality couldn’t survive evolution...except from growing, with time, in an un-maintainable big ball of mud. In addition you mentioned also the very important point of isolating logic during testing, which, again, supports exactly the same above view.

Again, software design is an engineering task (i.e. dealing with trade-offs), for example the above score() method might be perfectly ok given the requirements known at the time, hence we should not think in extremes.

Moreover I would appreciate if you could kindly provide some more info about the following statements since I’m afraid I didn’t understood (probably because I don’t know what SLA means in this context):

“Very formally it's mixing integration (calling functions) with doing stuff itself (logic). That's violating at least the SLA”

Thank you very much for your answer, which in my view, supports uncle bob’s much more, maybe, than the ‘deep’-ness concept expressed in this thread (which I’m not sure if it is the one of the book, as I anticipated above).

Indrit

[Ralf Westphal]

SLA: http://www.principles-wiki.net/principles:single_level_of_abstraction

[Indrit Selimi]

Thank you for sharing. I confess, I didn’t read it entirely since too long, and hence not sure if I understand it (interestingly though I don’t remember any SLA ‘principle’ from the clean code as reported in the page...but I have read the book many years ago, still the description on that page sounds somewhat odd to me). But, nevertheless, reading it made me think that it is still another evidence supportive of uncle bob’s, instead.

I will avoid talking about ‘...one thing’ or ‘..till you drop’ since that is something related to low level details of function organization and, hence not relevant, in my view, to the discussions about “software design” (as this book aims to address).

I noticed that you, including the author of the book, seems to particularly question the difference between replace() and replaceAllSymbols() (hence pointing to the “shallow-ness” of the replace() method) like as if having the two methods instead of just one is equivalent when in reality it is not, the relying information is not the same indeed. For example, if I was a developer looking for the first time at the code, I will think that there is a ‘replace’ message to the current object and if I would look inside I’ll understand that the current implementation is hinting to some concept related to “symbols” which is hidden from outside...now, according to the author of the book (if I understood correctly) to be “deep”, either the main method should have to be called like the inner one leaking this information outside or that information should be sallowed inside the same replace() method making it “implicit”.

But according to the book, if the developer makes something implicit, instead of explicit-ing it when it should be done so, isn’t adding more complexity to the code? If I remember correctly from the complexity definition in the book (a brilliant and very well written section, in my humble opinion) it should.

To close, in my opinion, the evidence and examples that were brought against uncle bob’s are in reality backing his views and, in the same time, challenging the ‘deep’ concept as reported in this thread, which (highlighting yet again) I’m not sure if corresponds to same ‘deep’ concept explained in the book.

Indrit

[Ralf Westphal]

Since you did not make it through the short SLA article I doubt you've actually read my blog post in full. So maybe you did not even get to the point where I showed this refactoring:

I agree, the notion of symbol is a detail of the solution. It should not be leaked by the method name. "replace()" hides it well.

But to derive from that a call to just one function "replaceAllSymbols()" does not really move the solution forward. It's just an indirection - and I'm no wiser than before. How does the solution do its job?

And then in "replaceAllSymbol()" I see a loop which calls "replaceAllInstances()"? What's the difference?

Compare that to the simple process in the above code sample:

0. replace() does not leak the notion of symbol. Then inside that method...

1. compile all symbols - ah, it's about symbols at all! interesting information.

2. replace the symbol names with the symbols - ah, that's where the replacement happens; but now I know what's getting replaced.

If I'm interested I can now zoom in and step down one or the other branch of the function call tree. But inside replace() the whole process is explained. I can understand the solution in a breadth-first manner. Less need to step through with a debugger.

You don't need to like this approach - but please grant me at least that I can explain to you exactly why things are as they are. There is a method to my madness :-D

And please grant John Ousterhout (to whom you refer as the author of the book) that he too has reasons for not liking what you venerate. There is potential to advance the programming trade even after Robert C. Martin has uttered some opinion.

[Indrit Selimi]

Hello Ralf,

I read your post and to say the truth I read also your 'stratified design' article since linked. Thank you for you answer.  I think I understood your refactoring, and for sure it works, though the fact that both methods receive the same parameter ('stringToReplace') make me suspicious that these two methods might be tightly coupled (that being said, honestly, I don't think I'm able to judge others code, if I like it or not it is not relevant; if the technique helps you and your team and you are happy with it, why not).  

I do agree with John (the author of the book :-) ),  that in this specific case, the refactoring isn't bringing any relevant value since the original code was trivial enough and I think that Robert Martin just wanted to make a point with it. I agree that there is "danger" to push ideas to the extreme, ivi including the "deep"-ness concept.  As I already anticipated above, there are many software schools, one should learn from all of them. Robert Martin has his own, I have seen many complaining for some aspects (including myself), rarely I have seen real arguments though.  

Now, the real question is, do we really need a "deep-ness" principle or all the other principles & learnings about software design (R. Martin, K. Beck, M. Feathers, A. Grimm, S. Metz, N. Pryce/S.Freeman, your desk mate, etc.) allow us to produce deep components and functions? My hope was, regardless my "veneration of Uncle Bob",  to discover the answer from the book, not sure yet if it is the case. 

Many thanks for providing real code examples and sharing your thoughts.

Indrit

[Ralf Westphal]

I read your post and to say the truth I read also your 'stratified design' article since linked. Thank you for you answer.  I think I understood your refactoring, and for sure it works, though the fact that both methods receive the same parameter ('stringToReplace') make me suspicious that these two methods might be tightly coupled

You might have guessed it: I don't agree :-) I think the same parameter signals decoupling because now both methods can be used (aka tested) independently.

It's a kind of functional programming style. I like to keep shared state to the minimum.

Now, the real question is, do we really need a "deep-ness" principle or all the other principles & learnings about software design (R. Martin, K. Beck, M. Feathers, A. Grimm, S. Metz, N. Pryce/S.Freeman, your desk mate, etc.) allow us to produce deep components and functions?

Principles are not needed. Programming is not about principles. It's about results for the customer. Those results are:

* functionality - Does the software do what it's supposed to do?

* efficiency - Does the software do what it does efficiently, i.e. quickly, securely, user friendly, scalable etc.

And in addition the customer wants the development team to show

* high productivity from day one on until the software is discarded

It's the third requirement that's causing so much trouble. It's this requirement that books like "Clean Code" or "The Phil of Software Design" are addressing.

As long as your productivity does not drop markedly over weeks, months, years, all is fine. You don't need any principles. Just keep doing what you're doing. You're master of programming.

But for the rest of developers, the mere mortals... some guidance is needed.

High longterm productivity is supported by a couple of software properties:

* correctness

* understandability

* testability

* preliminarity

If your software is full of bugs (low correctness) then you don't have much time to add new features which actually bring in the money. You're drowning  in waste, your productivity is low.

If your code is hard to reason about (low understandability) then you have a hard time to figure out bug fixes and how to enhance it in response to new feature requests. You productivity is low because you're wasting time with "decoding" code.

If your code is hard to test (low testability) then it takes you a long time to figure out whether it's already correct (mature) and/or still correct (free of regressions). Your productivity is low because your wasting time with manual tests or leave stuff untested which means possibly incorrect which means you're gonna run into bugs later on.

If your code does not allow for preliminary changes (aka prototypes) (low preliminarity) then you're forced to work on production code even in cases where requirements are very uncertain. That leads to high code churn which will affect the other properties negatively. You're wasting your time with unnecessary refactorings or worse.

If you get to high scores on all those properties w/o principles, that's just fine. You're a master of programming.

But for the rest of developers, the mere mortals... principles (and rules, heuristics, patterns) are helpful. Applied within reason they are saving a lot of time. They are like a value system to attack a chaotic situation with.

Principles are a means to an end. So don't be dogmatic about them. On the other hand don't neglect them. It's a fine line to walk, a balance to keep.

There is a pre-/trans-fallacy haunting software development (http://www.ptmistlberger.com/the-pre-trans-fallacy.php): a lot of devs think they are already masters of programming and don't need principles. But in truth they haven't even started on the journey of high longterm productivity.

They deem themselves "trans", as having transcended principles. But what they are is pre: pre-principles.

What they need is to go through a conventional phase, a phase where principles are their dogma (or almost). And only if they can leave that behind they really become masters by transcending the principles. They have to go through the "dark night of programming" :-)

There a are quite some pre-devs. And there are many principle-attached devs. Only few have transcended the principles. And not all of those who keep saying "it depends" have.

But for everyone who tries there is hope, I guess :-D Just stay open minded. Don't attach yourself to this or that guru. Question everything. Always know why you do something.

Enjoy the journey through the world of principles - and beyond! :-D

[Indrit Selimi]

"You might have guessed it: I don't agree :-)"  -> which is not necessarily a bad thing in this case :-).  

"I think the same parameter signals decoupling"  --> what exactly is decoupled? I don't see any decoupling here... 

"because now both methods can be used (aka tested) independently." --> what needs to be tested in the 2nd function, the programming language itself? indeed it contains very low logic, all the logic is contained in the first function; in fact Replace_Symbol_names is a very good example of a shallow function... 

"It's a kind of functional programming style. I like to keep shared state to the minimum." --> to me seems more like a procedural programming style but since, from the way you have written your reply gives to me the perception that you are convinced about your ideas, I think there is no further point on continuing this conversation. 

Good luck!

[Peter Ludemann]

Here's a relevant code review: someone did "Uncle Bob" style refactoring and a reviewer pushed back:

https://github.com/psf/black/pull/1775/files/a607ada90c0a399eec0765166caec4fbea52c3a2#r509954528

tl;dr: the refactoring changed simple tests for determining the type of a numeric literal such as 

    if "e" in text:

into calls to functions such as

    def is_scientific_notation(text: str) -> bool:
        """Checks if the supplied string is a number with scientific notation"""
        return "e" in text

It seems to me that a better solution would simply be a brief comment

    if "e" in text: # scientific notation

[Marko Ristin-Kaufmann]

I like the approach with the comment. I think that the main drawback of the approach with shallow functions is that many people vastly underestimate how difficult it is to properly name functions. Many code blocks can be easily described with a sentence or two, but giving them a distilled and succinct three or four word name is a much more difficult task.

[Dan Cross]

On Thu, Oct 22, 2020 at 2:27 PM Peter Ludemann <peter.l...@gmail.com> wrote:

Here's a relevant code review: someone did "Uncle Bob" style refactoring and a reviewer pushed back:

https://github.com/psf/black/pull/1775/files/a607ada90c0a399eec0765166caec4fbea52c3a2#r509954528

tl;dr: the refactoring changed simple tests for determining the type of a numeric literal such as 

    if "e" in text:

into calls to functions such as

    def is_scientific_notation(text: str) -> bool:
        """Checks if the supplied string is a number with scientific notation"""
        return "e" in text

It seems to me that a better solution would simply be a brief comment

    if "e" in text: # scientific notation

I agree with you. I didn't mind separating out the actual formatting into separate functions, but the one-liner predicate functions were getting silly.

A thing I've observed over the years is that finding the correct balance between concision and abstraction is a matter of taste that develops over time. It is, perhaps, worthy to strive for some kind of local maxima in each stanza of code, but the pithy one-size-fits-all slogans of the world's "Uncle Bob"s aren't balanced against anything: they are just dogmatic "process" applied mindlessly.

I remember having a discussion about this with Peter Weinberger about ten years ago. He had apparently been keen on software processes at one point, but someone at Bell Labs pulled him aside and said something like, "that's for turning fifth-rate practitioners into third-rate practitioners" and he let it drop after that. I think the agile dogmas are similar.

        - Dan C.

[Benoît Fleury]

It seems to me that a better refactoring here would have been to separate the two concerns:

1. guessing the type of number literal

2. formatting each type

I’m guessing 1 could be useful in other places as well. It is also something that depends on the Python language designers, contrary to 2.

I agree with what professor Ousterhout said earlier in the thread. SRP is really about the abstractions (logical level) and not the code.

--
You received this message because you are subscribed to the Google Groups "software-design-book" group.
To unsubscribe from this group and stop receiving emails from it, send an email to software-design-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/software-design-book/CAEoi9W6aBKmuUakssN-PVc_8O0%2B2O8mZj5o-AKhku_8KnCHp0g%40mail.gmail.com.

[Paul Becker]

Two thoughts for the pile:

1. is_scientific_notation("not a number in scientific notation") -> True. So abstracting the statement 'if "e" in text' creates a bug if we use this function elsewhere, expecting it to do what it says it does. Which is a reasonable expectation.

2. This is in a 7k line file basically sitting alone in a directory, with 40ish classes in it and no comments at the top. I feel like there is a more coarse refactoring that should take priority. :D

Paul

To view this discussion on the web visit https://groups.google.com/d/msgid/software-design-book/84c678c5-9edd-477c-840f-1dcd03ac602en%40googlegroups.com.

[Jonathan Jones]

I think the real lesson that should be learned from

if "e" in text:

Is that there is a missing abstraction. 

Indeed when you look at the context, this comes from a function normalize_numeric_literals which accepts a Leaf object as an argument and mutates it. It seems to me that Leaf needs a subclass NumericLiteral. If you need the normalised text, then that's what __str__ is for.

Best,

JJ

To view this discussion on the web visit https://groups.google.com/d/msgid/software-design-book/89eed4d0-85f9-1d4f-fd34-be7ca380c57a%40gmail.com.

[August Karlstrom]

On 2020-10-22 20:27, Peter Ludemann wrote:
> It seems to me that a better solution would simply be a brief comment
>
>     if "e" in text: # scientific notation

And then you find that you need to allow also "d" and now you have to
find and update all places where you search for "e" in a numeric string.


-- August

[Peter Ludemann]

There's no end to that rabbit hole, piling abstraction upon abstraction. The art is in deciding when it's enough.

Even if I had abstracted if "e" in text into is_scientific_notation(), there's no guarantee that somewhere else is_scientific_notation() would be used -- I can't remember all the functions I've written for my own code, let alone keep track of all that other people have written.

(Here's an exercise for you: in JavaScript, convert a date into ISO-8601 form in the client's local time. Good luck in wading through the various date abstractions. (I'll agree in advance that JS's date abstractions aren't very good; but neither are most libraries'.))

 

-- August

[August Karlstrom]

On 2020-10-23 01:33, Paul Becker wrote:
> 1. is_scientific_notation("not a number in scientific notation") ->
> True.

If the implementation only looks for an "e" then
is_scientific_notation("0xdeadbeef") is true as well.


-- August

[Peter Ludemann]

In this particular code example, starting with "0x", "0o", "0b" have already been eliminated, so the correction abstraction would be is_scientific_notation_if_non_integer. 

;)

 

-- August

[Jeff Doolittle]

"Do one thing" fails because it doesn't address the fundamental issue that John highlights in the book: Problem Decomposition.

On Thursday, April 25, 2019 at 4:51:02 PM UTC-7 John Ousterhout wrote:

On Thu, Apr 25, 2019 at 12:41 AM Ralf Westphal <ral...@gmail.com> wrote:

Oh, one more thing: Robert C. Martin is asking at the beginning " What the hell does 'one thing' mean?"

This might be the crux of my disagreement with Uncle Bob. To me, "do one thing" refers to the abstraction visible to callers: the method appears to have a single "tight" function (something that can be described and understood very simply). The original code meets this definition.

Perhaps Uncle Bob thinks "do one thing" means that a method only has one operation in its body? I don't see any particular value in that definition.

-John-