Finally trying to learn vectors

[jjordan33]

Hi, so, first, I'm making my own vector class instead of using the library. This is homework. I'm not necessarily asking for code. I have written some code, I just don't know if it is usable in the situation.

I have a Vector object(which is a double pointer array), and I'm overloading the = operator so that two objects can equal each other.

  // in main....
    Vector* a = new Vector();
    Vector* c = new Vector();
    c[0] = a[0];

    // in my .cpp
    
    Vector& Vector::operator=(const Vector &other){
    double *Equal = new double[other.size];

    size = other.size;
    capacity = other.capacity;

    for (int i = 0; i < size; i++){

        Equal[i] = other.ptr[i];

    }

    return *this;
}

Overload specifics...

Vector& operator=(const Vector &other);
/* Description:
* Assignment operator. Initializes a dynamic array of the appropriate
* size/capacity and copies data from other's array to the new array.
*
* Parameters:
* other - a constant reference to the vector that is to be copied.
*
* Post-conditions:
* ptr is initialized, and its array contains other.ptr's data,
* size=other.size, and vec_capacity=other.capacity. Note you must create a copy of other's array for the new array to get credit.
*/

Again, I understand that help is limited for hw, and that's okay. I just wanna know if I'm on the right track.

Thank you.

*edit*
There are no immediate errors with the code, compilation-wise.

*edit to include constructor and member variables*

private:
double* ptr;
int size, capacity;

// constructor 
Vector::Vector(){

     size = 0;
     capacity = 0;

     ptr = NULL;

[keskiverto]

First, the code in main seems both unnecessarily complex and insufficient for testing the assignment. I'll write a similar main:

int* a = new int;
int* c = new int;
c[0] = a[0];
delete c;
delete a;

Why not a simpler:

int a; // Vector a;
int c; // Vector c;
c = a;

Two objects and assignment. No dynamic allocation to obfuscate what we are actually focusing on.
However, how can you see that assignment makes a difference? With unique values:

int a = 42;
int c = 7;
c = a;
std::cout << "a " << a << " c " << c << '\n';

Now we get to "real deal":
* You did not show the definition of class Vector. We don't know what members it has.
* You did not show the implementation of Vector's constructor. We don't know the invariants that the assignment has to maintain.


You are on "a track", but in programming being just "one off" from the right track leads to unknown.

[jonnin]

If you were doing this to USE you might consider things like over-allocation and tracking memory vs item count differently.

double *Equal = new double[other.size*1.2]; //example 20% extra cap
and track 2 sizes (how much memory you have NOW, and how many THINGS are in there).

[jjordan33]

>>> keskiverto

I updated my post to include those things. Thank you.

>>> jonnin
Will the sizeof function return the max the array can hold or something else?
Thank you.

[jonnin]

it should only return the size of your container. this will have nothing to do with the size of the data that it contains; that is hidden behind a pointer and the byte sizeof counter ONLY COUNTS THE POINTER'S bytes NOT WHAT IS POINTED TO. A pointer is 4 or 8 bytes or something like this.

Consider:

vector<int> v(100000);
	for(int i = 0; i < 100000; i++)
		v[i] = i;
	cout << sizeof(v);  //I get 12 bytes!! Obviously, 100000 ints is much bigger than 12 bytes. 

[jjordan33]

Okay, that makes sense then.

Alright, so....
hmm

Is there any way to know that information other than updating size and capacity as you make changes to either of them?

[jonnin]

no, you have to track it. Capacity should only change in a couple of places. Size will change in many more, most likely. I would even say that capacity should only be touched in 1 place... some sort of (if pointer is null allocate first time and set capacity, else make new one, copy existing into it, and update capacity) allocation function. It may even make sense to use C's realloc, though that is a little messy. Reallocating is expensive. Avoid it as much as you can.

the implication of this is something like push-back is going to look like this...
if capacity == size //or something along these lines to detect full/can't add more without growth
this.realloc(); //assuming it grabs min(currentsize*1.??, minsize) or something similar where 1.??*minsize ensures growth
thing[size-1] = new data
size++;

how much should it grow and minsize... I have no idea what is practical here. 10k min? 1.2 (20% growth?) ??

[keskiverto]

Vector::Vector() {
     size = 0;
     capacity = 0;
     ptr = NULL;
}

1. There is member initializer list syntax that you should prefer
2. nullptr is safer than the blunt macro.
after those:

Vector::Vector()
 : ptr(nullptr), size(0), capacity(0)
{
}

(Note that order must match order in definition.)

Now we see that you do not allocate any memory for default constructed Vector.
Vector, that has capacity==0.

Think about your:

Vector a; // a.ptr == nullptr
Vector c; // c.ptr == nullptr
c = a;
// What is the value of c.ptr now? 

Did you assign something to c.ptr within the copy assignment?
Did you allocate memory? Did you leak memory?

Would it make sense to new double[ 0 ]?

What information is stored in the Vector::capacity?


What if c.ptr does already point to an array before assignment? What to do with that array?

We know the post-condition: c.capacity == a.capacity
What if the post-condition is true already before the assignment?

[jjordan33]

Thanks yall!
I got an A on it.
I had a few deductions for not understanding the functions purposes, but eh.

#include <iostream>
#include <fstream>

#include "Vector.h"

using namespace std;

Vector::Vector()
        : ptr(nullptr), size(0), capacity(0)
{
}

Vector::Vector(const Vector &other){

    size = other.size;
    capacity = other. capacity;

    for(unsigned int i = 0; i < size; i++){
        ptr[i] = other.ptr[i];
    }

}

Vector::~Vector(){
    delete[] ptr;
}

Vector& Vector::operator=(const Vector &other){

    double* temp = new double[other.size];

    size = other.size;
    capacity = other.capacity;

    for (unsigned int i = 0; i < size; i++){

        temp[i] = other.ptr[i];

    }

    return *this;

}

int Vector::getSize(){

    return size;

}

int Vector::getCapacity(){

    return capacity;

}

bool Vector::push(double element, unsigned int index){ // a confusing function. takes in an index
                                                      // but doesn't really use it.
    while(capacity < index){ // I got counted off for this.
        resize();                  // only one resize is needed.
    }

    if (size == index){
        ptr[index] = element;
        size++;
        return true;
    }
    else if (size > index){
        ptr[index] = element;
        size++;
        return true;
    }
    else {
        ptr[size] = element;
        size++;
        return false;
    }
}

double Vector::pop_front(){ // removing the first element

    double x = ptr[0];
    size--; // Another deduction for this being above the modification.

    for (unsigned int i = 0; i < size; i++){
        ptr[i] = ptr[i+1];
    }



    return x;

}

void Vector::resize(){

    if (capacity == 0){
        capacity = 1;
    }
    else{
        capacity = capacity * 2;
    }

    double* temp = new double[capacity];
    for (unsigned int i = 0; i < size; i++){
        temp[i] = ptr[i];
    }

    delete[] ptr;
    ptr = temp;

}

double& Vector::operator[](unsigned int index){

        return ptr[index];

}

Improvements are always welcome.
Thanks again.

[keskiverto]

Seriously? That code is unacceptable.

Vector::Vector(const Vector &other){
    size = other.size;
    capacity = other. capacity;
    // the ptr is uninitialized

    for(unsigned int i = 0; i < size; i++){
        ptr[i] = other.ptr[i]; // undefined behaviour
    }
}

Vector& Vector::operator=(const Vector &other){
    double* temp = new double[other.size];
    size = other.size;
    capacity = other.capacity; // Why? The temp has only size elements

    for (unsigned int i = 0; i < size; i++){
        temp[i] = other.ptr[i];
    }
    // the this->ptr was not modified
    // error: this is not a proper copy of other
    return *this;
    // memory leak
}

[jjordan33]

>> keskiverto

It was graded on a specific rubric since it is our first vector assignment. All following assignments will be graded the way you're thinking, so thank you for the input.

I can't really make any remarks on your comments since I don't really understand how to translate it into code. All I can really say is it compiled, doing what the overall point of the assignment wanted, so I thought it was correct. :/

[Ganado]

I can't really make any remarks on your comments since I don't really understand how to translate it into code

Let's just step through it one at a time.
When you call your Vector(const Vector &other) constructor, what is ptr being set to? Not ptr[i] but ptr itself. What is its value?

[jjordan33]

hm. I think I see the point now.

If Vector(const Vector &other) is called first, ptr is just a declared variable.
Question: if initializing ptr as null is okay, why can't ptr just simply be initialized and filled at the same time? Or put another way, why isn't filling ptr[i] with values considered initializing?

[Ganado]

If Vector(const Vector &other) is called first

I think this part of the misconception; there is no "if" here. Vector(const Vector &other) is a constructor; it is always called "first" (for the object being created); it is where the object's life begins. Forget about any other objects that exist (for this particular part).

Do you see why this is bad?

int main()
{
    int* ptr = nullptr; // doesn't actually matter whether it's nullptr or just uninitialized
    ptr[42] = 1729;
}

It's bad because there is no memory allocated at ptr[42]. A pointer needs to point to valid memory to be dereferenced (or have its array index accessed).

The same thing is happening at the beginning of your Vector(const Vector &other) constructor. You are attempting to access ptr[42] (or some other ptr[i]), but there is no valid memory at that location to access.

Assuming you're being forced to use new[]/delete[], you'd do something like
ptr = new double[size];

[jjordan33]

Alright, I can wrap my head around that. A dynamic object's life has to begin pointing to valid memory.

Should I have nulled ptr at the beginning of the parametized constructor?