tags:

views:

153

answers:

2

I'm trying to write a simple color class that's supposed to be as versatile as possible. Here's what it looks like:

class MyColor {
private:
uint8 v[4];
public:
uint8 &r, &g, &b, &a;

MyColor() : r(v[0]),  g(v[1]), b(v[2]), a(v[3]) {}
MyColor(uint8 red, uint8 green, uint8 blue, uint8 alpha = 255) : r(v[0]),  g(v[1]), b(v[2]), a(v[3]) {
    printf("%d, %d, %d, %d\n", red, green, blue, alpha);
    r = red;
    g = green;
    b = blue;
    a = alpha;
}
MyColor(uint8 vec[]) : r(v[0]),  g(v[1]), b(v[2]), a(v[3]) {
    MyColor(vec[0], vec[1], vec[2], vec[3]);
}
uint8 operator [](int i) {
    return v[i];
}
operator const GLubyte*() {
    return v;
}
};

And here's the code I'm trying:

uint8 tmp[] = {1,2,3,4};
MyColor c(tmp);
printf("%d, %d, %d, %d\n", c.r, c.g, c.b, c.a);

(I would have liked it if I could have done MyColor c = {1,2,3,4} but I'm not sure that's possible in the current spec?)

Anyway, it outputs this:

1, 2, 3, 4
112, 22, 104, 89

So the values it gets in the 2nd constructor are correct, but when it returns... those values are random??

r = red should set both r and v[0] to red shouldn't it? Since r is just a reference to v[0] they are actually share the same value, no? I'm not doing some weird reassigning of the reference to somewhere in space am I?

+4  A: 

Unfortunately, you can't do constructor forwarding at the moment in C++. The issue is here:

MyColor(uint8 vec[]) : r(v[0]),  g(v[1]), b(v[2]), a(v[3]) {
    MyColor(vec[0], vec[1], vec[2], vec[3]);
}

What this actually does is to bind the references to the member vector v and then in the body of the constructor create a temporary MyColor value which is then thrown away.

The second line in your output is printing the garbage initial values of the member vector v of the constructed MyColor.

I'd recommend breaking out the value assign part of the constructor taking 4 uint8s and calling that from both constructors.

void AssignColorValues( uint8 red, uint8 green, uint8 blue, uint8 alpha)
{
    printf("%d, %d, %d, %d\n", red, green, blue, alpha);
    r = red;
    g = green;
    b = blue;
    a = alpha;
}

MyColor(uint8 red, uint8 green, uint8 blue, uint8 alpha = 255) : r(v[0]),  g(v[1]),     b(v[2]), a(v[3])
{
    AssignColorValues( red, green, blue, alpha );
}

MyColor(uint8 vec[]) : r(v[0]),  g(v[1]), b(v[2]), a(v[3])
{
    AssignColorValues(vec[0], vec[1], vec[2], vec[3]);
}
Charles Bailey
Oh! *That's* what it's doing. Guess I should have picked up on that. We can't even do constructor forwarding? That seems like a pretty dumb limitation of C++.
Mark
As you can break out functionality into a member function for most things (see my update), constructor forwarding is only a big benefit when you are sharing some complex expressions in the initializer list of multiple constructors. I've only really missed the feature once or twice and it will be in the upcoming new version of C++.
Charles Bailey
Ah.. that's what I ended up doing anyway, but because I needed a set function anyways. Dual-purpose :)
Mark
A: 

Actually, I believe that you can do most of this today. boost::array can be initialized from a constant expression, so you should be able to as well. I believe that you will have to get rid of the other constructors, however. You can keep assignment operators, if that helps.

keraba