0

I'm trying to implement a SmartPointer template Class. Basically shared_ptr, when i'm right.

#include <iostream>

using namespace std;

template <typename T>
class SmartPointer
{
    T *_object;
    int *_counter;

    void removeOne() {
            *_counter -= 1;

        if (*_counter == 0) {
            delete _object;

        }
    }

public:
    SmartPointer(T *obj) {
        cout << "Konstruktor SmartPointer \n";
        _object = obj;
        *_counter++;
    }

    SmartPointer(SmartPointer& sp) {
        cout << "Kopier-Konstruktor SmartPointer \n";
        _object = sp._object;
        sp._counter += 1;
        _counter = sp._counter;
    }

    ~SmartPointer() {
        cout << "Destruktor SmartPointer \n";
        removeOne();
    }

    SmartPointer& operator=(SmartPointer& sp) {
        cout << "Zuweisungsoperator SmartPointer \n";

        if (this == &sp) {
            return *this;
        }
        if (*_counter > 0) {
            removeOne();
            sp._object = _object;
            sp._counter += 1;

            return *this;
        }
    }

    int getCounter() {
        return *_counter;
     }

    T* operator*() {
        return _object;
    }
};

template <typename T>
int fkt(SmartPointer<T> x) {
    cout << "fkt:value = " << *x << endl;
    cout << "fkt:count = " << x.getCounter() << endl;
    return x.getCounter();
}

int main() {

    SmartPointer<double> sp(new double(7.5));
    fkt(sp);

    return 0;
}

My Problem is that get an read access error for the getCounter function and that the * operator just returns the address. How can i make it work? I tried to use the & operator, but got i wrong.

Glad for every help!

Truut
  • 83
  • 1
  • 11
  • 2
    1) Did you try stepping through your code with a debugger? 2) "_I tried to use the & operator, but got i wrong._" One cannot learn C++ by guessing. Consider learning from a [good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). 3) Your `operator=` exhibits undefined behavior, due to not returning in all code paths. – Algirdas Preidžius Sep 26 '18 at 15:21
  • 4
    Even if `*_counter++;` in your first constructor *wasn't* referring to an *indeterminate* location (a bug in itself), the syntax is wrong if you want to increment the pointed-to value. That statement post-increments the pointer; not the value. – WhozCraig Sep 26 '18 at 15:22
  • yeah i used the Debugger and got the read access error. The smartPointer got the right value, but i dont know how to return it instead of the adress. – Truut Sep 26 '18 at 15:23
  • 4
    Where do you initialize `_counter` to actually point to something? – NathanOliver Sep 26 '18 at 15:24
  • I think you keep incrementing the pointer rather than the *pointed to* object. `sp._counter += 1` should be `*sp._counter += 1` etc... – Galik Sep 26 '18 at 15:27
  • When i try to initialize _counter with *_counter = 1 in the Constructur i get an read access error again. – Truut Sep 26 '18 at 15:30
  • That's because `_counter` is a pointer that was never set to some determinate address (see my first comment, first sentence, which explicitly mentions this as a bug). Smart pointers have a shared "data" block that at a *minimum* track the reference count. You have a pointer ready to point to that block, but never bothered to initialize it to something worth pointing to. – WhozCraig Sep 26 '18 at 15:35
  • same in different words: `int *_counter;` is just a pointer, but in your code there is no counter where this pointer would point to. You need to create that counter and destroy it when no longer needed. If you werent trying to implement a smart_pointer I would suggest to use a smart pointer instead of a raw one for `counter` ;) – 463035818_is_not_an_ai Sep 26 '18 at 15:38
  • ...why is `counter` a pointer at all? If you want to count up and down, you don't need a pointer. You just need an integer. – Max Langhof Sep 26 '18 at 17:03

1 Answers1

1

Well this sticks out:

SmartPointer(T *obj) {
    cout << "Konstruktor SmartPointer \n";
    _object = obj;
    *_counter++;      // I don't see _counter being initialized.
                      // So here you are incrementing the content of some
                      // random memory location.
}

This looks wrong:

SmartPointer& operator=(SmartPointer& sp) {
    ....
    if (*_counter > 0) {
        removeOne();
        sp._object = _object;
        sp._counter += 1;

        // Hold on you have not updated this object.
        //
        // You have made the other smart pointer point at your object
        // (which oucld have been deleted) but keep the reference
        // count for the old object and incremented it!!!!
        return *this;
    }
}

I wrote something about smart pointer. I think you definately need to read.

Smart-Pointer - Unique Pointer
Smart-Pointer - Shared Pointer
Smart-Pointer - Constructors

Martin York
  • 257,169
  • 86
  • 333
  • 562