0

I am learning about dynamic memory allocation and overloading operators in C++. I am trying out a simple program to test out my knowledge but I can't find out what I'm doing wrong. Here's the code:

#include <iostream>
#include <cstring>
using namespace std;
class myClass{
    private:
        char *ptr;
    public:
        myClass () {}
        myClass (char *str) 
        {
            ptr = new char[strlen(str)];
            strcpy(ptr, str);
        }
        myClass (const myClass &k) 
        {
            ptr= new char [strlen(k.ptr)+1];
            strcpy(ptr, k.ptr);
        }
        myClass& operator= (const myClass k)
        {
            delete [] ptr;
            ptr = new char [strlen(k.ptr)+1];
            strcpy(ptr, k.ptr);
            return *this;
        }
        ~myClass() {
            delete [] ptr;
        }
        void print() {
            cout<<*ptr;
        }
};

int main() {
    char s[6]="Hello";
    myClass p(s), m;
    m=p;
  m.print();
  return 0;
}

I am trying to use the operator = so I can assign the value of ptr from object p to object m, but I get no output. Any ideas what I am doing wrong?

D. D.
  • 97
  • 1
  • 2
  • 10
  • `myClass& operator= (const myClass k)` should be `myClass& operator= (const myClass& k)`. Uncertain if that will fix your problem, but it is wrong. – Yakk - Adam Nevraumont Mar 20 '18 at 23:49
  • `myClass (const myClass &k)` should be `myClass (const myClass &k):myClass(k.ptr){}`. `myClass (char *str)` should be `myClass (char const*str)`. Both of these are not the cause of your problem. – Yakk - Adam Nevraumont Mar 20 '18 at 23:51
  • @Yakk fixed it, now I get only "H" as an output. – D. D. Mar 20 '18 at 23:52
  • `cout<<*ptr;` should be `cout< – Yakk - Adam Nevraumont Mar 20 '18 at 23:52
  • The error is solved when `k` is passed by reference. I don't _why_, though. I can't possibly see any difference with pass by const copy or const reference. – Arnav Borborah Mar 21 '18 at 00:05
  • Be aware that stream operators (`<<`) treat character arrays differently from other data types, so it's probably wise to use a more typical data type when trying to learn these things. – Beta Mar 21 '18 at 00:16
  • In `operator=`, your first line breaks any time you assign something to itself `a = a;` – jmucchiello Mar 21 '18 at 00:32
  • Use copy-and-swap to implement `operator=`. My answer shows how. – Jive Dadson Mar 21 '18 at 00:35
  • 2
    The default constructor does not initialize `ptr`. When a default-constructed `myClass` goes out of scope, `delete[]` will be called on the uninitialized `ptr`, resulting in a crash if you're lucky. Are you still here? Speak to me. – Jive Dadson Mar 21 '18 at 00:51
  • 1
    In `myClass (char *str) ` you do not allocate enough bytes, causing undefined behaviour. If you fix that then you should see `H` as output (since you decided to output `ptr[0]` instead of `ptr`) – M.M Mar 21 '18 at 01:12
  • @JiveDadson changing the `myClass& operator= (const myClass k)` to `myClass& operator= (const myClass& k)` and fixing `cout< – D. D. Mar 22 '18 at 01:05

2 Answers2

0

Doing

*ptr, *(ptr+1), ...

you just acces one element of your char array.

My correction:

void print() {
    cout << ptr
}
Ratah
  • 299
  • 3
  • 11
0

The best way to do dynamic memory allocation is to let the compiler do it for you, perhaps in consort with standard library containers like std::string and std::vector. Standard containers manage the dynamic memory allocation without bothering you to lift a finger.

Read all about it.

If you really need to roll your own, read up on the Rules of zero, three, and five.

Also, learn the copy-and-swap pattern. The best way to do operator= is to use that pattern.

Here's the way it's done. I keep this code handy for when I forget. As you can see, doing a container properly is not trivial. That's why we use STL containers.

namespace dj::example {

    template<class T>
    class my_vec
    {
    public:

        // constructors
        my_vec()
           : mSize(0)
           , mArray(nullptr)
        {}

        explicit 
        my_vec(std::size_t size) 
            : mSize(size)
            , mArray(new T[mSize]) // Can throw when size is huge
        {}

        // copy-constructor
        my_vec(const my_vec& other)
            : mSize(other.mSize)
            , mArray(new T[mSize]) // Can throw when size is huge
        {
            // For Visual C++
            // Put _SCL_SECURE_NO_WARNINGS in vc++ preprocessor 
            // to shut up VC++ about "deprecated" std::copy
            std::copy(other.mArray, other.mArray + mSize, mArray);
        }

        // destructor
        virtual ~my_vec()  // Make virtual in case my_vec is used as base class.
        {
            delete [] mArray;  
        }

        // Specialize external swap! Necessary for assignment operator below,
        // and for ADL (argument-dependant lookup).
        friend void swap(my_vec& first, my_vec& second) noexcept
        {
            // enable ADL (best practice)
            using std::swap; 

            // by swapping all the members of two objects,
            // they are effectively swapped
            swap(first.mSize, second.mSize); 
            swap(first.mArray, second.mArray);
        }

        // Assignment operator. Note that the argument "other" is passed by value.
        // This is the copy-and-swap pattern (best practice).
        // It assures that noexcept is true.
        my_vec& operator=(my_vec other) noexcept
        {
            swap(*this, other); // Uses specialized swap above. It must.
            return *this;
        } 

        // move constructor - construct and swap idiom (best practice)
        my_vec(my_vec&& other) noexcept
            : my_vec()
        {
            swap(*this, other);
        }


    // xxxxxxxxxxx
    // Everything below here is only to make my_vec a working example.
        // Member functions and typedefs below are for testing the example
        // ... not part of schema.

        using const_iterator = const T*;
        using iterator = T*;
        using size_type = std::size_t;
        using value_type = T;
        using type = my_vec<T>;

        iterator begin() { return mArray; }
        iterator end() { return mArray + mSize; }
        const_iterator begin() const { return mArray; }
        const_iterator end()  const { return mArray + mSize; }
        const_iterator cbegin() const { return mArray; }
        const_iterator cend()  const { return mArray + mSize; }
        T operator[](int i) const {
            return *(begin()+i);
        }
        T& operator[](int i) {
            return *(begin()+i);
        }
        std::size_t size()  const { return mSize; }

    private:
        std::size_t mSize;
        T* mArray;
    };

} // namespace dj
Jive Dadson
  • 16,680
  • 9
  • 52
  • 65