introducing pointers to a large software project

Question

I have a fairly large software project written in c++.

In there, there is a class foo which represents a structure (by which i don't mean the programmers struct) in which foo-objects can be part of a foo-object.

Here's class foo in simplest form:

class Foo
{
    private:
        std::vector<unsigned int> indices;
    public:
        void addFooIndex(unsigned int);
        unsigned int getFooIndex(unsigned int);
};

Every foo-object is currently stored in an object of class bar.

class Bar
{
    private:
        std::vector<Foo> foos;
    public:
        void addFoo(Foo);
        std::vector<Foo> getFoos();
}

So if a foo-object should represent a structure with a "inner" foo-object, I currently do

Foo foo;
Foo innerFoo;
foo.addFooIndex(bar.getFoos().size() - 1);
bar.addFoo(innerFoo);

And to get it, I obviously use:

Foo foo;
for ( unsigned int i = 0; i < foo.getFooIndices().size(); ++i )
{
    Foo inner_foo;
    assert( foo.getFooIndices().at(i) < bar.getFoos().size() );
    inner_foo = bar.getFoos().at(foo.getFooIndices().at(i));
}

So this is not a problem. It just works. But it's not the most elegant solution.

I now want to make the inner foos to be "more connected" with the foo-object. It would be obviously to change class foo to:

class Foo
{
    private:
        std::vector<Foo*> foo_pointers;
    public:
        void addFooPointer(Foo*);
        std::vector<Foo*> getFooPointers();
};

So now, for my question: How to gently change this basic class without messing up the whole code? Is there a "clean way"?

Answer 1

I'll take a stab at this.

It all boils down to what kind of memory management you want/need. You can either store pointers in Foo, or Foo objects. Since you said you do not need bar, which currently 'owns' the Foo's memory-wise, I suggest you put them directly in Foo:

class foo
{
public:
  add_child(foo child); //Foo by value. Utilize 'move' semantics if copy is not possible.
  const std::vector<foo>& get_children(); //Get a reference to the children. Could use iterators or whatever.

private:
  std::vector<foo> foos;
}

Usage:

foo parent;
foo child;
foo grand_child;
child.add_child(std::move(grand_child)); //move semantics.
parent.add_child(child);  //no move semantics.

parent.traverse(root);

void traverse(foo root)
{
    root.discombobulate_the_parafunctors(); //act on root

    auto children = root.get_children();        
    std::for_each(children.begin(), children.end(), traverse);
}

This is what I would try to move towards, but you said you want to do it 'gently' - I guess you mean incrementally, without breaking things? Since ownership is transferred from bar to foo in the example above, it will not be possible to use both at the same time. The pointer approach, however, can be layered on top of the existing bar implementation.

If I really wanted to 'gently' switch, I'd make a class called something like foo_tree_view, which is like your first approach, with pointers, and doesn't assume ownership. I'd build it from a given bar instance, and then use it in client code.

std::vector<foo_tree_view> bar_view;

class foo_tree_view
{
public:
  foo_tree_view(foo this_foo, bar source);
  std::vector<foo*> get_children();
private:
   ...
}

At this point, you start gently changing all places to use this new foo_tree_view class. When all code uses it, you change to implementation of foo_tree_view to use the above implementation, without 'bar', and rename it to foo.

Hope you'll make some sense out of this answer!

Answer 2

What I'd do:

1. Add a member variable Bar * bar and int index to Foo.
2. Have Bar::addFoo set foo->bar = this and foo->index = foo.size()
3. Implement Foo:getFoo on Foo to use the getFooIndex and bar to return a foo pointer
4. Implement Foo:addFoo similiarily

Then you should be able to use both APIs simultaneously. You can deprecate the index based apis and slowly remove them.

Answer 3

I don't think having the foo object be responsible for its index in a container is a good idea. It should be blissfully unaware of the container(s) it is in. Maintaining these indices is the job of the container (if it is applicable for that container).

Your current code requires a foo to retrieve the inner foos from bar so why not just retrieve them directly from foo?

for (unsigned int index = 0; index < foo.getFooPointers().size(); ++index)
{
    Foo* inner_foo = foo.getFooPointers().at(index);
}

Why do you need the inner foos stored in bar? If there is some situation where you want to loop through all foos, including inner ones, you could create an iterator.

As a side note I would probably call the member function something like getInnerFoos() or getChildren() instead of getFooPointers(). The signature already tells you that it returns pointers of type foo.