0

I'm getting this error when I try to compile my code. I dont have any *(pointers) and can't understand why im getting this. Im working now with template. You can check my code too see:

#include <iostream>
#include <cstdlib>
#include <stdlib.h>
#include <conio.h>
#include <string.h>
#include <stdio.h>
#include <math.h>
#include <algorithm>
using namespace std;

template <class T>
class Set
{
public:
    T **p;
    int n;
    Set(){
    };
    Set(int n)
    {
        this->n = n;
        p = new T*[n];
    }
    ~Set()
    {
      if(p) delete []p; 
    }
    void setValues(T k,int l)
    {
        p = k;
        n = l;
    }
    void add(T k)
    {
        T p1;
        p1 = p;
        p = new T[n+1];
        p = p1;
        p[n+1] = k;
        n++;
    }
    void remove(T k)
    {
        T p1;
        int l =0;
        p1 = p;
        p = new T[n-1];
        for(int i=0;i<n;i++)
            if(p1[i]!=k) 
            {
                p[l] = p1[i];
                l++;
                n--;
            } 
    }
    void operator+(Set s)
    {
        for(int i=0;i<n;i++)
            p[i]+=s.p[i];
    }
    void operator-(Set s)
    {
        for(int i=0;i<n;i++)
            p[i]-=s.p[i];
    }
    void operator*(Set s)
    {
        for(int i=0;i<n;i++)
            p[i]*=s.p[i];
    }
    void show()
    {
        for(int i=0;i<n;i++)
            cout<<p[i]<<" | ";
    }
};
int main()
{
    Set<int> s1,s2;
    int arr[]={0,2,3,4,3,6};
    int n=6;
    float arr2[]={0.5,12.1,1.7,23.15};
    char arr3[]={'a','h','m','k','c','e'};
    s1.setValues(arr,n); // <------------- here is error;
    s1.show();

}

I'm getting error on this line s1.setValues(arr,n); This is setValues() method:

void setValues(T k,int l)
{
    p = k;
    n = l;
}

I had tried to avoid the error by using & like that: s1.setValues(arr,&n) and s1.setValues(arr,*n)

Also I tried to change it in method: void setValues(T k,int &l) and void setValues(T k,int *l) in class: public: T **p; int *n; and public: T **p; int &n;

In my first version I have tried to use: s1.setValues(arr,6) where 6 is the length of the array. But I was also getting the error;

Christophe
  • 68,716
  • 7
  • 72
  • 138
dreeeeeedd
  • 29
  • 1
  • 5
  • 1
    The problem is the `arr` part not the `n` part. Consider that `T` is `int` and that `arr` is an array of `int`. What do you want `k` to be in the function? – walnut Jan 18 '20 at 17:19
  • Keep in mind that in C++, when you try to pass an array to a function, it [decays](https://stackoverflow.com/questions/1461432/what-is-array-decaying) to a pointer. This explains why you have an error regarding pointers even though you haven't explicitly used any. – Nate Eldredge Jan 18 '20 at 17:21
  • Note that the statement `p=k` in `setValues` is also nonsense. In this instance, `p` is a pointer to pointer to int, and `k` is an int. It doesn't make sense to assign one to the other. What are you actually trying to achieve here? – Nate Eldredge Jan 18 '20 at 17:23
  • Im trying to use constructor , and to copy all from array in class – dreeeeeedd Jan 18 '20 at 17:26

2 Answers2

1

Edit: I had misread the type of p, and also apparently read the error backwards.

The class is intantiated with T as int. All arrays decay to pointers, so arr is of type int*. This means you cannot pass it in as the first argument of setValues.

void setValues(T k, int l);

...

s1.setValues(arr, n); //arr is type int*, but k is of type int.

I'm afraid I don't understand what your code is doing well enough to suggest how you might want to fix it, but this is why the compiler is emitting that error.

Korosia
  • 593
  • 1
  • 5
  • 19
0

Preliminary remark: since you do not use std::set I suppose this is an exercise, and that you are not allowed to use std::vector which would considerably facilitate the management of p

The problem(s)

There are some contradictions in this code, and some more errors that do not appear yet because the corresponding template member function is not used:

  • The member T** p is defined as a pointer to pointer of T
  • In the constructor you allocate an array of pointers to T, which will give you indeed a pointer to pointers of T.
  • But in add() and remove() you try to reallocate p's with an array of T, which will give you a T* instead of T**. In addition you clearly want to add or remove a value and not a pointer.
  • But in set value, it is not clear if you want to set a single value (which the T k in the signature suggests) or if you want to set all the values from an array (which the length argumen int l suggests). Given the name of the function has Values in plural, I will assume the second alternative.

Solution Step 1: Get it to compile. But it won't really work

You need to correct:

T *p;  

and adapt the constructor accordingly with p = new T[n];

   auto p1 = p;

You then need to change your function definition to:

void setValues(T k[],int l)  // option 1 
void setValues(T *k,int l)   // or option 2 

The good news is that the code will then compile

The bad news is that it will not do what you want. Furthermore the code of the function and some other problems will cause UB (meaning that a lot of bad things can happen, including memory corruption, crash, etc...).

Solution Step 2: assignment of arrays, pointers and the rule of 3

This code does not what you expect it to do:

void setValues(T *k, int l)
{
    p = k;   // OUCH !!!!!! pointer copy and not copy of the array elements
    n = l;
}

Currently, you do not copy the elements of an array, but you replace a pointer. This has following consequences:

  • first you will leak memory, since the pointer to the allocated memory is lost and will never be freed.
  • second, you will make this pointer point to an array that was not allocated with new[], which might hurt you badly when you later delete[] this pointer.
  • third: the array to which your class points might get deleted by the owner of the array, in which case you'd use a dangling pointer.

So you need to adapt your code as follows:

void setValues(T k[],int l) // change signature
{
    if (n!=l) {          // if the size is the same, we'll reuse p, if not:
        delete[] p;            // avoid the memory to leek 
        p= new T[l];           // start with a fresh memory 
        n = l;                 // define new length
    }
    for (int i=0; i<l; i++) 
        p[i]=k[i];              // copy array elements one by one 
}

Another important problem is the rule of 3. This is not a problem with your current main() but quickly will become: if you work with pointers in a constructor or a destructor, you should define a copy construtor and an assignement operator. If you don't, you risk to end with shallow copies of your Set (2 different object but using the same pointer).

Solution Step 3: get rid of the remaining issues

First, your default constructor leaves p and n unitialized. They could be random value, which might cause the destructor to try to delete something that was not allocated. And the same for anny other function that assumes that things that assumed the member values would be consistend. So:

Set(){ 
    p=nullptr; 
    n=0; 
};

Then you have similar problems in add(), that you had in setValues() with the pointer assignment. IN addition you go out of bounds, even after the realocation: the last element of an array p with n+1 element is p[n] and not p[n+1]. I let you as an exercise to improve it.

Online demo

Christophe
  • 68,716
  • 7
  • 72
  • 138