tags:

views:

144

answers:

5

Please refer to rule #41 of C++ Coding Standards or Sutter's Gotw #70, which states that:

Make data members private, except in behaviorless aggregates (C-style structs).

I often would like to to add a simple constructor to these C-style structs, for the sake of convenience. For example:

struct Position
{
    Position(double lat=0.0, double lon=0.0) : latitude(lat), longitude(lon) {}
    double latitude;
    double longitude;
};

void travelTo(Position pos) {...}

main()
{
    travelTo(Position(12.34, 56.78));
}

While making it easier to construct a Position on the fly, the constructor also kindly zero-initializes default Position objects for me.

Maybe I can follow std::pair's example and provide a "makePosition" free function? NRVO should make it as fast as the constructor, right?

Position makePosition(double lat, double lon)
{
    Position p;
    p.latitude = lat;
    p.longitude = lon;
    return p;
}

travelTo(makePosition(12.34, 56.78));

Am I going against the spirit of the "behaviorless aggregate" concept by adding that measly little constructor?

EDIT:

Yes, I was aware of Position p={12.34, 56.78}. But I can't do travelTo({12.34, 56.78}) with pure C structs.

EDIT 2:

For those curious about POD types: http://stackoverflow.com/questions/146452/what-are-pod-types-in-c

FOLLOW-UP: I've asked a follow-up question here that is closely related to this one.

+5  A: 

I routinely provide structs with a constructor, with no problems. However, if the constructor is "non-trivial", then the struct is no longer considered to be a POD type, and there will be restrictions on what you can do with it. If this is an issue for you (it never has been for me), then a make_XXXX function is obviously the way to go.

anon
+2  A: 

Note that without the constructor, the following already does what you need:

int main()
{
  Position pos = { 12.34, 56.78 };
  travelTo(pos);

  Position pos2 = {}; // zero initialises
  travelTo(pos2);
}
James Hopkin
You are limited to where you can use initialisation like this - for example, you can't use it to create a nameless temporary for a function call - something I do very frequently.
anon
True. I mainly wanted to point out that aggregate syntax also provides the zero defaults.
James Hopkin
+2  A: 

Conceptually, it's fine - you aren't going against the spirit of a "behaviorless aggregate".The problem is that the struct is no longer a POD type, so the standard makes fewer guarantees about its behaviour and it can't be stored in a union.

Have you considered this instead?

Position p = {12.34, 56.78};
Joe Gauterin
+5  A: 

We regularly define constructors for our aggregate types, with no adverse effects. In fact the only adverse effects I can think of are that in performance critical situations you cannot avoid default initialisation and that you can't use the type in unions.

The alternatives are the curly brace style of initialisation

Position p = {a,b};

or a free "make" function

Position makePosition(double a, double b)
{
    Position p = {a,b};
    return p;
}

the problem with the former is that you can't use it to instantiate a temporary to pass into a function

void func(Position p)
{
    // ...
}

// func({a,b}) is an error

the latter is fine in this case, but is very slightly more typing for the lazy programmer. The problem with the latter form (a make function) is that it leaves the possibility that you forget to initialise your data structure. Because uninitialised variables leave me feeling rather uncomfortable I prefer to define a constructor for my aggregate types.

The main reason std::make_pair exists is actually not for this reason (std::pair has constructors), but in fact because to call the constructor of a template type you have to pass the template arguments - which is inconvenient:

std::pair<int,int> func()
{
    return std::pair<int,int>(1,2);
}

Finally, in your example, you should at least make your constructor explicit

explicit Position(double lat=0.0, double lon=0.0)

otherwise you allow an implicit cast to a Position from a double

Position p = 0.0;

which might be lead to unintended behaviour. In fact I would define two constructors, one to initialise to zero and one to initialise with two values because the Position construct probably doesn't make much sense without both a latitude and a longitude.

Adam Bowen
Thanks for putting it all together in one answer.
Emile Cormier
Hmm...lots of answers and comments while I typed all that out... :)
Adam Bowen
Oh jeez... sorry Adam. I wasn't being sarcastic or implying you were coalescing previous answers together. :) I had a hard time picking an answer to accept until you posted one that seemed to cover all angles.
Emile Cormier
Hey, no worries I didn't take it that way. It always amazes me just how quick people can be to reply on stackoverflow, one has to wonder how anyone gets any work done!
Adam Bowen
You are not the only one surprised!
Matthieu M.
A: 

Since I posted that question, I've been bitten in the behind for defining constructors for my aggregates. Using my Position example above, GCC complains when I do this:

const Position pos[] = 
{
    {12.34, 56.78},
    {23.45, 67.89},
};

warning: extended initializer lists only available with -std=c++0x or -std=gnu++0x|

Instead I have to do this:

const Position pos[] =
{
    Position(12.34, 56.78),
    Position(23.45, 67.89)
};

With that workaround, I'm worried that in embedded systems, my constant table would not be stored in flash/ROM.

EDIT:

I tried removing the Position constructor, and the pos array has indeed moved from the .bss to the .rodata segment.

Emile Cormier