toSnakeCase: string_view ftw

[anordal]

--- a/src/include/ZividPython/DataModelWrapper.h
+++ b/src/include/ZividPython/DataModelWrapper.h
@@ -154,8 +154,7 @@ namespace ZividPython

                     using MemberType = std::remove_const_t<std::remove_reference_t<decltype(member)>>;

-                    std::string name{ MemberType::name };
-                    name = toSnakeCase(name);
+                    const std::string name = toSnakeCase(MemberType::name);

                     pyClass.def_property(
                         name.c_str(),
--- a/src/include/ZividPython/Wrappers.h
+++ b/src/include/ZividPython/Wrappers.h
@@ -14,13 +14,15 @@

 #include <pybind11/pybind11.h>

+#include <string_view>
+
 namespace ZividPython
 {
     namespace
     {
         /// Inserts underscore on positive case flank and before negative case flank:
         /// ZividSDKVersion -> zivid_sdk_version
-        std::string toSnakeCase(const std::string upperCamelCase)
+        std::string toSnakeCase(std::string_view upperCamelCase)
         {
             if(upperCamelCase.empty())
             {
@@ -29,12 +31,14 @@ namespace ZividPython

             if(!isupper(upperCamelCase[0]))
             {
-                throw std::runtime_error{ "First character of string: '" + upperCamelCase + "' is not capitalized" };
+                std::stringstream msg;
+                msg << "First character of string: '" << upperCamelCase << "' is not capitalized";
+                throw std::runtime_error{ msg.str() };
             }
             std::stringstream ss;
             ss << char(tolower(upperCamelCase[0]));

-            for(auto i = 1; i < upperCamelCase.size(); ++i)
+            for(size_t i = 1; i < upperCamelCase.size(); ++i)
             {
                 if(isupper(upperCamelCase[i]))
                 {
@@ -96,8 +100,8 @@ namespace ZividPython
                                   const WrapFunction &wrapFunction,
                                   const char *nonLowercaseName)
     {
-        std::string name{ nonLowercaseName };
-        auto submodule = dest.def_submodule(toSnakeCase(name).c_str());
+        const std::string name = toSnakeCase(nonLowercaseName);
+        auto submodule = dest.def_submodule(name.c_str());
         wrapFunction(submodule);
     }
 } // namespace ZividPython

[eskaur]

Should this issue be closed, @anordal ?

[anordal]

It doesn't look like this is applied. Did you mean to, @trym-b?

I think there is actually one important fix in here: To not call c_str() on a temporary: toSnakeCase(name).c_str().

[nedrebo]

I agree with @anordal, that is a bug. Calling the function is actually fine, but storing the pointer it returns is not.

[trym-b]

This task is currently not on my todo list, so it is unlikely that this will be prioritized. Feel free to create a PR.

[anordal]

Solved by #133