Creating a custom Vector class

I'm new to C++ and am doing the C++ 4th Edition Stroustrup book. I expanded on one of the examples, and have a few questions to ask (embedded within the code: ////QUESTION 1-9).

Please provide any critiques, I'd like to bang in safe practice as early as possible. The Vector.h file simply declares these functions, and two private members, elem (list of elements, int*) and sz (the size, int).

```
Vector::Vector(std::initializer_list list) //called via list init: ie, Vector v = {1, 2, 3, 4};
:elem{ new int[list.size()] }, sz{ list.size() }
{
copy(list.begin(), list.end(), elem); //copy list from start to end to elem

}

Vector::Vector(int s) //Constructor w/ size
{
if (s =size()) throw out_of_range{ "Vector::operator[]" };
return elem[i];
}

Vector& Vector::operator=(const Vector& a) { //copy assignment op
int* p = new int[a.sz];
for (int i = 0; i elem = p;
this->sz = a.sz;
return *this;
}

const bool Vector::operator==(Vector& right) const { ////QUESTION 4
if (size() != right.size())
return false;
else {
for (int i = 0; i elem = p;
this->sz += a.sz;
return *this;
}

const Vector& Vector::operator++() {
this->pushBack(0);
return *this;
}

//MEMSAFE
////QUESTION 6
const Vector& Vector::operator--() {
//delete elem[sz - 1]; //delete (elem+sz); //this hates me.
this->sz -= 1;
return *this;
}

const Vector Vector::operator+(const Vector& a) { ////QUESTION 7
if (this->sz != a.sz)
return NULL;
Vector v = sz; //init's Vector with all 0's ( O(2n) with this init, and the for loop below..)
for (int i = 0; i sz; i++)
v.elem[i] = elem[i] + a.elem[i];
return v;
}

const Vector& Vector::operator+(int x) {
this->pushBack(x); //recycling working code
return *this;
}

//Returns a Vector with the calling Vector's remaining elem's (ie, all except last) - doesn't affect calling Vector in any way
Vector Vector::softRest() const {
Vector v = *this;

Questions:


How would it differ if I set elem & sz in the body of the constructor? As of now, they are being declared after the method declaration, but before the start of the actual function (in the Copy Constructor).

It's best to set everything in the initializer list (good habit when things could be arbitrary objects).


removed


I use the sz variable, as an upperbound for a loop. What is safest? Should I use sz, this->sz, size() or this->size()? size() is a function within the code which returns this->sz;

Just use sz.

Use of this-> is discouraged as it means you are trying to force the compiler to resolve a particular variable that is shadowed which means you are using a bad naming scheme that is suitable to shadowing.

Shadowing causes all sorts of problems. One way to get around it is to force the use of this-> on all members (which is fine until you accidentally miss one).

The better option is turn up your compiler warnings so it warns you about shadowing. Then treat all warnings as errors (your code should compile warning free on the highest warning level).


Am I overusing const? Since there are no assignment operators within the function, it doesn't perform any changes to its class members - so is the last const useless?

• Don't bother with const on the return type when returning by value.

• Be judicious on your use of returning Vector by const ref.

A lot of the time you want to return *this as a ref to allow chaining.


My operator==(Vector&) function is pretty ugly. Any suggestions for a nicer/more efficient solution?

You could use a standard algorithm to help you check. But your code does not look that bad.


In my operator--() function (which is meant to remove the last element in the vector), I simply reduce the sz variable for the calling vector by 1. I'm not actually deleting anything. Is this bad practice? What is a better solution? Is there a way to delete a single entry in an array?

Normally vector contain two sizes.

• The number of elements currently in the vector.

• The amount of space allocated.

This is space allocated but currently unused. Normally when creating arrays you allocate slightly more space than you need. So you can use it without having to reallocate the whole data segment and copy it just for adding a single value (or when deleting a value you just reduce the size and can safely re-use it).


I understand that a (const Vector& a argument to the operator+(..) function) is a const, but the function doesn't change the argument whatsoever. If the function remains as is, could I remove the const declaration which prepends the argument?

?


In my pushBack(int) function, an int array (called temp) is created using new - which means it must be deleted. However, using _CrtDumpMemoryLeaks();, I get no objection from the compiler. Is this because it automatically self-deconstructs because it's an int array?

?


Why shouldn't my size() return sz instead of this->sz. Am I correct in understanding this is primarily for multithreaded reasons?

Its the same thing. See my description of this usage above.

Comments on Code:

Always prefer to use the initializer list. The compiler is going to plant the appropriate code anyway. May as well take advantage of this fact and use the compiler to put the corret initial values in place. (Note with POD data there is no initialization but for user defined types there will be. So it will construct the object members before the function is entered.

Vector::Vector(int s) //Constructor w/ size
// Add initializer list.

The following can be done in a single line:

elem = new int[s];
    for (int i = 0; i < s; i++)
        elem[i] = 0; //init elems to 0

    //
    elem = new int[s](); // zero initialize all members.
                         // Or default construct them if you change the Vector to
                         // use generic types.

Note: members are initialized in the order they are declared in the class declaration (not the order they appear in the initializer list). If you turn up wanings the compiler will warn you about this. If you make the compiler treat warnings as errors (as you should be doing) then it will not compile if the initializer list is in the wrong order.

Vector::Vector(const Vector& a)
    :elem{ new int[sz] },             // Is `sz` defined at this point ???
                                      // I can't tell because I don't have the class
                                      // declaration.

    sz{ a.sz } ////QUESTION 1         // But the order here is not conjusive to read
                                      // as it looks like you are setting sz after
                                      // you have used it in the previous line.

In this one you don't initialize elem.

Vector::Vector() //empty constructor - functionally useless
{
    sz = 0;
}

This means it is pointing at some random piece of memory. When the destructor is run you

StackExchange Code Review Q#50975, answer score: 8