4

I regularly have the situation that I have some complex logic that I break into steps. For example an import of an Excel file where I need to apply some logic or a complex calculation that I break into smaller calculations.

Another situation is shown below. This is one big User Story: As a user I want to show the graphs. So I break this into steps:

  1. Get a scenario domain model from database based on a given id.
  2. Load data from external weather API: based on the location property of the scneario loaded by step 1 (Location has lang and lat properties);
  3. Save the weather data from step 2 into the database. We also need the scenario of step 1, so I can store the weather data to the specific location.
  4. Perform calculations based on the data I get from step 2 and 1. This is very complex calculator. Scientists have made many calculations and I have translated them into C#. The outcome of all those calculations are based on hourly weather data.
  5. Based on the outcome of step 4 and 1 aggregate values by month. So we go from hourly calculations to monthly calculations.;
  6. Save the data genereted in step 4 and 5. This data is stored because the calculation is step is heavy and when the data is wrong we can use the stored data to debug and find out why the data is wrong.
  7. Generate graph data based on the outcome of step 6. The graphs (10 of them) are based on the monthy values, which are calculated in step 5;
  8. Save the genereted graph data with data from step 1. I Store the genereted outcome so when we need to show the same graphs next time, I don't have to calculate everything again.

As you can see, each step requires the outcome of a previous step, but every step needs the data from step 1.

What I usually do in these situations is to create a Context object. This object contains properties for each step. So each step, sets it's property on the context. And off course this context is passed to every step. So in the case above I have a context class with 5 properties (the saving steps don't set anything on the context).

Next, every step implements this interface:

public interface IStep
{
   void Execute(Context context);
}

Then in the root class I manually compose a List with all these steps and finally I loop over this list so every step gets executed.

I don't feel very happy with this and I was wondering if this can be done smarter. It doesn't feel SOLID:

  • chances are that values on the context are null;
  • for every step I have to modify the context class;
  • context class can become very large;
  • not solid at all...

So how would you handle such a scenario?

  • 1
    Does Chain of responsibility design pattern address the issue you're having? – Andy Jun 10 at 13:15
  • Why are you creating a separate class for each step? Are those step classes reusable in other scenarios, or are they really only usable in this one story? – Bart van Ingen Schenau Jun 10 at 14:09
  • @Bart van Ingen Schenau Those classes are not reusable They're only used in this one story – Martijn Jun 11 at 21:37
5

Your gut is right, that's sub-optimal for an object-oriented design.

What you're doing is basically having a procedural decomposition, that is, you are thinking about the technical steps needed to make your code work. You even created an abstraction over basically a generic procedure and called it IStep.

This is unfortunately a completely natural way to decompose a problem for us technicians, but is a poor choice when it comes to object-orientation. In OO we want to identify things we have, not technical steps needed to be taken.

So, for example if your requirements are to show a graph of a report, I would start with something like this:

interface Report {
   Graph displayGraph()
}

Then, try to find out what are the 2-3 things this report is based on and define those, try to implement Report with those things. Then continue break it down. At each step, the thing should be a thing in your domain, not some technical detail like database, webservices, processor, step or things like that.

The difference is this gets you a hierarchy of proper objects that hopefully reflect the domain, not a linear execution of random procedures on a common data structure.

Note: this requires a complete re-thinking of how to design, and is obviously not easy to do, but it does solve your problem.

| improve this answer | |
  • 1
    Thank you very much for your answer. I really like to learn on how to think in OO and how to think in 'things'. Could you add more pseudo code based on my post so I can get a better hang of it? I would really really appreciate that :) – Martijn Jun 11 at 7:56
  • @Martijn There is not enough domain knowledge given in your description to propose proper objects. What are the "things"? What is the data loaded from the database (what is the "thing")? What is the data from the external API? Why is it needed for the report? What is the graph? In what are these things transformed to (the "calculation")? Etc. – Robert Bräutigam Jun 11 at 8:06
  • I understand. I have updated my post and make it more descriptive. I hope this helps you more. – Martijn Jun 11 at 8:24
2

In trying to adhere to some acronym, you are overengineering your solution.

The steps you mention are excellent candidates to be (private) methods in a class, with the method you have now for composing and iterating over the list of abstract Step classes calling those step methods instead.

As the step-methods don't need to have all the same signature, you don't have to invent a Context class that is half-empty most of the time, because you can pass to each method exactly what it needs.

If you think that this goes against the Open-Closed principle, think about what changes need to be made in both cases when a new step has to be inserted.

With Step classes, you create a new, single-use, class and you modify the orchestration code to insert an instance at the right point in the sequence. Possibly, you need to update the Context class.
With the step methods, you create a new method and you modify the orchestration code to call that method at the right point in the sequence.

In both cases, the step code of the existing steps is unchanged and the orchestration code needs to be modified.

| improve this answer | |
  • Thanks for your answer. I think I understand what you are saying. The thing is, is that each step contains a lot of code, therefore I put every 'step' in it's own class, to get things maintainable. Otherwise I would have one big God class.. – Martijn Jun 15 at 7:58
  • @Martijn, I fully understand it if you want to split the code over multiple files to keep the file sizes manageable. But don't think that a big class is automatically a God class. A God class is by definition involved in multiple (most?) scenarios. – Bart van Ingen Schenau Jun 15 at 8:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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