Limits for struct member

For the first iteration of the walk through, we have min / max functions that we can and should use:

struct Color 
{
    explicit Color(double x): r{x}, g{x}, b{x} 
    {
        r = std::max(r, 0.0);
        r = std::min(r, 1.0);

        g = std::max(g, 0.0);
        g = std::min(g, 1.0);

        b = std::max(b, 0.0);
        b = std::min(b, 1.0);
    }

    double r, g, b;
};

      

        
        
        
      

    

I would also suggest making this constructor explicit, as it is rather convoluted for a scalar to implicitly convert to Color

.

The reason is that this is possibly an update with even about the same amount of code and perhaps not the biggest improvement in readability, because while compiler optimizations can generate faster unallocated code here, min

and max

can often guarantee an efficient implementation. You also express what you are doing in a slightly more direct way.

There is some truth to this somewhat counterintuitive idea that writing higher-level code helps to achieve efficiency, if only because the low-level logic used to implement the high-level function is more efficient than what people repeatedly write differently in their more casual, everyday kind of code. It also helps to direct your codebase towards more central optimization targets.

As a second pass, this may not improve the situation for your specific use cases, but in general I found it useful to represent the color and vector components using an array so that you can access them using loops. This is because if you start out doing a few tricky things with colors like mixing them together, the logic for each color component is non-trivial, but identical across all components, so you don't want to end up writing code like this three times all the time. or always be forced to write the logic for each component in a separate function or something.

So we can do this:

class Color 
{
public:
    explicit Color(double x)
    {
        for (int j=0; j < 3; ++j)
        {
            rgb[j] = x;
            rgb[j] = std::max(rgb[j], 0.0);
            rgb[j] = std::min(rgb[j], 1.0);
        }
    }

    // Bounds-checking assertions in these would also be a nice idea.
    double& operator[](int n) {return rgb[n]};
    double operator[](int n) const {return rgb[n]};

    double& red() {return rgb[0];}
    double red() const {return rgb[0];}

    double& green() {return rgb[1];}
    double green() const {return rgb[1];}

    double& blue() {return rgb[2];}
    double blue() const {return rgb[2];}

    // Somewhat excess fluff, but such methods can be useful when
    // interacting with a low-level C-style API (OpenGL, e.g.) as 
    // opposed to using &color.red() or &color[0].
    double* data() {return rgb;}
    const double* data() const {return rgb;}

private:
    double rgb[3];
};

      

        
        
        
      

    

Finally, as others have pointed out, it is useful here to use the function to freeze values ​​in the range, so the last pass is:

template <class T>
T clamp(T val, T low, T high)
{
    assert(low <= high);
    return std::max(std::min(val, high), low);
}

// New constructor using clamp:
explicit Color(double x)
{
    for (int j=0; j < 3; ++j)
        rgb[j] = clamp(x, 0.0, 1.0);
}