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

Pointer handle - only allocates in stack

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

Problem

This is a pointer class of mine that I would like to have reviewed. Right now it only allocates in the stack and has no copy or move operations.

template
class Ptr {
public:
    Ptr(T* t, int s = 1) :p{t},sz{s} { }
    T& operator*() {
        check_range();
        return *p;
    }
    T& operator[](int i) {
        check_range(i);
        return p[i];
    }
    Ptr& operator+(int i) {
        index += i;
        check_range(index);
        p+= i;
        return *this;
    }
    Ptr& operator-(int i) {
        index -= i;
        check_range(index);
        p -= i;
        return *this;
    }
    Ptr& operator++() {
        *this = *this+1;
        return *this;
    }
    Ptr operator++(int) {
        Ptr old{p};
        ++(*this);
        return old;
    }
    Ptr& operator--() {
        *this = *this-1;
        return *this;
    }
    Ptr operator--(int) {
        Ptr old{p};
        --(*this);
        return old;
    }
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");
        }
    }
};


How can I make this shorter and less ugly? Is there a better solution than what I did?

Solution

I would expect operator+ and operator- to return a new object (leaving the current one intact.

Ptr& operator+(int i) {
        index += i;
        check_range(index);
        p+= i;
        return *this;
    }
    Ptr& operator-(int i) {
        index -= i;
        check_range(index);
        p -= i;
        return *this;
    }


If I was using normal pointers.

T* x  = getAT();
 x + 1;           // Does not change x
 T* y = x + 1;    // Does not change x (but y has the value of x+1).


Usually you see operator+ defined in terms of operator+=

Ptr& operator+(int i) {
   Ptr result(*this);
   return result+=1;
}


The this = this+1; look very strange.

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


I would have just called the operator+=

Ptr& operator++() {
        return operator+=(1);
    }


Again don't like ++(*this);

Ptr operator++(int) {
        Ptr old{p};
        ++(*this);
        return old;
    }


I would have used:

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


You should strive for the strong exception guarantee (also known as transaction exception guarantee).

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


Though this code looks like it does that. It is usually called in a way that prevents it.

index -= i;
    check_range(index);  // If this throws your object is now in the new state
                         // with index out of range. I would have tried to make sure
                         // that if an exception is thrown then the state of the object
                         // remains unchanged.

    // Change to this:

    check_range(index - i); // If it throws then the object is still
                            // in a consistent unchanged state.
    index -= 1;             // If we get here then change the state.

Code Snippets

Ptr& operator+(int i) {
        index += i;
        check_range(index);
        p+= i;
        return *this;
    }
    Ptr& operator-(int i) {
        index -= i;
        check_range(index);
        p -= i;
        return *this;
    }
T* x  = getAT();
 x + 1;           // Does not change x
 T* y = x + 1;    // Does not change x (but y has the value of x+1).
Ptr& operator+(int i) {
   Ptr result(*this);
   return result+=1;
}
Ptr& operator++() {
        *this = *this+1;
        return *this;
    }
Ptr& operator++() {
        return operator+=(1);
    }

Context

StackExchange Code Review Q#74194, answer score: 7

Revisions (0)

No revisions yet.