I am using a particular pattern that appears often in one of my programs, and I can't see if I am doing a bad code architecture or if it is a good thing to do.
Let's imagine that we have a Class A
, defined as follows:
template <typename T>
struct DataProcessor {
DataProcessor(const T& processedData) : m_processedData(processedData) {};
const T m_processedData;
}
template <typename T>
class A {
public:
// Constructor used by the user: takes unprocessed data as input
A(const T& unprocessedData) : A(getDataProcessor(unprocessedData)) {}
// True constructor, called by user constructor and working with DataProcessor structure
A(const DataProcessor<T>& dataProcessor) : m_processedData(dataProcessor.m_processedData) {}
T getProcessedData() const {
return m_processedData;
private:
static DataProcessor<T> getDataProcessor(const T& unprocessedData);
const T m_processedData;
};
Now let me explain my logic: the thing is that the class A should take as input here unprocessed data, BUT should work only with processed data that will always be constant and never able to be modified, so in my head should absolutely be constant. But since we do not know them at the instantiation because they need to be computed, I need to pass by another structure (here DataProcessor) computing them. To me it makes sense, although I can clearly see that it is not very readable and it's getting a little bit out of hand now that my code is getting bigger and more complicated. I should also add that at the origin, I wanted to have the DataProcessor structure declared inside the private part of class A, since it will always be used there and only there.
Is this a common way to build a class, or should it be avoided? If yes (or no), why?
This is fine and I have seen it many times. You provide a "helper" function that transforms an argument to the actual argument you need. This has many uses, is easy to use, and can be extended by other constructors.
That said, there other things you could do which I've seen often:
Regarding your example - it may be simpler to just have a method/function returning processed data and setting the field directly, rather then jumping between constructors like that.
Just have the constructor taking processed data, probably by &&. The caller is then in charge of passing the processed data and yielding ownership of it. This has the benefit of complete segregation of the two concepts (A, and the data processor), and gives the caller more flexibility in how they create processed data.
If this is still a very useful construct (unprocessed data -> A with processed data using a specific data processor), you can have a static Create
method that creates your object and returns them. This has the benefit that if your processing fails, you can cleanly return an error with no exceptions.
In both your example and (2) here - the struct is an implementation detail. Both it and the constructor that takes it can be made private, the dataprocessor struct defined within A. There's a good chance the data processing can just be a method. Otherwise using (1) often makes sense, giving the caller responsibility for the processor and data processing.