HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Pointer handle - follow-up

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pointerfollowhandle

Problem

This is a followup to:

  • Pointer class/handle



Please review this pointer class.

template
class Ptr {
public:
    Ptr(T* t, int s = 1) {
        sz = s;
        p = new T[sz];
        for (int i = 0; i  old{p};
        operator++();
        return old;
    }

    Ptr& operator--() {
        operator-=(1);
        return *this;
    }

    Ptr operator--(int) {
        Ptr old{p};
        operator--();
        return old;
    }

    ~Ptr() {
        delete[] p;
    }

private:
    T* p;
    int sz;
    int index = 0;

    void check_range(int i) {
        if (i  sz-1) {
            throw std::out_of_range("out of range");
        }
    }

    void check_range() {
        if (p == nullptr) {
            throw std::out_of_range("null pointer");
        }
    }

};

Solution

c++11 Allows constructor chaining:

Ptr(const Ptr& t) {
    sz = t.sz;
    p = new T[sz];
    T* x = t.get();
    for (int i = 0; i < sz; ++i) {
        p[i] = x[i];
    }
}


Can be written as:

Ptr(const Ptr& t): Ptr(t.p, t.sz) {}


Good to see you using the copy and swap idiom.

Ptr& operator=(const Ptr& t) {
    Ptr copy{t};
    std::swap(copy.sz, sz);
    std::swap(copy.p, p);
    return *this;
}


But his can be simlified by making the compiler to the copy.

Ptr& operator=(Ptr copy) {
        //   ^^^^^    Pass by value is generating a copy.       
    std::swap(copy.sz, sz);
    std::swap(copy.p, p);
    return *this;
}


This looks good.

~Ptr() {
    delete[] p;
}


But you ca still move p

void operator+=(int i) {
    check_range(index+i);
    index += i;
    p+= i;               // P has changed
}

void operator-=(int i) {
    check_range(index-i);
    index -= i;
    p -= i;               // P has changed
}


Thus using it with delete after the point it changed is going to cause UB.

Code Snippets

Ptr(const Ptr& t) {
    sz = t.sz;
    p = new T[sz];
    T* x = t.get();
    for (int i = 0; i < sz; ++i) {
        p[i] = x[i];
    }
}
Ptr(const Ptr& t): Ptr(t.p, t.sz) {}
Ptr& operator=(const Ptr& t) {
    Ptr copy{t};
    std::swap(copy.sz, sz);
    std::swap(copy.p, p);
    return *this;
}
Ptr& operator=(Ptr copy) {
        //   ^^^^^    Pass by value is generating a copy.       
    std::swap(copy.sz, sz);
    std::swap(copy.p, p);
    return *this;
}
~Ptr() {
    delete[] p;
}

Context

StackExchange Code Review Q#74918, answer score: 4

Revisions (0)

No revisions yet.