Create a copy or use reference for parameter?

Question

My problem is that I want to return a common substring of two strings s1, s2. Apparently, s1 and s2 are symmetric.

string shortest_common( const string& s1, const string& s2 ) {

}

There are three possible solutions to this problem that I came up with:

Either make a copy of s1 and s2
Or swap them, which means I have to sacrifice their const-ness
Or worst, duplicate code!

I personally prefer the first case, since intent is to find the shortest-common string not changing s1 or s2. So my question is: Which option is ideal in this case?

Thanks,
Chan

They're symmetrical meaning palindromic? As in, 'abba' and 'cabbac' are valid input and 'abba' would be the result? — Matt K, Jan 28 '11 at 18:11
Wait, shortest common string? That's not even itneresting as the answer is always "" :) — Matt K, Jan 28 '11 at 18:12
@Matt Kane: It's a modified version, not a traditionally palindromic. It has many sub-cases that I have to handle. — roxrook, Jan 28 '11 at 18:14
Matt is correct. You would need it to be shortest common substring with a minimum length. Anything less than 2 would give you a lot of results for any non-trivial pair of strings. — Zac Howland, Jan 28 '11 at 18:26

wheaties · Accepted Answer · 2011-01-28T18:18:50.430

2

I would opt to go with the signature that you've displayed. If you're finding a common substring then you don't want to have side-effects. That's not what people think of when they would call your function. I don't expect a function called "add_two_numbers" to modify one of the numbers and return a value.

edited Jan 28 '11 at 18:18

answered Jan 28 '11 at 18:13

wheaties

35,646
15
94
131

Your thought matched mine ;). I will go for this option. – roxrook Jan 28 '11 at 18:59

score 0 · Answer 2 · answered Jan 28 '11 at 18:18

0

You could use recursion to swap the meanings of parameters without changing the objects themselves in any way.

answered Jan 28 '11 at 18:18

Ben Voigt

277,958
43
419
720

sbi · Answer 3 · 2011-01-28T19:25:16.610

0

I'm a bit confused as to what your actual question is, so I'll judge by the title.

Do both:

void shortest_common( string& s1, const string& s2 )
{
  // real algorithm changing s1
}

inline string shortest_common( string s1, const string& s2 )
{
  shortest_common( s1, s2 );
  return s1;
}

edited Jan 28 '11 at 19:25

answered Jan 28 '11 at 18:20

sbi

219,715
46
258
445

@sbi: If you're just going to copy the parameter, isn't it better to use pass by value to make the copy? I thought that opens up some additional optimizations (and of course in C++0x, the "real implementation would use an rvalue-reference, and the forwarder would use `std::move`) – Ben Voigt Jan 28 '11 at 19:13
@Ben: I'm using VC++ 2010. According to what you mentioned, is passing `std::string` by copy even better? – roxrook Jan 28 '11 at 23:33
@Chan: Usually you pass class object per `const` reference, __unless__ you need a copy of the parameter within the function anyway. (See [How to pass objects to functions in C++?](http://stackoverflow.com/questions/2139224/how-to-pass-objects-to-functions-in-c/2139254#2139254)) The reason is that the compiler might be able to elide the copying when you pass an rvalue (a temporary object) as this parameter. – sbi Jan 29 '11 at 00:27
@Chan: In VC2010, the first function (the real implementation) would be `string shortest_common( string&& s1, const string& s2 )`, and the forwarder than doesn't eat its first argument `inline string shortest_common( string s1, const string& s2 ) { return shortest_common(std::move(s1), s2); }`. – Ben Voigt Jan 29 '11 at 01:35

Create a copy or use reference for parameter?

3 Answers3