C++: Why doesn't this sync() work in this Composition pattern?

Question

I'm trying to build a progressbar class that can have an arbitrary number of subprogressbars by using something that looks like the composition pattern.

let's say I have this class pbar:

class pbar
{
    public:
        pbar(const int w) { width = w; } // already sets the
        ~pbar() {}

         void setwidth(const int w) { width = w; } // set the width to w
         void show() const;
         void sync();

         void add(const pbar bar)
         {
              // add's a subbar
              subbars.pushback(bar);
         }

     private:
         std::vector<pbar> subbars; // the sub-process progressbars
         int width;                 // onscreen width of the pbar
};

As you can see the pbar has two members: the width and the subprogressbars (which are themselves pbars). I've been trying to implememt a sync function wich changes all the widths of the pbars in subbars to match that of the pbar it was called from:

void pbar::sync()
{
    for ( pbar bar : subbars )
    {
         bar.setwidth(width);  // first set the width of the subbar
         bar.sync();           // secondly make it sync up it's subbars
    }
}

but this does not seem to work. I've tried using this test program:

int main()
{
    pbar a(1);
    pbar b(2);
    pbar c(3);
    pbar d(4);

    c.add(d);
    b.add(c);
    a.add(b);

    a.show();
    std::cout << "syncing" << std::endl;
    a.sync();
    a.show();
}

with the show function defined as:

void pbar::show() const
{
    std::cout << w << std::endl;
    for ( pbar bar : subbars )
    {
         bar.show();
    }
}

The expected output would be:

1
1
1
1

yet it is:

1
2
3
4

The strange this is that the show() function does properly iterate down to all the subbars, but it looks like the sync() doesn't (in fact, using cout I've affirmed that in actually does, but it seems to have no effect).

What is wrong with my code? It is not the use of the c++0x type for loop, because I've tried using older iterator loops. I cannot find the mistake I made. I think it has something to do with the fact that I'm changing the wrong pbars when using setwidth in sync.

disclaimer: this is actually part of a larger project and the class is a lot more complicated than is shown here, but I've managed to reproduce the unwanted behaviour using the above code (which by the way is not copy-pasted and might contain typo's)

Answer 1

The issue you are having is that you're using a local variable, "bar" in the loop in your sync() method. That is making a copy of each of your subbars, and manipulating the copy rather than the original version (which remains in the vector). This is why you don't seen the changes "stick" when you later call the show() method.

You can probably fix this by using a reference instead of a regular variable. Try:

for ( pbar &bar : subbars )
{
    ...
}

You might want to make a similar change in your addSubBar() method, since you're also copying the values that you're passing in before saving another copy in the vector. You can skip one copy by making its parameter a reference. Avoiding the second copy would require some more care to deal with memory (which I'll leave for another question).