1

I am fairly new at the C/C++ pointer stuff, so I was wondering if this could cause any problems?

I got a class with a pointer array variable, which I would like to move along with the other variables in the move constructor. Can I just reasign its pointer, or is that a bad idea?

class C {
    private:
        char* a;
        int size;
    public:
        C(int s)
        {
            size = s;
            a = new char[s];
        }
        ~C() noexcept
        {
            size = 0;
            delete[] a;
        }
        C(const C& o): C(o.size)
        {
            std::copy(o.a, o.a + size, a);
        }
        C(C&& o) noexcept: size(o.size)
        {
            a = o.a;
            o.a = nullptr;
        }
        C& operator=(const C& o)
        {
            if(this != &o)
            {
                char * l = new char[o.size];
                size = o.size;
                delete[] a;
                a = l;
                std::copy(o.a, o.a + size, a);
            }
            return *this;
        }
        C& operator=(C&& o) noexcept
        {
            if(this != &o)
            {
                delete[] a;
                size = std::move(o.size);
                a = o.a;
                o.a = nullptr;
            }
            return *this;
        }
};

Any feedback is welcome!

EDITS

  • Added move & assignment operator and copy constructor for complecity sake
  • Updated for full 'working' MVP
n247s
  • 1,898
  • 1
  • 12
  • 30
  • `int main() { C c1(1); C c2(2); c2 = c1; }` -- a double delete error occurs, plus a memory leak. You need to get something simple like that to work first before being concerned about move constructors. – PaulMcKenzie Jan 29 '22 at 11:02
  • I am sorry, do you reffer to the lack of assignment/move operator defenitions? Or how would this make a 'double delete error' ? – n247s Jan 29 '22 at 11:15
  • Write that piece of code. Use the debugger and place a breakpoint in the destructor. Should be very simple to see that the destructor is called twice using the same pointer value. [What is the rule of 3?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), and look at the **Managing resources** section of that link. – PaulMcKenzie Jan 29 '22 at 11:17
  • Oke, so I guese you meant the lack of assignment/move operators? I added a bit more. The above code does compile and run without errors for me though? (also the pointers are different in the destructor), or are you refering to something else? – n247s Jan 29 '22 at 11:34
  • `int main() { C c1(1); C c2 = c1; }` -- Now you are writing to an uninitialized pointer. You really should clean up your code so that it has correct copy semantics with copy, assignment, destruction. Then worry about move constructors. – PaulMcKenzie Jan 29 '22 at 11:38
  • @n247s , have you considered using `std::string`? I don't know what the broader context is and how you use your class but the functionality that you presented in the piece of code above is realized by `std::string` so maybe there is no need to manually manage resources as already pointed out by others. – navyblue Jan 29 '22 at 12:01
  • @PaulMcKenzie Yup, I am trying to copy parts for a clear MVP, so I just switched to building it from scratch. Now it should be 'working'? (I know this is not copy-and-swap idiom, but that is the point because the pointer array shouldn't be copied if possible). – n247s Jan 29 '22 at 13:52
  • @navyblue True if it were just a char-array. I just chose it for simplicity of the MVP. I actually got an array of struct-containers in the full code, so either I convert it to json-strings :P, or I would have to work this out. Thanks for your input btw! – n247s Jan 29 '22 at 13:55
  • Ok, if the type is something more complex than a simple `char` then how about `std::vector`? – navyblue Jan 29 '22 at 13:58
  • True, hadn't thought of that. For knoweledge sake I would still love to hear if its 'safe' to move a pointer as part of a 'move' operation. I would assume so, as deleting a `nullptr` doesn't do anything? – n247s Jan 29 '22 at 14:16
  • Yes, it's safe. I'd also set the `size` field of the object you move from to `0`. Also you don't need to use `std::move` on `int`s (I'm talking about the `size` filed) because it still results in a regular copy of that integer so it's useless (not incorrect but doesn't bring any benefits). Note that your implementation is correct because you're managing an array of `char`s. If you manage some more complex objects which could throw on copy then this code is not sufficient (although the move operations which you asked about are still OK). – navyblue Jan 29 '22 at 15:26
  • 1
    If you see yourself using `new`, you are either doing something wrong, doing a school assignment that requires you to use `new`, or writing a general-purpose resource management component (the last one is unlikely if you are new to C++). – n. m. could be an AI Feb 12 '22 at 09:18

1 Answers1

-1

Why don't you use std::string and let the compiler do the work for you?

class C {
private:
    std::string a;
    
public:
    C() {}    
}
Coding Mason
  • 162
  • 1
  • 14
Richard Day
  • 49
  • 3