pnoltes commented on PR #447: URL: https://github.com/apache/celix/pull/447#issuecomment-1330802073
> > > Generally LGTM. > > > I suggest reconsidering API modification using universal reference, which may requires less code than the current approach. However, I'm by no way an C++ expert, and thus might be wrong. > > > > > > I could be wrong, but a universal reference for `const std::string&` and `std::string_view` won't work. For `const std::string&` the `c_str()` method must be used to retrieve the underlining `const char*` and for `std::string_view` the `data()` method must be used to do the same. I think i will work if we construct a `std::string` from the input, but that means unnecessary copies. > > I agree that this requires quite some code to achieve, but I am ok with this as long as we can test this. > > Hmm looking at https://en.cppreference.com/w/cpp/string/basic_string/data, it seems that since C++11 `data()` and `c_str()` should have the same result. I was not aware of this. Maybe this can be solved by using template for string arguments. I will look into this. Nevermind that conflicts with `const char*` template infer. For now I will keep this if c++17 switch. I also prefer to use literal string types instead of templates for readability/usability. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
