Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/359#discussion_r195782604
--- Diff: libminifi/include/core/Property.h ---
@@ -65,15 +65,15 @@ class Property {
dependent_properties_(std::move(dependent_properties)),
exclusive_of_properties_(std::move(exclusive_of_properties)),
is_collection_(false) {
- values_.push_back(std::string(value.c_str()));
--- End diff --
I'm curious why this existed the way it did, but my guess is it's
overriding the C++11 pointer copy to take ownership of that pointer. We have a
named variable already and std::string has a move constructor so most of this
is implicitly unnecessary [1], but I'm more concerned with the impetus for the
ptr and std::string. That may have been someone ( could be me ) taking
advantage of the ptr copy. I'll do al little research into that to validate,
but in general push_back of already constructed objects with a move constructor
should be fine; however, even in the case above I would have preferred
emplacing the pointer than pushing since we're constructing an object to be
appended, so I'll take a look at the string specs before approving to see if I
can gather what the original intent was, here. My be a micro-optimization for
pointer copies...but I think the move constructor does that for for SSOs, which
many of these may be.
[1] http://ptspts.blogspot.com/2017/02/fast-vector-append.html
---
