0

I am working with an existing numerical model implemented in C++, and I don't have much of a background in software design. This model reads in many (~100) of structured text files, performs numerous calculations, and then spits out an answer. The current organization of the program is as follows:

  1. Main function creates a DataBox object, an instance of a data holder class, which has public variables for all the variables that are used in more than one step. DataBox has ~100 variables in it, including matrices, strings, maps, etc.
  2. Reads in the data and stores it in the DataBox.
  3. Calls calc1(&DataBox), stores the results in DataBox in the function, calls calc2(&DataBox), etc. until done. calcn depends on values from step 2 as well as calc1->calc(n-1), not just n-1. The same instance of DataBox is passed to each function.

This DataBox approach seems to have the many of the same disadvantages as using global variables. Although the DataBox doesn't pollute the global namespace, it can be difficult to determine where a DataBox member variable is first initialized, and where in the code it is possibly modified. It can also cause merge problems, as the merge can occur without conflicts, but then the program fails because a member of DataBox was modified without the other file knowing.

The obvious alternative to me is to explicitly pass the variables needed at each step rather than the DataBox, rather than the full DataBox, but this is annoying because each step can require lots of variables from prior steps, and we sometimes want to change which data is available. Is there a better alternative? One idea I had was to make all the DataBox variables private and then create only create getter functions, so that the data can be accessed easily, but not modified. However, it is not obvious to me how to control the context in which setter functions can be accessed.

  • You could make all of your databox variables const, and set them in the constructor. stackoverflow.com/a/7170186 – Robert Harvey May 14 at 22:20
  • The constructor has to be done all at once though right? I could construct the object and set the data read in as const, but then what about the data I can't calculate until step 3? – yakzo May 14 at 22:23
  • Aye, there's the rub. – Robert Harvey May 14 at 22:24
2

The risks

First of all, there is nothing fundamentally wrong in using DTOs to carry the data between process, threads, or even functions. In principle, such DTOs should combine data belonging to well defined business objects, and follow clear transfer rules (e.g. from A to B; or from A to B with C enrinching it).

But in your case, the all calculation functions could read or write a lot of different members without clear ownership and without single responsibility. So your DataBox seems indeed to be only a substitute for global variables that lacks proper encapsulation.

This has several consequences:

  • lack of encapsulation: if tomorrow you'd like to change some of the public member variables, it could potentially affect all the code, since it's unclear where the variables are used or updated.
  • tight coupling: in fact calc1(), ..., calcn() are tighly coupled. If you want to change how or when variables are calculated, in one of the function, it might impact all the others.
  • no parallelisation potential: while one could hope for opportunities to parralelize or to pipeline the calculations, the lack of encapsulation and the tight coupling will prevent you doing so.

I would see this as a kind of OO version of spaghetti code. But there is not enough infos about what’s really in the databox, nor about data volumes, so my arguments are to be taken with care.

Reducing the risk

Protecting access to the variables through getters and setters would improve encapsulation. You could at least change the internal representation of some results and hide these changes to the calc() functions. But this will not remove the tight coupling (it's just more visible) nor allow for more parallelisation.

You could combine this with a state pattern if you want to disable some setters depending on the progress of the processing on the databox.

Improving the design - decoupling and enabling parralelization

A better solution could be to use several different DataBoxes, e.g. one DataBox1 (or several) for input, and one DataBox2 (or several) for output, and this for every step. This would decouple the calculation steps:

  • instead of having every step coupled with every other, you'd have only every step coupled with the previous step.
  • if multiple functions would write the same PartialDataBoxXYZ, you would be aware of it, and could think of some refactoring.
  • you could even draw a graph of how data flows accross the differnt functions to better understand the relationships.

Adding parallelization - if needed

Based on this improvement, you could then even think of processing DataBoxes in an asynchronous manner: every calc could start as soon as input databox is ready, even if previous calcs are still working for some other results.

Depending on how you process and combine your sources, it may even be possible to stream the processing, using an input queue of specific DataBoxes for every calcn(). The calcn() would then feed the queues of the subsequent claculations. Using such an architecture would allow you to parralelize the processing by having several concurrent workers running the same claculation step but for different DataBoxes.

Conclusion

If the first proposed improvement doesn't work, because there would be too many different intermediary partial databoxes, then I'd suggest to refactor your functions. Because , maybe it's not the data which is the problem, maybe it's only the functions that are doing to many things.

  • Well, we use DTO's all the time in business applications, and nobody ever complains that they're just proxies for global variables, although sometimes people do complain about anemia. – Robert Harvey May 15 at 14:46
  • Hmm, creating a new class for each calcn is exactly the annoyance the DataBox fixes though. To elaborate, the process is a more complex version of this (many more inputs for each function): Start with x1, x2. calc1(x1) gives x3 calc2(x1, x3) gives x4 calc3(x2, x3) gives x5, x6 calc4(x1, x4, x5) gives x7 If I create a new class for each calcn, there will a lot of code just initializing all the objects for each function call it seems. – yakzo May 15 at 15:07
  • @RobertHarvey My point here is not so much about anemia nor about using DTOs to carry the data of well defined business objects. DTOs are in many cases a perfectly valid approach. I worry here about the fact that distinct calculation blocks could all read or write a lot of different members without clear ownership and without single responsibility. I would see this as a kind of OO version of spaghetti code. But there is not enough infos about what’s really in the databox, nor about data volumes. Maybe it’s simpler than I thought. But maybe there are coupling and mass-processing issues at stake – Christophe May 15 at 15:14
  • @yakzo could you edit your question and clarify: 1) what’s in your darabox (2 variables? or 2000?) - 2) if there is a mass processing issues (you speak of hundreds of datasources) - 3) what relation there is btw databox instance and data source - 4) what reason makes you really doubt of your current solution if it’s not about coupling and encapsulation ? – Christophe May 15 at 15:20
  • Edited question. My concern indeed is "I worry here about the fact that distinct calculation blocks could all read or write a lot of different members without clear ownership and without single responsibility." – yakzo May 15 at 17:44
1

Make the generational nature of the data explicit:

  1. Create Databox1 from the initial set of values in step 2. Databox1 is immutable.
  2. Pass Databox1 to the next stage, producing Databox2; Databox2 either holds a reference to Databox1 or copies all the fields in place. Databox2 is immutable.
  3. Pass Databox N to stage N, and produce Databox N+1.

The immutability happens in stages. By wrapping each stage in its own box, you can determine what stage computed the value. Each field is immutable, so you know it never gets changed; if a value gets "updated" at more than one stage, keep each value separate and immutable and wrapped in its own box.

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.