Jump to content
  • Advertisement
Sign in to follow this  
Servant of the Lord

Naming functions that 'operate on' vs 'copy and return'

This topic is 2501 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

What are your views on naming identical functions that do the same thing, with the only difference being whether they operate on the variable passed in (by reference), or operate on a copy of the variable (passed in by const reference) and returning it?

In my current code, I want to have two functions with the same purpose, but different usages. (both functions I have in a "String::" namespace)
I have:
std::string ReplaceAll(const std::string &str, ....);

I also have:
void ReplaceAll(std::string &str, ...);

They do the exact same thing (takes a string to operate on, a string to find, and a string to replace it with), but I use them in different ways.
I have more than just 'ReplaceAll()', I want multiple versions of several different functions. (Since I can always implement one function by calling the other, it doesn't introduce code bloat, and bug-fixes or optimizations to one automatically effect the other).

The first is useful for passing as parameters, such as this type of situation: (Supposing ToLower() is the function we are talking about)
map[ String::ToLower(key) ] = value;

It works well. If it didn't return a value, I'd have to do this:
std::string lowercaseKey = key;
String::ToLower(lowercaseKey);
map[ lowercaseKey ] = value;


That makes code less clear when doing a series of operations, because you have boilerplate lines for every operation of the function, instead of just having each operation be self-explanatory.

However, when I'm operating on a single value, or a series of values, I don't want to go like this:
str = Operation(str);
str = SecondOperation(str);
str = OtherOperation(str);
str = NextOperation(str);


I would much rather do the following, for code readability:

Operation(str);
SecondOperation(str);
OtherOperation(str);
NextOperation(str);



But I can't have two different versions of the same function named the same thing, for two reasons:
1) It'd be confusing to read. Which version am I calling here, version A, or version B?
2) C++ wouldn't allow it, even if I wanted it - the function parameters are too identical so the function names would collide.

So what would you call the two functions to distinguish them?
I was thinking something like,
ReplaceAllOn(str, ...) //Operates 'on' str, and returns void.
ReplaceAllFrom(str, ...) //Operates on a copy of str, and returns the copy.


Another possibility is using pointers for the 'operate on' version of functions, letting the & symbol indicate what's going on.
ReplaceAll(&str, ...)
str = ReplaceAll(str, ...)


However, I prefer using references when I know the variable address won't change during the function, and there's still the possibility of accidentally going:
ReplaceAll(str, ...) //Returning a copy, and having no effect, because you forgot the '&' so it called the wrong function. Hard to spot mistake when scanning code visually.

What I'd really love is a compiler error if I'm using the wrong function, but I can't think of a way to make that happen.
Blah = OperateOn(blah); //Compiler error: Blah = void is invalid. (This error already works)
OperateOnCopyOf(blah); //Compiler error: You're not assigning the return value to anything, you moron! (Sadly, this error my compiler doesn't inform me of)


Thoughts?

Share this post


Link to post
Share on other sites
Advertisement
I would perhaps argue that this should be avoided. There should be one idiomatic usage in the code that is consistent.

That said, I would use something like ReplaceAllOn, ReplaceAllInplace or ReplaceAllMutating. Why? Mutating behavior should be the less preferred version, so should have the more annoying name. But it is similar behavior so should have the same beginning of the function name so it's next to the other one via intellisense/auto-doc and thus aiding in discover-ability.

The non-mutating version would be simply ReplaceAll.

Share this post


Link to post
Share on other sites
Just about the technical side:

2) C++ wouldn't allow it, even if I wanted it - the function parameters are too identical so the function names would collide.[/quote]

C++ allows to overload on const and non-const references.

Share this post


Link to post
Share on other sites
I'd second the "just have one way to do it" sentiment, plus the "if you have to, give the mutating version the less optimal name, such as an [font=courier new,courier,monospace]InPlace[/font] suffix" sentiment.

Share this post


Link to post
Share on other sites
+1 for the _copy suffix. I would argue that it is the most idiomatic "c++" way of doing things, it's the approach taken by both the standard library and boost. As an example, take a look at the std::string member functions, the name of the function is the operation, and they are all mutating:

http://www.cplusplus.com/reference/string/string/replace/

So I would expect the default behaviour of a function to be mutating, unless the name of the function specifies otherwise.

I agree that the convenience of the copy version and the fact that it takes next to no work to implement make it worth the while.

For some inspiration you might want to take a look at the boost string algorithms:

http://www.boost.org/doc/libs/1_48_0/doc/html/string_algo.html

Share this post


Link to post
Share on other sites
The thing std::string's functions having '_copy()' functions is because it's part of a class. My functions are free-floating - there's a difference in expected usage.


Mutating behavior should be the less preferred version


Why should it be less-preferred? I get the benefits of compiler-errors if I mistake it for the copy version ("Warning: Can't assign 'void' to 'std::string'"), but not vice-versa. Plus, it doesn't bother creating unneeded copies that it just discards. It's safer and faster, so why not prefer it?

What are your thoughts about using namespaces for this purpose?

string = String::Copy::ReplaceAll(string);
String::Mutate::ReplaceAll(string);


Or maybe having the mutate version just be String::ReplaceAll() and the copy versions of all the string functions, being the specialized versions, exist in String::Copy::ReplaceAll().

Share this post


Link to post
Share on other sites

The thing std::string's functions having '_copy()' functions is because it's part of a class. My functions are free-floating - there's a difference in expected usage.

You might wanna look at <algorithm> (which I linked to in my first post).

Personally, I'd prefer to have them in the same namespace.

Share this post


Link to post
Share on other sites
For string mutation, you usually can't assume that the storage won't need to change size anyhow, so you probably end up doing a re-allocation internally anyhow. Just return the new string, and if you wish to over-write the old version, assign to it.

Share this post


Link to post
Share on other sites

Why should it be less-preferred? I get the benefits of compiler-errors if I mistake it for the copy version ("Warning: Can't assign 'void' to 'std::string'"), but not vice-versa. Plus, it doesn't bother creating unneeded copies that it just discards. It's safer and faster, so why not prefer it?


That C++ isn't good enough to return errors on unused return values isn't a mark against the practice of favoring mutating functions in general.


It's not safer. At least not in the ways that I value.

Mutating functions tend to cause side effects with other things that reference the object, causing harder to find bugs; for somethings this is expected behavior. For most things that are not literal objects (conceptually) it's not.
Mutating functions are far harder to test in isolation, causing more runtime errors.
Mutating functions are far harder to design with in a concurrent scenario, leading to more awkward code and/or worse bugs.
You can make mutating behavior by combining non-mutating behavior and assignment; you cannot make non-mutating behavior from a mutating function. This gives you more flexibility as things evolve.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!