"Tony Johansson" <johansson.andersson@telia.com> wrote in message
news:5y2Oe.32105$d5.186616@newsb.telia.net...
: This class template and main works perfectly fine. But could be
better.
....
: Now to my question.
: In this class template Handle I have one assignment operator and one
copy
: constructor.
: This code for assignement operator and cpy-ctor. I thing the order
is wrong
: I must have statement
: ref_count = h.ref_count; after
: (*h.ref_count)++;
: Now it's before so that's wrong order.
: It's the same order mistake in both assignment operator and the
cpy-ctor.
: Do you agree with me.?
I do not think that this is a problem in this case. The
only thing to be aware of is that, in either case, the
behavior will be undefined in a multi-threaded program
(an atomic operation or a lock would be needed then).
: Below can you find the complete class template Handle and some parts
of
: main.
:
: Handle(const Handle& h) //copy constructor
: {
: body = h.body;
: ref_count = h.ref_count;
: (*ref_count)++;
: }
This is ok.
: const Handle& operator=(const Handle& h) //assignment operator
: {
: if (this != &h)
: {
: (*ref_count)--;
: if (!*ref_count)
: deleteAll();
: ref_count = h.ref_count;
: body = h.body;
: (*h.ref_count)++;
: }
: return *this;
: }
Ok as well. Note that the self-assignment test
becomes unnecessary if you order the operations
properly. The whole function can be written as:
++*h.ref_count;
if( 0 == --*ref_count ) deleteAll();
body = h.body;
ref_count = h.ref_count;
: //Tony
:
:
: template<class T>
: class Handle
: {
: public:
: Handle()
: {
: body = new T(0);
Note that whis will fail if T cannot be initialized
with 0 - making Handle less generic than needed.
You could write instead:
body = new T( T() ); // explicit call to dflt-ctr
// will initialize int and any
// built-in type to zero.
: ref_count = new int(1);
: }
:
: Handle(T* body_ptr) //Constructor
It is safer to make this constructor explicit:
explicit Handle(T* body_ptr)
(this will help avoid unexpected conversions).
: {
NB: it would probably be good practice to test
that body_ptr is not null (assert or throw), as
the rest of the code assumes/implies it never is.
: body = body_ptr;
: cout << "Body adressen = " << body << endl;
: ref_count = new int(1);
: }
:
: ~Handle() //Destructor
: {
: (*ref_count)--;
: if (!*ref_count)
: deleteAll();
: }
:
: T operator*() const { return *body; }
Usually the following function signature would
be used instead:
const T& operator*() const { return *body; }
If T is a complex class (as expected if a Handle
class is needed), this may avoid unnecessary
object copies.
: T& operator*()
: {
: if (*ref_count > 1)
: {
: (*ref_count)--;
: body = new T(*body);
: ref_count = new int(1);
: }
: return *body;
: }
Ok, so this implements copy-on-write (COW).
This has many caveats for the user, but
this function looks ok.
: T* operator->()
: {
: if (*ref_count > 1)
: {
: (*ref_count)--;
: body = new T(*body);
: ref_count = new int(1);
: }
: return body;
: }
:
: T* operator->() const { return body; }
The returned type should be const for consistency:
T const* operator() const { return body; }
: bool operator==(const T& h) { return *body == h; }
: bool operator==(const Handle& h) { return *body == *h.body; }
NB: these are best implemented as 'friend' operators. E.g.:
friend bool operator==(const Handle& a, const Handle& b)
{ return *a.body = *b.body; }
This ensures that x==y and y==x are always equivalent,
in particular if implicit conversions are involved.
: Handle(const Handle& h)
: {
: body = h.body;
: ref_count = h.ref_count;
: (*ref_count)++;
: }
:
: int getRefCount() { return *ref_count; }
This probably can be: ^const
: void operator()(Handle& h) { cout << *h.body << endl; }
Hum... unusual semantics.
: const Handle& operator=(const Handle& h)
: {
: if (this != &h)
: {
: (*ref_count)--;
: if (!*ref_count)
: deleteAll();
: ref_count = h.ref_count;
: body = h.body;
: (*h.ref_count)++;
: }
: return *this;
: }
:
: private:
: T* body;
: int* ref_count;
:
: void deleteAll()
: {
: delete body;
: body = NULL;
: delete ref_count;
: ref_count = NULL;
: }
: };
:
: void addNodeToSTL(list<handle_t>* myList)
NB: it would be more appropriate to use a reference:
void addNodeToSTL(list<handle_t>& myList)
: {
: handle_t myh(new Integer(getValue("Ange vardet som du vill lagga
in i
: listan/listorna: ")));
: myList->push_front(myh);
: }
: typedef Handle<Integer> handle_t;
:
: int main()
: {
: list<handle_t>* myList1 = new list<handle_t>;
: list<handle_t>* myList2 = new list<handle_t>;
: do
: {
: switch (huvudMeny())
:
: {
: case 0: return 0;
: case 1: addNodeToSTL(myList1);
: break;
: case 2: displaySTL(myList1, myList2);
: break;
: case 3: deleteSelectedValue(myList1);
: break;
: case 4: deleteAll(myList1);
: break;
: case 5: findSelectedValue(myList1);
: break;
: case 6: testOperatorStar(myList1);
: break;
: case 7: testOperatorArrow(myList1);
: break;
: case 8: copyList(myList1, myList2);
: break;
: }
: } while(1);
: return 0;
: }
:
: //Many thanks
:
: //Tony
I hope this helps,
Ivan
--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact
form
Brainbench MVP for C++ <>
http://www.brainbench.com 
