2

I was designing an RGBA class, where four parameters needed to be passed to the class's constructor, to instantiate the class. The constructor looks like the one below:

RGBA(int red = 0, int green = 0, int blue = 0, int alpha = 255)
{
    auto valid_color = [](int param) {return (param >= 0 && param <= 255) ? param : 0; };
    m_red = valid_color(red);
    m_green = valid_color(green);
    m_blue = valid_color(blue);
    m_alpha = valid_color(alpha);
}

As you can see above, I have used a lambda for each parameter to verify the passed paramater. This lead me to wonder, what advantages could such a lambda have over a function, such as this (declared private in the the interface):

int valid_color(int param)
{
    return (param >= 0 && param <= 255) ? param : 0;
}

So my options are this:

  1. Keep using the lambda, and let the code be.
  2. Instead of a lambda, declare an actual function, like the one above, and use that.
  3. Just write it out completely for everything. (Slightly tedious and definitelt error-prone).

Which option seems the best, and why?

Arnav Borborah
  • 11,357
  • 8
  • 43
  • 88
  • I would do `inline int validate_color()`. The use of a lambda in this case seem quite unnecessary and likely to penalize performace. – Emerald Weapon Aug 27 '16 at 15:26
  • 1
    @EmeraldWeapon Eh... lambda vs. not isn't really the factor there; if you want performance, don't validate the parameters and/or provide a non-validating construction path for internal use and/or don't repeatedly construct zillions of validated `RGBA`s in inner loops. And the obligatory: make sure that's where your performance hit is in the first place. – Jason C Aug 27 '16 at 15:47
  • I always assumed that lambdas suffer from performance penalty compared to regular functions, this is not the case apparently as [this](http://stackoverflow.com/questions/15930755/are-lambda-functions-in-c-inline) post explains. Sorry, I take back my previous comment on performance, but still think this use of lambda is uncalled for. – Emerald Weapon Aug 27 '16 at 16:06
  • As far as the object code generated by an optimizing compiler, a lambda is indistinguishable from a regular function. Lambdas are basically just syntactic sugar: two different ways of writing the same thing. It's just that lambdas are sometimes more convenient and more readable for the programmer than having to write a separate function. The compiler doesn't care. I agree that in this case the lambda is unnecessary. The validation function should be an inline function in an unnamed namespace inside the code file. @emerald – Cody Gray - on strike Aug 28 '16 at 05:58
  • Why was this just downvoted? – Arnav Borborah Aug 31 '16 at 02:02

3 Answers3

5

Well, if you use a lambda, you can't do this:

RGBA(int red = 0, int green = 0, int blue = 0, int alpha = 255)
    : m_red{valid_color(red)}
    , m_green{valid_color(green)}
    , m_blue{valid_color(blue)}
    , m_alpha{valid_color(alpha)}
{}

This uses a constructor initialization list to initialize the members, rather than doing it within the constructor body.

Also, your member function should be a static member function. Or an internal global/namespace-scoped function. It doesn't use this.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
3

If there is a chance that there are going to be other functions that need to call validate_color, then make it a function. Otherwise, it doesn't really matter.

evan
  • 1,463
  • 1
  • 10
  • 13
  • Other functions will call it, but I feel uneasy thinking that a constructor may call a function. – Arnav Borborah Aug 27 '16 at 15:24
  • 2
    @ArnavBorborah Don't feel uneasy. There's zero reason for constructors to not call other functions. Just maintain awareness of what things have been initialized / not initialized yet and various function's side-effects. You could always make `validate_color` a `static` member if it helps you sleep (and it makes sense anyways if `validate_color` doesn't require a specific `RGBA` instance to do its job). – Jason C Aug 27 '16 at 15:25
  • @JasonC just out of curiosity, if the constructor calls a function, which that then throws an uncaught exception, then what happens? Does the object get constructed? (This doesn't apply to me, since I have the `no-throw` guarantee right now.) – Arnav Borborah Aug 27 '16 at 15:27
  • @ArnavBorborah The exception is thrown from the constructor at that point, and the object is not constructed and the destructor is not called. But you are responsible for cleaning up any e.g. dynamic memory allocations that you did in the constructor prior to that point. In your `RGBA` example as-is you would require no special treatment if `validate_color` threw an exception. Actually it wouldn't be unusual to intentionally have `validate_color` throw a parameter validation error that gets thrown back out of the constructor, that's often a design choice and can be very convenient. – Jason C Aug 27 '16 at 15:29
2

Your example is simple enough that both solutions have equal readability and clarity, and there is no specific advantage one way or the other.

However, if you have to validate parameters in other contexts besides that constructor, and you start duplicating that lambda function everywhere, then you start to make a mess unnecessarily (and, for example, you run into trouble if you have to change your validation rules). If that's a possibility or that starts to happen, that's a good case for making it a member function instead, even a static one (since it doesn't actually need this to do its job).

By the way:

  1. Just write it out completely for everything. (Slightly tedious and definitelt error-prone).

Yeah, don't do that. As you've already identified, it's slightly tedious and definitely error-prone, and error-prone or not, <insert deity here> knows there's already enough tedium in software development, don't punish yourself.

Jason C
  • 38,729
  • 14
  • 126
  • 182