6

Uncle Bob mentions in his book "Clean Code" that "[f]unctions should do one thing [...] only" and advocates that a method should stick to one abstraction level (I'd call it flight level). I struggle a bit on how to do that in reactive programming, though.

Say I've got an expensive database call. And that depending on a property on the objects it returns, I want to process these objects (very) differently.

I have seen pople use the groupBy(...) operator - but that means one method (in which the groupBy is used) usually ends up doing multiple things (in both senses: tasks and flight levels) - at least in the examples.

I've thought about splitting the stream (a related question with an example is on SO), but it doesn't feel quite right, either.

So I was wondering if there is any literature (blog posts, articles, books) or good examples on e.g. GitHub on the topic of how to break a reactive stream up into building blocks so that it conforms to the "do one thing" principle? (I guess usually you'd just open up new subscriptions, but when the source is expensive to obtain, that's not a good way to do it, either, I guess.)

Or if there's a different way to tackle the issue (from splitting the stream)?

Doc Brown
  • 218,378
Christian
  • 171
  • 1
  • 5

4 Answers4

20

What you're asking how to do is called decomposing. "Do one thing" never meant your function couldn't be decomposed into more things. Every function can be decomposed into more things. No, "do one thing" means stick to your thing. Don't do random other things.

The biggest clue of what your functions thing should be is its name. I offered "take out the trash" as an example of a function that shouldn't also "bring in the mail" back here. This follows The Principle of Least Astonishment. After looking at the functions name I shouldn't be surprised by what I find inside.

I have seen pople use the groupBy(...) operator - but that means one method (in which the groupBy is used) usually ends up doing multiple things (in both senses: tasks and flight levels) - at least in the examples.

So long as those multiple other things fit nicely under the heading of one thing this is fine. If they are not at all what one would expect to find in here then something is wrong. Either the name of the function needs to change or this stuff needs to be moved elsewhere.

I've thought about splitting the stream

Structural changes can be important here mostly if they give you an opportunity to add another name. But realize, at it's root, the problem you're grappling with here is semantic. Make the names make sense.

candied_orange
  • 119,268
17

“Do only one thing” may include “do everything to display the user’s information on the screen and allow editing it”.

Taking SRP too literal is a recipe for disaster.

gnasher729
  • 49,096
10

In general, do not follow any principles suggested without a rationale (and Uncle Bob is not rational).

To follow the principle blindly, you can introduce a layer of abstraction - a dispatcher.

It takes a Stream, and for each of its elements, calls either task A, or task B.

Pseudo code:

function dispatch(stream) {
    stream.forEach(element -> {
        if (element.hasProperty) {
            taskA(element)
        } else {
            taskB(element)
        }
    });
}

For further separation of concerns, pass taskA and taskB as arguments.

Basilevs
  • 3,896
4

This is one abstraction level (pseudocode). The "one thing" it does is sequence step1 and step2.

stream
  .groupBy(step1)
  .map(step2)

This is multiple abstraction levels. It is doing both the sequencing and the implementation of the lower abstraction steps.

stream
  .groupBy{
     // Long inline implementation of step 1
  }.map{
     // Long inline implementation of step 2
  }
Karl Bielefeldt
  • 148,830