StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>You have already gotten a few good answers concerning the errors in you <code>myString::swap()</code> function. Yet, I'd like to add another one. There's some many things wrong with that function, I first found it hard to think of where to begin. But then I realized that you fail on some fundamental issue which I'd like to point out: </p> <p>As a convention, a function called <code>swap</code> is expected to perform its task </p> <ol> <li>in O(1)</li> <li>without ever throwing an exception. </li> </ol> <p>(Yes, I know, there are exceptions: <code>std::tr1::array<>::swap()</code>. But those should be very well justified.) Your implementation fails on both accounts. It is O(n) (<code>strcpy</code>) and might throw an exception (<code>new</code>) -- and it does so unnecessarily and without justification. </p> <p>When you look at <code>myString</code>, you'll see that it only has two pieces of member data, which both are of built-in type. That means swapping two objects of this class is <em>really</em> simple to do while keeping to the conventions mentioned above: just swap the member data. That's as simple as calling <code>std::swap</code> on them: </p> <pre><code>void myString::swap(myString &from) { std::swap(this->stringPtr,from.stringPtr); std::swap(this->stringLen,from.stringLen); } </code></pre> <p>This is will never fail (swapping two pointers and two integers cannot fail), executes in O(1), is very easy to understand (well, once you get a grip on that swapping, anyway; it is an idiomatic form of implementing a class-specific <code>swap</code> function), and consists of two lines of code calling something well-tested in the standard library instead of 8 lines of code doing error-prone (and, in your case, erroneous) manual memory management. </p> <p>Note 1: Once you've done this, you should specialize <code>std::swap</code> to call your implementation for your class: </p> <pre><code>namespace std { // only allowed for specializing function templates in the std lib template<> inline void std::swap<myString>(myString& lhs, myString& rhs) { lhs.swap(rhs); } </code></pre> <p>Note 2: The best (simple, exception-safe, and self-assignment-safe) way to implement assignment for your class is to use its <code>swap</code>: </p> <pre><code>myString& myString::operator=(const myString& rhs) { myString tmp(rhs); // invoke copy ctor this->swap(tmp); // steal data from temp and leave it with our own old data return *this; } // tmp will automatically be destroyed and takes our old data with it </code></pre>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload