0
\$\begingroup\$

I have a data structure consisting of one main Parent container that allows direct access to the Child objects it contains. These Child objects all have their own data, but also have some shared data (in the example provided, importantIndices). To prevent data duplication between the thousands of Child objects, I've stored the shared data in the Parent object, and passed a pointer to the Parent to each child, which they can use to access the shared data.

The issues are that I feel that this breaks OOP encapsulation, and it makes it hard to test because the objects are tightly coupled. However, it also seems to be the best option - in the real scenario, there's enough shared data and enough Child objects that duplication is not an option. Is there a better way to implement this concept that allows for less coupling, while keeping the data available to all Child objects?

Unfortunately, even the names of the actual objects would be giving away proprietary information, so here I've provided a minimal working example of the problem I'm facing. Also, if this would be more appropriate at a different stackexchange site then please direct me there, but it didn't feel as much like a stackoverflow or softwareengineering question as a codereview.

Provided below are a Child class, a Parent class, and a basic example of their use in main.cpp. Note that I also have Qt and C++14 available, if those could help.

child.h:

#ifndef CHILD_H
#define CHILD_H

#include <vector>

using std::vector;

class Parent;

class Child
{
    /* This feels awkward - a circular dependency of sorts that breaks
     * encapsulation entirely.
     */
    Parent* myParent;

    /* Some arbitrary integer data. */
    vector<int> myData;

public:
    Child(Parent* parent, int totalIndices);

    void setData(int index, int data);
    int sumImportantData() const;
};

#endif // CHILD_H

child.cpp:

#include "child.h"
#include "parent.h"

/* Constructor. Initializes myData with size totalIndices. */
Child::Child(Parent* parent, int totalIndices) :
    myParent(parent),
    myData(totalIndices)
{}

/* Sets the data at the given index. */
void Child::setData(int index, int data)
{
    myData[index] = data;
}

/* Returns the sum of the data at the important indices. */
int Child::sumImportantData() const
{
    const vector<int> &indices = myParent->getImportantIndices();

    int sum = 0;
    for (auto iter = indices.cbegin(); iter != indices.cend(); ++iter) {
        sum += myData[*iter];
    }
    return sum;
}

parent.h:

#ifndef PARENT_H
#define PARENT_H

#include <vector>
#include "child.h"

using std::vector;

class Parent {
    /* Container of child objects. */
    vector<Child> childVect;

    /* Data indices that are actually important - we store data at all indices,
     * but don't use that data in most cases. We only use the data at the
     * important indices for data analysis.
     */
    const vector<int> importantIndices;

public:
    /* Constructor. Initializes numChildren children with totalIndices points
     * of data, where the important indices are in importantIndices.
     */
    Parent(int numChildren, int totalIndices,
           const vector<int> &importantIndices) :
        importantIndices(importantIndices)
    {
        for (int index = 0; index < numChildren; ++index) {
            childVect.push_back(Child(this, totalIndices));
        }
    }

    /* Returns a const reference to the vector of important indices. */
    const vector<int> &getImportantIndices() const
    {
        return importantIndices;
    }

    /* Allows direct access to Child objects. */
    Child& operator[](int index)
    {
        return childVect[index];
    }

};

#endif // PARENT_H

main.cpp:

#include <iostream>
#include "parent.h"

int main()
{
    int numIndices = 7;
    Parent container = Parent(200 * 200, numIndices, std::vector<int>{2, 4, 5});

    for (int i = 0; i < numIndices; ++i) {
        container[235].setData(i, i * 100);
    }

    std::cout << container[235].sumImportantData() << std::endl;

    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Why couldn't the parent object allocate some shared data container and inform each of its childrem about that container, but not about himself? \$\endgroup\$ – CiaPan Oct 24 '17 at 9:19
0
\$\begingroup\$

I'd say that the Child doesn't necessarily need a reference/pointer to a concrete Parent - it wants to reference one of the interfaces that Parent implements. Specifically, a SharedChildData interface (you can think of a better name - it's a bit difficult with borderline-offtopic placeholder code).


Or perhaps it makes sense to decouple child and parent by passing the importantIndices collection to the method? Like this:

// in Child
int sumImportantData(const vector<int>& indices) const;

Then you could ask the parent to sum important data of a particular child

// in Parent
int getChildSum(size_t child_index) const;
// or pass an iterator etc instead of index

Child has no copy or assignment, and uses the parent pointer without checking - that smells like a reference would be better than a pointer. Also, if you don't need to modify the parent from the child (and if you do, that's a danger sign for the design), then prefer a reference to const:

class Child
{
    const Parent& myParent;
    // or const SharedChildData& mySharedChildData;

    // ...
};

This method looks cumbersome:

int Child::sumImportantData() const
{
    const vector<int> &indices = myParent->getImportantIndices();

    int sum = 0;
    for (auto iter = indices.cbegin(); iter != indices.cend(); ++iter) {
        sum += myData[*iter];
    }
    return sum;
}

A range-based for loop would be clearer and shorter:

int Child::sumImportantData() const
{
    const auto& indices = myParent->getImportantIndices();

    int sum = 0;
    for (auto& i: indices) {
        sum += myData[i];
    }
    return sum;
}

You might be able to get something even clearer using std::accumulate().

\$\endgroup\$
  • \$\begingroup\$ That works perfectly. I moved the shared data out of the Parent and into its own class that gets constructed by the Parent and passed in to the Child class' constructor, in which it's stored by reference. Works as intended with a relatively clean interface, and is much more easily testable/mockable. Thanks! \$\endgroup\$ – Jamie S Oct 25 '17 at 21:07
  • \$\begingroup\$ It's very gratifying to know that my review was helpful. The accept vote is appreciated; the comments even more. \$\endgroup\$ – Toby Speight Oct 26 '17 at 7:51

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.