0

I am doing an exercise in which I create a class Color which contains three attributes, red, green, and blue which are pointers to ints.

Initially, I did my constructor as follows.

class Color{
    
    int* red;
    int* green;
    int* blue;

    public:
    //getters 
    int getRed()const{return *red;}
    int getGreen()const{return *green;}
    int getBlue()const{return *blue;}

    //constructor
    Color(int Red, int Green, int Blue): red(&Red), green(&Green), blue(&Blue){
        cout<<"Regular constructor called."<<endl;
    }

   
    //copy constructor
    Color(const Color& c){
        
    }

  

};


int main(){

    Color buddy=Color(1,2,3); 

    cout<<"The color's name is buddy. red: "<<buddy.getRed()<<". green: "<<buddy.getGreen()<<". blue: "<<buddy.getBlue()<<endl;

     

    cout<<"buddy red before copy constructor called: "<<buddy.getRed()<<endl;

    Color holly(buddy);
    // Color *geonwoo=erin;

    cout<<"buddy red after copy constructor called: "<<buddy.getRed()<<endl;

    return 0;
}

This gave an output of:

Regular constructor called. The color's name is buddy. red: 1. green: 2. blue: 3 buddy red before copy constructor called: 1 buddy red after copy constructor called: -249563152

(I'm aware the copy constructor isn't doing anything, I'm just calling it to demonstrate what's happening to the object passed in).

When I changed the constructor to this:

    Color(int Red, int Green, int Blue){
        this->red=new int(Red);
        this->green=new int(Green);
        this->blue=new int(Blue);
    }

I got the desired result. Can anyone tell me why this is? I don't feel I'm fully understanding what's happening.

popoccy
  • 1
  • 1
  • if your copy ctor does nothing, then that's what it does, so the pointers don't get initialised. also, the chance of you needing a pointer to int is vanishingly small. – Neil Butterworth Nov 01 '22 at 00:02
  • Why do you have pointers in the first place? What is the problem that is supposed to solve? There's just no reason what so ever to use pointers here, as far as I can see. – Some programmer dude Nov 01 '22 at 00:06

1 Answers1

1

Firstly, it's unclear why you want to store pointers, but I will say that there needs to be a darn good reason and you don't have one.

That said, to answer your question, the problem is that you were storing pointers to temporary values, that are only valid while the constructor is being called. Once the constructor returns, those pointers cannot be used. Doing so results in undefined behavior.

Your solution to allocate and copy is technically correct, but beware: you must at least follow the Rule of Three here, otherwise you can have other problems.

Much better is to avoid doing what you're doing altogether. Do not store pointers in your Color class. Just store integer values.

paddy
  • 60,864
  • 6
  • 61
  • 103
  • using pointers was a requirement for the exercise. i was stuck here so hadn't yet gotten to including the big three. this cleared things up, thanks! – popoccy Nov 01 '22 at 00:35
  • I see. Well, that sure is triggering. There are so many valid uses for pointers, it drives me nuts to see exercises designed to use them in stupid ways that nobody does in the real world. – paddy Nov 01 '22 at 00:49
  • If you are *required* to use pointers (learning purposes) then there's more you need to do to avoid undefined behaviour - there is more work to get code working right when using pointers than your code shows. Your choices include (1) change each argument to reference or pointer, and have the caller pass references or pointers to `int`s that will exist for at least as long as the instance of `Color` does or (2) pass by value and dynamically allocate *copies* of the arguments AND (importantly, if you ever copy or assign a `Color` object) follow the rule of three or the rule of five. – Peter Nov 01 '22 at 01:01