CODEWITHSUNDEEP
c++
Emile Cormier
Emile Cormier

Reputation: 29219

Ok to provide constructor for behaviorless aggregates (bundle-o-data) in C++?

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: What are POD types in C++?

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

Upvotes: 5

Views: 261

Answers (5)

Emile Cormier
Emile Cormier

Reputation: 29219

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.

Upvotes: 0

Adam Bowen
Adam Bowen

Reputation: 11230

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.

Upvotes: 7

JoeG
JoeG

Reputation: 13192

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};

Upvotes: 2

James Hopkin
James Hopkin

Reputation: 13973

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);
}

Upvotes: 2

anon
anon

Reputation:

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.

Upvotes: 5

Related Questions