-2

I have created my own vector class and I have two constructors:

1)First one creates a zero Vector with dimension dim

2)Second one takes as an input vector v and fills in my own Vector:

//Vector:
#include <iostream>
#include <vector>
#include <string>
#include <stdexcept>
#include <cmath>
using namespace std;

class Vector{
private:
    int dim;
    double *elem;

public:
    Vector(int dim); //n-dimensional zero vector
    Vector(const vector<double>& v); // vector based on the input coordinates
    double& operator[](int i) const {return elem[i];}
    Vector operator+(const Vector& v);
    Vector operator-(const Vector& v);
    Vector operator*(const Vector& v);
    void norm();
    int get_size() const {
        return dim;
    }


};

Vector::Vector(int size){
    elem = new double[size]; //adress of the array
    dim = size;
    for(int i = 0;i<size; ++i){
        elem[i] = 0;
    }
}

Vector::Vector(const vector<double>& v){
    for(unsigned i = 0;i<v.size();++i){
        elem[i] = v[i];
    }
    dim = v.size();
}

Compiler throws me no error, but when I try to fill in my Vectors nothing happens.

int main(){

    Vector v1(3);
    vector<double> vect = {1,2,3};
    Vector v2(vect);
    for(int i = 0;i<v1.get_size();++i){
        cout << v1[i];
        }

}

Can you tell me, where did I do a mistake?

vitsuk
  • 85
  • 7
  • 1
    you didn't allocate anything. – Incomputable Dec 22 '17 at 19:26
  • I don't see where you tried to fill in your vector. You created an three vectors, one of which is empty, and then display the empty one. What did you expect to happen? – Mooing Duck Dec 22 '17 at 19:26
  • 2
    Also, your `Vector` violates [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Mooing Duck Dec 22 '17 at 19:27
  • Your `operator *`, `operator +`, etc. return `Vector` by value, yet your `Vector` class is totally unprepared to return `Vector` by value. You need to implement a copy constructor, assignment operator, and destructor first. Then *test* copying, assignment and destruction. Once those functions are tested **then** you add things like `operator *`, `operator -`, etc. – PaulMcKenzie Dec 22 '17 at 19:54

2 Answers2

2

Your program exhibits signs of undefined behavior.

The undefined behavior stems from the incorrectly implemented second constructor.

You are using:

Vector::Vector(const vector<double>& v){
    for(unsigned i = 0;i<v.size();++i){
        elem[i] = v[i];
    }
    dim = v.size();
}

but you have not allocated memory for elem. You need to allocate memory for it first before you can assign values to it.

Vector::Vector(const vector<double>& v){
    elem = new double[v.size()]; // Need this.
    for(unsigned i = 0;i<v.size();++i){
        elem[i] = v[i];
    }
    dim = v.size();
}

Also, please read up on the rule of three and make sure to implement the copy constructor, the destructor, and the copy assignment operator correctly.

An aside

Since you are using std::vector<double> in the interface, is there any reason why you can't use it in the implementation?

Use of

class Vector{
  private:
    std::vector<double> elem;

  ...
};

would make your implementation a lot less cumbersome and error prone.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Thank you for your explanation. I did my implementation in this style, because I wanted to practice in using pointers. But actually I don't understand precisely, when should I use pointers, and when I hould avoind them – vitsuk Dec 22 '17 at 22:28
  • 1
    @vitsuk, You are on the right track for learning. For better or worse, we learn best from our mistakes. – R Sahu Dec 22 '17 at 22:32
1

Your constructor that takes a std::vector needs to first resize your internal array

Vector::Vector(const vector<double>& v){
    elem = new double[v.size()];
    for(unsigned i = 0;i<v.size();++i){
        elem[i] = v[i];
    }
    dim = v.size();
}

By the way you should have a destructor so you aren't leaking memory

Vector::~Vector()
{
    delete[] elem;
}
Cory Kramer
  • 114,268
  • 16
  • 167
  • 218