C++ circular array queue setCapacity

Question

I am working on an assignment that involves me writing a template class for a queue. It uses a dynamically allocated array. I am having trouble with the copying of the array. This is the prompt.

aQueue.setCapacity(newCapacity), that changes the capacity of aQueue to newCapacity. (This is much trickier than Stack::setCapacity(): if newCapacity is zero or < getSize(), setCapacity() should throw an exception; otherwise, it should allocate a new array with the new capacity, copy the values from the old array into the new array, and deallocate the old array.) To help you see all the things that can go wrong, you should make certain your method passes all the tests in this test method before working on the rest of the project.

Instance variables are:

unsigned mySize;       // number of items I contain
unsigned myCapacity;   // how many items I can store
unsigned myFirst;      // index of oldest item (if any)
unsigned myLast;       // index of next available spot for append (if any)
Item*    myArray;      // dynamic array of items

This is what I have so far:

template <class Item>
void ArrayQueue<Item>::setCapacity(unsigned newCapacity) {
    if (newCapacity == 0 || newCapacity < mySize) {
        throw QueueException("setCapacity()","newCapacity is too small");
    } else {
        Item * newArray = new Item[newCapacity];
        unsigned y = myFirst;
        for (unsigned x = 0; x < mySize; x++) {
            newArray[y] = myArray[x];
            y++;
        }
        delete [] myArray;
        myArray = newArray;
        myCapacity = newCapacity;
        myLast = y;
    }
}

These are the methods for getFirst() and getLast():

// Method returns first Item in ArrayQueue
template <class Item>
unsigned ArrayQueue<Item>::getFirst() const {
    if (this->isEmpty()) {
        throw EmptyQueueException("getFirst()");
    } else {
        return myArray[myFirst];
    }
}

// Method returns last Item in ArrayQueue
template <class Item>
unsigned ArrayQueue<Item>::getLast() const {
    if (this->isEmpty()) {
        throw EmptyQueueException("getLast()");
    } else {
        return myArray[(myLast - 1 + myCapacity) % myCapacity];
    }
}

I have been working on this for hours so any help would be greatly appreciated.

Manually "execute" your copying code. This will lead you to your main issue. Apart from that: 1) `myMember` names for members are not idiomatic C++. 2) You're managing dynamic memory manually, don't forget the rule of 3/5. 3) You shouldn't be managing dynamic memory manually, try to use e.g. a `std::vector` as your backing memory. — Daniel Jour, Apr 09 '17 at 21:48
I would like to use a vector but for this assignment I am required to use an array... — Patrick Maynard, Apr 09 '17 at 21:51
How about using y % newCapacity as the index into newAarray in the for loop? — Shiping, Apr 09 '17 at 22:03
That helps thanks. However, in my test method I am still failing assertions for myFirst and myLast. I am not exactly sure how to set those variables properly. This is my test method: — Patrick Maynard, Apr 09 '17 at 22:12
//test setCapacity() on a queue that has items starting //in the middle of the array, not wrapping around ArrayQueue q4(5); for (int i = 0; i < 5; i++){ q4.append(i+1); } q4.remove(); q4.remove(); assert( q4.getCapacity() == 5 ); assert( q4.getSize() == 3 ); assert( !q4.isEmpty() ); assert( !q4.isFull() ); assert( q4.getFirst() == 3 ); assert( q4.getLast() == 5 ); — Patrick Maynard, Apr 09 '17 at 22:13
//increase the capacity q4.setCapacity(10); assert( q4.getCapacity() == 10 ); assert( q4.getSize() == 3 ); assert( !q4.isEmpty() ); assert( !q4.isFull() ); assert( q4.getFirst() == 3 ); assert( q4.getLast() == 5 ); cout << " 3a" << flush; — Patrick Maynard, Apr 09 '17 at 22:13
You might want the assignment the other way round: `newArray[x] = myArray[(y % myCapacity];` -- afterwards, you set `myFirst = 0;` and `myLast = mySize;` — Aconcagua, Apr 09 '17 at 22:22
Oh, and 'last' is not really the best term - myLast does not point to the last element, but to the subsequent (dummy) one. You might perhaps prefer firstOccupied and firstFree instead - or you use the terms the STL uses, too: begin/end... — Aconcagua, Apr 09 '17 at 22:27
wow thanks so much it works! If you can let me know of a way I could make it more efficient. — Patrick Maynard, Apr 09 '17 at 22:29
@More efficient? There won't be much you can still do. You might gain minimally using a pointer for the target (saves you one addition in the loop), and you could try to replace the modulo calculation *within* the loop by catching the corner case (`myLast < myFirst`) outside the loop (and have two loops). That will, however, only be a gain on large data, but results in code much more complicated, so I recommend doing so only if you prove the current iteration being a bottle neck... — Aconcagua, Apr 09 '17 at 22:42

score 0 · Accepted Answer · answered Apr 09 '17 at 22:46

Credit goes to: Aconcagua

    for (unsigned x = 0; x < mySize; x++) {
        newArray[x] = myArray[(y % myCapacity)];
        y++;
    }
    delete [] myArray;
    myArray = newArray;
    myCapacity = newCapacity;
    myFirst = 0;
    myLast = mySize;

By setting myFirst to 0 and myLast to mySize this properly allocates the indexes correctly. In addition to this when copying the array you most set newArray[x] to myArray[(y % myCapacity)].

C++ circular array queue setCapacity

1 Answers1