{"action":"created","issue":{"url":"https://api.github.com/repos/gflags/gflags/issues/157","repository_url":"https://api.github.com/repos/gflags/gflags","labels_url":"https://api.github.com/repos/gflags/gflags/issues/157/labels{/name}","comments_url":"https://api.github.com/repos/gflags/gflags/issues/157/comments","events_url":"https://api.github.com/repos/gflags/gflags/issues/157/events","html_url":"https://github.com/gflags/gflags/issues/157","id":166772946,"number":157,"title":"Use StringView\\StringPiece class instead of C API strlen/strcmp for strings.","user":{"login":"dreamer-dead","id":2077170,"avatar_url":"https://avatars.githubusercontent.com/u/2077170?v=3","gravatar_id":"","url":"https://api.github.com/users/dreamer-dead","html_url":"https://github.com/dreamer-dead","followers_url":"https://api.github.com/users/dreamer-dead/followers","following_url":"https://api.github.com/users/dreamer-dead/following{/other_user}","gists_url":"https://api.github.com/users/dreamer-dead/gists{/gist_id}","starred_url":"https://api.github.com/users/dreamer-dead/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dreamer-dead/subscriptions","organizations_url":"https://api.github.com/users/dreamer-dead/orgs","repos_url":"https://api.github.com/users/dreamer-dead/repos","events_url":"https://api.github.com/users/dreamer-dead/events{/privacy}","received_events_url":"https://api.github.com/users/dreamer-dead/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/gflags/gflags/labels/enhancement","name":"enhancement","color":"207de5"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2016-07-21T09:15:15Z","updated_at":"2016-07-22T14:25:29Z","closed_at":null,"body":"It seems, that gflags calls strlen/strcmp many times for strings represented as `char*`.\r\nHow do you think, is it a good idea to implement lightweight wrapper for `char*` that will hold the length of the string, like [std::string_view](http://en.cppreference.com/w/cpp/experimental/basic_string_view) \\ [base::StringPiece](https://developers.google.com/optimization/reference/base/stringpiece/StringPiece/) does?\r\nCached length then can be used to return early when comparing two strings of different length, for example."},"comment":{"url":"https://api.github.com/repos/gflags/gflags/issues/comments/234558083","html_url":"https://github.com/gflags/gflags/issues/157#issuecomment-234558083","issue_url":"https://api.github.com/repos/gflags/gflags/issues/157","id":234558083,"user":{"login":"schuhschuh","id":975451,"avatar_url":"https://avatars.githubusercontent.com/u/975451?v=3","gravatar_id":"","url":"https://api.github.com/users/schuhschuh","html_url":"https://github.com/schuhschuh","followers_url":"https://api.github.com/users/schuhschuh/followers","following_url":"https://api.github.com/users/schuhschuh/following{/other_user}","gists_url":"https://api.github.com/users/schuhschuh/gists{/gist_id}","starred_url":"https://api.github.com/users/schuhschuh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schuhschuh/subscriptions","organizations_url":"https://api.github.com/users/schuhschuh/orgs","repos_url":"https://api.github.com/users/schuhschuh/repos","events_url":"https://api.github.com/users/schuhschuh/events{/privacy}","received_events_url":"https://api.github.com/users/schuhschuh/received_events","type":"User","site_admin":false},"created_at":"2016-07-22T14:25:29Z","updated_at":"2016-07-22T14:25:29Z","body":"Overall, I would think it be better to template `FlagRegisterer` over the flag type, make `FlagValue` an abstract base class and create a subclass templated over the type of the flag value. This would be cleaner than converting back and forth between string and type enumeration. All this is rather internal, so it should not affect the exposed API.\r\n\r\nA change that requires less modification could be to exchange the `const char *type` parameter of the `FlagRegisterer` and `FlagValue` constructors by an enumeration value instead. Then you avoid type name to enum value conversion all together.\r\n\r\nTo not having to expose the `FlagValue::ValueType` enumeration (though I think that wouldn't be dramatic to have in the public header file), we could only template the `FlagRegisterer` and the constructor of each template specialization calls `new FlagValue` with the corresponding `FV_` enum value. We could use type trait such as:\r\n\r\n```c++\r\ntemplate <class T> struct flag_traits\r\n{\r\n  FlagValue::ValueType type();\r\n};\r\n\r\ntemplate <> struct flag_traits<bool>\r\n{\r\n  FlagValue::ValueType type() { return FlagValue::FV_BOOL; }\r\n};\r\n\r\ntemplate <class T>\r\nFlagRegisterer<T>::FlagRegisterer(...)\r\n{\r\n  /* ... */ = new FlagValue(/*...*/, flag_traits<T>::type(), /*...*/);\r\n}\r\n```"}}
{"actor":{"display_login":"schuhschuh"}}