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)?

3
  • 4
    SRP is not at all "do one thing". SRP is about classes and cohesion and "do one thing" is for functions (at at one level of abstraction)
    – Christophe
    Apr 17 at 11:49
  • Do you process each item individually or do you process each group of related items? Because it only makes sense to think of spliting the items if you process groups. Apr 17 at 14:08
  • 2
    ... I took the freedom to edit the wrong usage of the term "SRP" out of the post, so people can focus on answering the core question instead of explaining your misunderstanding (see Christophe's comment above.
    – Doc Brown
    Apr 17 at 16:00

4 Answers 4

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.

3
  • Not really sure what this was down-voted. That's basically how you do it. Apr 17 at 12:41
  • 4
    @GregBurghardt I suspect it was downvoted for saying Uncle Bob is not rational. Apr 18 at 4:08
  • 1
    Many principles seem to get dogmatically applied in cases where they are counter-productive. I think what's often more helpful is to think in terms of questions like "Would a function that does part of what this function does be useful". If the answer is yes, and such a function doesn't already exist, that would suggest that the functionality should be split. On the other hand, sometimes functions' usefulness may stem from the fact that they maintain invariants that require that if X is done, Y will also be done, and if Y fails X will be rolled back. Splitting undermines that purpose.
    – supercat
    Apr 18 at 15:19
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.

1
  • 1
    The perennial existential question for software developers: "Oh, what's in a name!?" --- Everything. Apr 18 at 17:24
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.

2
  • Granted. But if you thought it to be appropriate in a specific scenario (reactive programming, expensive source, very different handling of objects depending on their properties), how would you do it?
    – Christian
    Apr 19 at 9:02
  • And in general taking Martin's words as gospel can lead to some really messy code
    – inaba
    Apr 22 at 11:02
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
  }
2
  • Okay, say I can pull a banana or an apple out of my database. If it's a banana, I want to bake banana bread with it. If it's an apple, I want to use it for preparing fruit salad. I could call my method prepareFood() and be done with it. But if I then pull a Martini bottle out of storage next, preparing cocktails is no longer part of food preparation. At this point, groupBy probably no longer works. I could just map it all and use a switch case in the body. Polymorphism is out in my use case. So I thought let's have separate "paths"/pipelines for each. Is there a way to make that work?
    – Christian
    Apr 18 at 8:41
  • 1
    @Christian you want a part of code that does exactly one thing: dispatch the objects to whatever is intended to handle them. In an HTTP API, you always have a part which routes the requests to appropriate handlers. It's this kind of thing.
    – jaskij
    Apr 18 at 14:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.