Re: [thrift] C++ structure constructorThis is a nice utility patch. My vote would be for a strict ordering by field identifier -- though putting the negatives at the end and counting downwards for them seems fine.
The two examples below (with field identifiers, just opposite order) should definitely produce the same constructor. ----- Original Message ----- From: Eric Anderson To: [email protected] Cc: Alexander Shigin Sent: Wednesday, July 09, 2008 11:48 PM Subject: Re: [thrift] C++ structure constructor (Redirecting to [email protected]) Alexander Shigin writes: > ,AP・,AP嘆２€, 08/07/2008 ,AP2 18:19 -0700, Eric Anderson ,AP?P8Q・5Q?: > > We're mostly using the C++ interface to thrift, and I wanted to be > > able to initialize structures more easily than a whole series of lines > > that set each of the parameters separately. Attached is a patch that > > adds a constructor to the C++ objects that allow you to fully > > initialize an object in a single call. I added two tests for this, > > one in the DebugProtoTest.cpp file, and one in the > > OptionalRequiredTest.cpp. The patch is against the 20080411p1 release. > > I like the idea of the patch, but I have a little nitpick. > > The order of constructor arguments depend on order of appearance > in .thrift file, instead of field identifiers. I mean two structure > produce different interface: > > struct Bonk > { > 2: i32 type > 1: string message, > } > struct Bonk > { > 1: string message, > 2: i32 type > } > I'm fine with either interpretation; if the consensus is that the initializers should be ordered according to the field numbers, I can order them that way. The only weirdness is I would have to order reversed by negative numbers or struct Bonk { i32 type, string message, } would have the initializers in the order message, type because -2 is before -1, and IIRC the auto-assigned numbers count downwards. I'll wait for more consensus on what choice is best before producing another patch. -Eric
