On 20-2-2014 20:25, Martin Owens wrote:
On Thu, 2014-02-20 at 18:58 +0100, Johan Engelen wrote:
- Please don't add functions that use C-strings unnecessarily. Simply rewrite parseDataUri to parseDataUri(const std::string
&uri),
I have a question about this revert. The logic is being copied from image.cpp (where it uses c-strings) and I figured calling std::string( uri ) on a massive embedded image uri would be bad.
So, should I rewrite uri.cpp to only use std::string or accept the string copy that might result in lots of member use?
I don't fully understand your question, but eventually, pretty much everything of Inkscape should use std::string. So it's good not to add any C-string stuff, to make the refactoring easier. I agree that copying a massive data structure, only to discard it after does not make sense. I'm sorry, I didn't think enough about the function's use cases. Perhaps in this case, indeed it is better to use char*, if that is how the uri was stored in the first place. So you'd have to look at who will call parseDataUri and if they have the uri in a std::string or char*. (if so, you could add a brief comment of why you think char* is preferred above std::string)
cheers, Johan