0

I have got this assignment but this program is not working and output is not getting properly. This program compiles successfully, but it gives error - Segmentation fault (core dumpped). I am not getting why this is happening. Please tell me what is the problem in the below code -

    #include<iostream>
    using namespace std;
    class Vector
    {
        int *V;
        int size;
        public:
        Vector(int m)
        {
            V = new int[size=m];
            for(int i=0; i< size; i++ )
            {
                V[i] = 0;
            }

        }
        Vector(const int *a)
        {
            for(int i=0;i<size;i++)
            {
                V[i] = a[i];
            }
        }
        int operator * (Vector &y)
        {
            int sum = 0;
            for(int i=0;i<size;i++)
            {
                sum+= this->V[i] * y.V[i];
            }
            return sum;

        }
        void printVector()
        {
            for(int i=0;i<size;i++)
            {
                printf("%d\t",this->V[i]);
            }
        }
    };
    int main()
    {
        int x[3] = {1,2,3};
        int y[3] = {4,5,6};

        Vector V1(3);
        Vector V2(3);
        V1.printVector();
        V2.printVector();
        V1 = x;
        V2 = y;
        V1.printVector();
        V2.printVector();
        int r = V1*V2;
        cout<<endl<<"r = "<<r<<endl;
    }
  • 1
    unrelated, but your `Vector` class is missing a destructor. without it, there won't be anything to `delete [] V;` - hence, memory leak. – selbie Jan 03 '22 at 06:10
  • 1
    The direct use of new/delete should be kept to a minimum (C++ core guidelines,https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) thus use std::make_unique (https://stackoverflow.com/questions/21377360/proper-way-to-create-unique-ptr-that-holds-an-allocated-array). By using a unique_ptr you don't have to write delete in your destructor. – Pepijn Kramer Jan 03 '22 at 06:12
  • @PepijnKramer But then it's better to just use `std::vector`... but I'm guessing the assignment is about writing your own vector, possibly to teach about dynamic memory allocations. – Etienne de Martel Jan 03 '22 at 06:15
  • @EtiennedeMartel Totally agree with std::vector. And the lesson for dynamic memory allocation is "manual management of dynamic memory allocations is a pain" and leads to runtime problems. – Pepijn Kramer Jan 03 '22 at 06:18
  • 1
    Unfortunately, not every school is aware of that. – Etienne de Martel Jan 03 '22 at 06:18
  • `std::vector` has been part of C++ for 24 years now, older than most college students. It's crazy that in this day and age, `vector` is looked upon as something new or odd. – PaulMcKenzie Jan 03 '22 at 07:12
  • 1
    @EtiennedeMartel *possibly to teach about dynamic memory allocations* -- The issue I have with this is -- if this is the case, why the teachers just throw their students into the lions cage without telling them anything about the lion? Why the OP didn't know about the destructor, or about user-defined copy operations, etc.? C++ is one of the most difficult languages out there, and just handing students these types of assignments without even acknowledging what needs to be done is a disservice. – PaulMcKenzie Jan 03 '22 at 07:16
  • @OP *This program compiles successfully* -- This only means that the program has no syntax errors. It doesn't mean that the program is logically correct. If I asked you to write a program to add two numbers, but instead the program subtracts two numbers, that program will compile successfully, but is it correct? No. – PaulMcKenzie Jan 03 '22 at 07:21

2 Answers2

2

Vector(const int *a) does not initialize Vector::V, replace it with Vector(const int *a, int size) : Vector(size)

but you will have to replace

        V1 = x;
        V2 = y;

with

        V1 = Vector(x,3);
        V2 = Vector(y,3);

This code will result in a memory leak btw.

NoNae
  • 332
  • 3
  • 10
  • thanks sir, it means in a life of object only 1 constructor is called, doesn't matter it is copy constructor is called or not – Umesh Bagade Jan 03 '22 at 15:17
1

Initialization of vector is should not be done in that way.

        V1 = x; // V1 is a Vector object, x  is an Integer array
        V2 = y;

These lines do not initialize a vector.

Here is the code after making the above changes:


    #include<iostream>
    using namespace std;
    class Vector
    {
        int *V;
        int size;
        public:
        Vector(const int *a, int m)    
        {
            V = new int[size=m];
            for(int i=0; i< size; i++ )
            {
                V[i] = a[i];
            }

        }
        int operator * (Vector &y)
        {
            int sum = 0;
            for(int i=0;i<size;i++)
            {
                sum+= this->V[i] * y.V[i];
            }
            return sum;

        }
        void printVector()
        {
            for(int i=0;i<size;i++)
            {
                printf("%d\t",this->V[i]);
            }
            cout << endl;
        }
    };
    int main()
    {
        int x[3] = {1,2,3};
        int y[3] = {4,5,6};

        Vector V1(x, 3);
        Vector V2(y, 3);
        V1.printVector();
        V2.printVector();
        int r = V1*V2;
        cout<<endl<<"r = "<<r<<endl;
    }

This should work.

Happy coding!

csgeek
  • 711
  • 6
  • 15