Prev: Small grammar addition for c++0x. Why not?
Next: More proposals for c++ modules.
From: Mathias Gaunard on 28 Mar 2008 20:14
On Mar 28, 6:13 pm, mpho <tjab...(a)gmail.com> wrote:

> To my earlier question in this thread, a (simpler :-) solution that
> worked without throwing all sorts of exceptions is:
>
> T& T::operator=(const T& other){
>
> if (this == &other) return *this;
> else {
> type *p = new type[s] //s -> size of array pointed to by
> 'pointer'
> std::copy(other.pointer, other.pointer + other.s, p);
> s = other.s;
> delete [] pointer;
> pointer = p;
> //similarly for any other pointers.
> }
> return *this;
>
> }

Since you're using something like this, it looks like you should be
using std::vector<type> instead of your type* member.



--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Daniel Krügler on 30 Mar 2008 01:18
On 28 Mrz., 18:13, mpho <tjab...(a)gmail.com> wrote:
> To my earlier question in this thread, a (simpler :-) solution that
> worked without throwing all sorts of exceptions is:

How did you come to the conclusion that your code below
might never fail? Since you are using new[], this
operation might fail as well.

> T& T::operator=(const T& other){
>
> if (this == &other) return *this;
> else {
> type *p = new type[s] //s -> size of array pointed to by
> 'pointer'
> std::copy(other.pointer, other.pointer + other.s, p);
> s = other.s;
> delete [] pointer;
> pointer = p;
> //similarly for any other pointers.
> }
> return *this;
>
> }
>
> See "Programming with Exceptions" by Bjarne Stroustrup.

Your code snippet does not use a better/safer strategy as
any of the answers you got so far. In contrast, this code
will leak memory, if the copy c'tor of type can throw.

As recommended by Mathias Gaunard, you should probably use
std::vector<type> instead. Most implementations I know of
can even better optimize your code as your hand-crafted
vector via new[]/delete[] will do - *and* will be exception-safe.

Greetings from Bremen,

Daniel Kr�gler


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Ulrich Eckhardt on 9 Apr 2008 08:56
mpho wrote:
> To my earlier question in this thread, a (simpler :-) solution that
> worked without throwing all sorts of exceptions is:
>
> T& T::operator=(const T& other){
> if (this == &other) return *this;
> else {
> type *p = new type[s] //s -> size of array pointed to by 'pointer'
> std::copy(other.pointer, other.pointer + other.s, p);
> s = other.s;
> delete [] pointer;
> pointer = p;
> //similarly for any other pointers.
> }
> return *this;
> }

First thing I notice is the structure:

if(...) {
return *this;
} else {
...
}
return *this;

What I mean is that
1. There are two 'return *this;', which could be merged:

if(!...) {
...
}
return *this;

2. There is no need for an 'else', when the 'if' branch already _always_
returns:

if(...)
return *this;
...
return *this;


Secondly, I noticed that you are allocating 'this->s' elements and then copy
over 'other.s' elements, those numbers don't necessarily match. Note that
using std::vector would have avoided that. Further, using std::vector would
even have been more efficient, because it might have not allocated
anything, when there was already enough storage. Also, your way
default-constructs the elements and then assigns to them, which is
potentially less performant.

Lastly, your initial statement is wrong, i.e. that this works without
throwing any exceptions. Of course, new[] can throw just as std::copy() can
throw if the assignment operators of the elements throw.

Summary: Use std::vector!

Uli


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

 | 
Pages: 1
Prev: Small grammar addition for c++0x. Why not?
Next: More proposals for c++ modules.