[
https://issues.apache.org/jira/browse/THRIFT-1947?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13689373#comment-13689373
]
Vitali Lovich commented on THRIFT-1947:
---------------------------------------
* You were using string concatenation to build up dval in the first patch. I
was saying use a stringstream instead. Doesn't look like that's happening now
(except for the default constructor).
* You have an unnecessarily nested if/else clause within an else instead of
them continuing as part of the parent clause
if (cv != NULL) {
dval = render_const_value(out, (*m_iter)->get_name(), t, cv);
} else if (t->is_enum()) {
dval = enum_default_value((t_enum*)t);
} else {
dval = t->is_string() ? "" : "0";
}
* "It isn't possible to call one constructor from another. It has been added in
C++0x." It's released now so it's C++11 :P. I wasn't saying you should call
your own default constructor. I was saying just call the default constructor
of the member. You can just do memberVariable() on primitives & it'll do the
right thing. Similarly for optional strings you can just skip them from the
initializer list all-together as the default constructor is guaranteed to be
called. if you don't feel comfortable & want to remain explicit, then you can
just call the default constructor without "" as using "" may actually allocate
a 1-byte array depending on the implementation of the STL string (which may
actually be more depending on the policies of the string + the default memory
allocator).
* I personally think the constructor initializer is more readable as:
+ if (!dval.empty()) {
+ if (!init_ctor) {
+ init_ctor = true;
+ out << " : ";
+ } else {
+ out << ", " <<;
+ }
+ out << (*m_iter)->get_name() << "(" << dval << ")";
+ }
* Looking at the outputted code, I don't see when the second loop would ever be
hit. Can you provide context?
* The parameters to the constructor should be accepted as const& for
non-trivial types. i.e. thrift structs, lists, maps, sets.
> C++ constructor with initializer list should be generated
> ---------------------------------------------------------
>
> Key: THRIFT-1947
> URL: https://issues.apache.org/jira/browse/THRIFT-1947
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Compiler
> Reporter: Vitali Lovich
> Assignee: Kamil Sałaś
> Attachments: 0001-Thrift-1947.patch, 0001-Thrift-1947_v2.patch,
> 0001-Thrift-1947_v3.patch, v3_test_out.zip
>
>
> It would be really nice if the thrift constructor took in an initializer list
> like the generated Java constructor does for non-optional fields.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
