Assignment operator copying everything except for the free space in the array

Question

I've been playing around with dynamic allocation and queue recently. Keep in mind that I am fairly new to data structures and I’ve been teaching myself by watching YouTube tutorials and reading some tutorials from websites so forgive my ignorance if I don’t know every single thing there is to know about this subject. However, while playing around with the implementation of an assignment operator for my queue, I seem to have made a mistake somewhere in its code. You see whenever I try and use the assignment operator

i.e.

Queue names2;
names2 = names;

the assignment operator copies everything already in the queue except the empty elements of the array. Am I implementing my copy constructor or my assignment operator wrong that is causing it to not add the empty unused capacity to the newly created queue?

main.cpp

  #include "Queue.h";

int main()
{
    Queue names;
    names.enqueue("A");
    names.enqueue("B");
    names.enqueue("C");
    names.dequeue();
    names.enqueue("D");
    names.showFullQueue();
    Queue names2;
    names2 = names;
    names2.showFullQueue();
    names2.enqueue("Tom");
    names2.enqueue("Alice");
    names2.showFullQueue();

    return 0;
}

Queue.h

#ifndef Queue_H
#define Queue_H

#include <iostream>
#include <cstring>
#include <algorithm>
#include <string>

using namespace std;

class Queue {
public:
    Queue(); //Defualt costructor
    Queue(const Queue& source); //Copy constructor
    ~Queue(); //Destructor

    Queue& operator=(const Queue& source); //Assignament Operator //NOT WORKING

    void enqueue(string value); // add item to queue
    string dequeue(); // remove item from queue

    void showFullQueue() const;

    //memory allocation
    void memory(int currentCapacity);
private:
    void shift();
    string * arr;
    const static int front = 0;
    int back;
    int capacity;
    int usedCapacity;
};


#endif // !Queue_H

Queue.cpp

#include "Queue.h"

Queue::Queue()
{
    back = -1;
    capacity = 1;
    usedCapacity = 0;
    arr = new string[capacity];
}

Queue::Queue(const Queue & source)
{
    cout << "Invoking copy constructor" << endl;
    this->back = source.back;
    this->capacity = source.capacity;
    this->usedCapacity = source.usedCapacity;
    arr = new string[source.capacity];
    copy(source.arr, source.arr + usedCapacity, this->arr);
}

Queue::~Queue()
{
    delete[] arr;
}

Queue & Queue::operator=(const Queue & source)
{
    if (this == &source)
        return *this;
    cout << "Invoking Assignament operator" << endl;
    Queue temp(source);
    swap(temp.back, back);
    swap(temp.capacity, capacity);
    swap(temp.usedCapacity, capacity);
    swap(temp.arr, arr);

    return *this;
}

void Queue::enqueue(string value)
{
    ++back;
    arr[back] = value;
    usedCapacity++;
    memory(usedCapacity);
}

string Queue::dequeue()
{
    string returnValue = arr[front];
    shift();
    usedCapacity--;
    memory(usedCapacity);
    return returnValue;
}

void Queue::showFullQueue() const
{
    cout << "Front: " << front << endl;
    cout << "Back: " << back << endl;
    for (int i = 0; i < capacity; i++)
    {
        cout << arr[i] << ", ";
    }
    cout << endl;
}

void Queue::memory(int currentCapacity)
{
    if (currentCapacity >= capacity)
    {
        int newCapacity = (currentCapacity * 3) / 2 + 1;
        string * arr2 = new string[newCapacity];
        copy(arr, arr + usedCapacity, arr2);
        delete[] arr;
        arr = arr2;
        capacity = newCapacity;
    }
    else
        cout << "allocation error";
}

void Queue::shift()
{
    for (int i = front; i <= back; i++)
    {
        arr[i] = arr[i + 1];
    }
    --back;
}

Any help would be greatly appreciated. Thank you in advanced.

You would have saved yourself a whole lot of stress if you used `std::vector` rather than `std::string*` — WhiZTiM, Jun 06 '16 at 19:02
Yeah, I just wanted to play around with dynamic string arrays. I'll will use vectors next time, but for now, I just wanted to know how to implement a queue without using vectors. — Richard Schwartz, Jun 06 '16 at 19:08

score 1 · Accepted Answer · edited May 23 '17 at 12:31

Well, a couple of bugs here to fix..

Your use of the data member back and the static member front seem redundant to me.
I don't see any beneficial use of shift function.

Also, In your copy assignment operator...

Queue & Queue::operator=(const Queue & source)
{
    if (this == &source)
        return *this;
    cout << "Invoking Assignament operator" << endl;
    Queue temp(source);
    swap(temp.back, back);
    swap(temp.capacity, capacity);
    swap(temp.usedCapacity, capacity);    //Notice the Bug here...?
    swap(temp.arr, arr);

    return *this;
}

Well, here is a cleaned up version... (notice a few changes like not using namespace std; in global scope of a header file; and moving unnecessary #includes to the implementation file...

Queue.h

#ifndef Queue_H
#define Queue_H

#include <string>

//using namespace std; ...typically a bad idea in header files

class Queue {
public:
    Queue(); //Defualt costructor
    Queue(const Queue& source); //Copy constructor
    ~Queue(); //Destructor

    Queue& operator=(const Queue& source); //Assignment Operator // WORKING :-)

    void enqueue(std::string value); // add item to queue
    string dequeue(); // remove item from queue

    void showFullQueue() const;

private:
    std::string* arr;
    int capacity;
    int usedCapacity;

    //memory allocation
    void memory(int currentCapacity);    
};


#endif // !Queue_H

Queue.cpp

#include "Queue.h"
//You can put the remaining includes here
#include <iostream>
#include <algorithm>

using namespace std; // You can us it in your cpp file... Not a bad idea.. :-)

Queue::Queue()
{
    capacity = 3;
    usedCapacity = 0;
    arr = new string[capacity]();
}

Queue::Queue(const Queue & source)
{
    cout << "Invoking copy constructor" << endl;
    capacity = source.capacity;
    usedCapacity = source.usedCapacity;
    arr = new string[source.capacity]();
    copy(source.arr, source.arr + source.capacity, arr);
}

Queue::~Queue()
{
    delete[] arr;
}

Queue & Queue::operator=(const Queue & source)
{
    if (this == &source)
        return *this;
    cout << "Invoking Assignament operator" << endl;

    using std::swap; //For ADL: ...redundant here... since you already have using namespace std;
    Queue temp(source);
    swap(temp.capacity, capacity);
    swap(temp.usedCapacity, usedCapacity);
    swap(temp.arr, arr);

    return *this;
}

void Queue::enqueue(string value)
{
    memory(usedCapacity);
    arr[usedCapacity] = value;
    ++usedCapacity;
}

string Queue::dequeue()
{
    string returnValue = arr[--usedCapacity];
    //memory(usedCapacity); //needless
    return returnValue;
}

void Queue::showFullQueue() const
{

    //This styple prevents printing the last comma on the last item
    int i = 0;
    for (; i < usedCapacity-1; i++)
    {
        cout << arr[i] << ", ";
    }
    cout << arr[i] << endl;
}

void Queue::memory(int currentCapacity)
{
    if (currentCapacity >= capacity)
    {
        int newCapacity = (currentCapacity * 3) / 2 + 1;
        string* arr2 = new string[newCapacity]();
        copy(arr, arr + capacity, arr2);
        delete[] arr;
        arr = arr2;
        capacity = newCapacity;
    }
}

And there were no changes to your main.cpp ...:-) See it Working here

Of cause, there are a few more improvements we can make like

Thank you very much! I now understand what I did wrong. I will certainly try to improve my skills in the areas you pointed. Again, thank you for your tremendous help. — Richard Schwartz, Jun 06 '16 at 20:41
@RichardSchwartz, you are welcome... Its a tough job, but we are all learning it... So, more grease to your elbow! — WhiZTiM, Jun 06 '16 at 20:52

score 0 · Answer 2 · answered Jun 06 '16 at 19:15

In Queue & Queue::operator=(const Queue & source) , I think you are making a wrong assignment.

swap(temp.capacity, capacity);
swap(temp.usedCapacity, capacity);

I assume it should be:

swap(temp.capacity, capacity);
swap(temp.usedCapacity, usedCapacity);

Assignment operator copying everything except for the free space in the array

2 Answers2