How to not violate the single responsibility principle?

Question

Let's say I'm writing a class that tracks a single stock ticker. In this simplified example, the StockTracker class holds a string that tells me the trend direction and a variable window that holds x amount of historical prices.

Now I want to expose this class so whenever a new stock price comes in, I can call a function to update the stats and trend for this stock. So I make a public function that calls two private functions, each having one responsibility each

class StockTracker(val window: List<Double>, val trendDirection: String) {

    private fun updateWindow(xxx) {
        // Long business logic goes here
    }

    private fun updateTrend(xxx) {
        // Long business logic goes here
    }

    fun ingestNewRate(newRate) {
        updateWindow(newRate)
        updateTrend(newRate)
    }
}

In this scenario:

1. Am I following the single responsibility principle? Technically the function ingestNewRate is doing two things: updating stats and updating the trend. Is this fine?
2. When unit testing this, would I have to test the 2 private functions?
3. Should I be exposing updateWindow and updateTrend in new classes so that they can be a publically available function that can be tested?
4. Is there anything else I should be looking out for in this example that is potentially bad practice?

Answer 1

The problem here is not about SRP or do-one-thing. This is about how you're storing your state.

Under SRP StockTrackeris perfectly fine. SRP is not against having different methods or composing "functions". It's against lumping together, thru composition or whatever else, different reasons to change the code. No class should serve more than one master. Two masters can demand conflicting changes to the code. There is a problem here but that's not it.

ingestNewRate() follows do-one-thing so long as the code underneath focuses on ingesting a new rate. updateWindow and updateTrend both seem to focus on ingesting the new rate. Just into different things. The name ingestNewRate has a singular focus. Just don't betray the name. This isn't the problem either.

Here's the problem:

You are storing redundant information. Redundant state means potentially conflicting state. If window: List<Double> shows that the trend direction is up but trendDirection: String show's that it's down then which is correct?

What you're violating here is called Single Source of Truth. It's only very loosely related to SRP but it's a different idea. This is about avoiding conflicting state by storing that state only once. Not in two potentially conflicting ways.

What you need is a public method that expresses some useful work that the object has done with that state. If you were only storing window: List<Double> then you could calculate "up" or "down" from the last two doubles. Since "up" and "down" are no longer internal state this doesn't break encapsulation. That method could be toString(). It could be calculated against window: List<Double> every time it's called.

Write your tests against that public toString(), not any private methods, and you're free to change your private structure without breaking your tests. The outside world doesn't need to know you changed anything. We call that refactoring. Don't prevent refactoring by writing tests that lock you into particular implementations. Tests should prove what code does. Not how it does it.

Answer 2

This seems not really about Uncle Bob’s Single Responsibility Principle, which is about classes and their reason to change.

But it is definitively about the do-one-thing rule, recalled by the same author, and which is about functions and methods.

In this regard you did well: your top function does one thing: ensuring that the stock price is added. This involves several operations, that are organized according to functional decomposition, delegating to other functions the doing of each of these more detailed operation. And moreover you respected the single level of abstraction rule.

All your other questions are not directly related to these principles:

• tests depend on your test strategy. The unit test is meant to test the elementary unit of functionality delivered. I’d understand it as testing the public functions. The private functions are behind the scene and only relevant if you know the internals;
• what to expose publicly is yet another question, and only you can decide: could the stats or trend calculations make any sense on their own?
• Finally, even if you keep your auxiliary functions private, you could encapsulate them in a strategy. Not for SRP, not for Do-One-Thing, not for tests, but because it makes your design more flexible. This makes only sense if you could envisage using alternate implementations/formulas/algorithms. In this case the auxiliary methods would be public within the strategy class, and hence unit testable on their own.

Answer 3

The two functions updateWindow and updateTrend should be implemented in classes that implement an interface (lets say an IUpdatable interface).

IUpdatable
  Update(...)

StockTrackerWindow : IUpdatable
  Update(...) 

StockTrackerTrend : IUpdatable
  Update(...) 

The StockTrader class can then delegate the updating of window and trend, and anything else in the future, to these concrete classes.

class StockTracker(val window: List<Double>, val trendDirection: String) {
 
    List<IUpdatable> updatables;

    fun ingestNewRate(newRate) {
        updatables.ForEach(Update(newRate));
    }
}

In this way the StockTracker has a single responsibility, to update each of the updatables. The concrete classes StockTrackerWindow and StockTrackerTrend each have a single responsibility and can be unit tested separately.

In terms of testing you can now used Mock to inject whatever behavior you want into the StockTracker; even mocks that throw exceptions.

The design of StockTrader now depends on abstractions.