0

I am working on an assignment that calls for the writing the function implementations of the Big 3 when constructing a class with pointers. I seem to be having trouble with the copy constructor. I think I am doing the other 2 correctly.

class RealBox
{
private:
  float* m_reals;                     // Array of Real Numbers
  int m_boxsize;                      // number of Real Numbers in this box


public:
  // Purpose: Constructs an Real-Box
        // Preconditions:
        //     's' is greater than 0;
  // Postconditions:
  //     m_reals points to a dynamically allocated array of size 's'
  //     all elements of m_reals[] are initiallized to 'a'.

  RealBox(int s, float a);



  /*
   * --------- Big 3 Member Functions -----------
   */

  // Purpose: Destructor
  // Postconditions: m_reals[] deallocated
  ~RealBox();

  // Purpose: Operator=, performs a deep copy of 'rhs' into 'this' RealBox
  // Parameters: rhs, RealBox to be copied
  // Returns: *this
  // Postconditions: *this == rhs
  const RealBox& operator=(const RealBox& rhs);

  // Purpose: Copy Constructor
  // Parameters: rhs - RealBox to be copied
  // Postconditions:  *this == rhs
  RealBox(const RealBox& rhs);


  /*
   * ----- Simple Accessor Operations -----
   */

  // Purpose: Sets a value in the RealBox
  // Parameters: 'i' location to set
  //             'x' value to store
  // PreConditions: 'i' is between the boundaries of the RealBox
  // Postconditions:  element 'i' in the RealBox is set to 'x'
        void set( int i, float x);


  /*
   * ----- Complex Accessor Operations -----
   */

  // Purpose: prints the RealBox
  friend std::ostream& operator<< (std::ostream& out,
                                   const RealBox& box);
}; // RealBox

#include "realbox.h"
#include <iostream>
using namespace std;

RealBox::RealBox(int s, float a)
{
 m_reals = new float[s];
 for (int i=0; i < s; i++)
 {
  m_reals[i] = a;
 }
}

RealBox::~RealBox()
{
 delete [] m_reals;
 m_reals = NULL;
}
const RealBox& RealBox::operator =(const RealBox& rhs)
{
 if(this != &rhs)
   *this = rhs;
 return(*this);
}

RealBox::RealBox(const RealBox& rhs)
{
 m_reals = new float[m_boxsize];
 *this = rhs.m_reals;
}


void RealBox::set(int i, float x)
{
 m_reals[i] = x;
}

std::ostream& operator<< (std::ostream& out, const RealBox& box)
{
 out <<"[ ";
 for (int i = 0; i < box.m_boxsize; i++)
 {
  out << box.m_reals[i] << ", ";
 }
 out <<"]"<< endl;
 return(out);
}

When I try to compile, I am getting the error:

realbox.cpp: In copy constructor ‘RealBox::RealBox(const RealBox&)’: realbox.cpp:33:8: error: no match for ‘operator=’ (operand types are ‘RealBox’ and ‘float* const’)

*this = rhs.m_reals; ^ realbox.cpp:33:8: note: candidate is:

realbox.cpp:23:16: note: const RealBox& RealBox::operator=(const RealBox&) const RealBox& RealBox::operator =(const RealBox& rhs) ^ realbox.cpp:23:16: note: no known conversion for argument 1 from ‘float* const’ to ‘const RealBox&’

I think my operator = overload is faulty as well but I'm not sure exactly what went wrong. It didn't give me that error until I started trying to fix. It's trying to say I'm comparing two different data types but I'm not sure how that is happening.

icedwater
  • 4,701
  • 3
  • 35
  • 50
  • These code snippets are obviously not runnable HTML/JavaScript. And really, in 2015, you should be following the rule of _zero_, not the rule of three. Get yourself a lovely `std::vector` and quit with all this manual memory management. – Lightness Races in Orbit Feb 27 '15 at 12:09

2 Answers2

1

This line:

*this = rhs.m_reals;

Attempts to assign a float* to a RealBox object. The two types are incompatible.

Second, you failed to initialize m_boxsize when you constructed the object.

What you want to do is this:

RealBox::RealBox(const RealBox& rhs) 
{
  m_boxsize = rhs.boxsize;
  m_reals = new float[m_boxsize];
  for (int i = 0; i < m_boxsize; ++i )
     m_reals[i] = rhs.m_reals[i];
}

This can be shortened to this:

RealBox::RealBox(const RealBox& rhs) : m_boxsize(rhs.m_boxsize), 
                                        m_reals(new float[rhs.m_boxsize])
{  
   std::copy(rhs.m_boxsize, rhs.m_boxsize + m_boxsize, m_reals);
}

In addition, your other constructor needs to initialize the m_boxsize variable:

RealBox::RealBox(int s, float a) : m_boxsize(s)
{
   m_reals = new float[s];
   for (int i=0; i < s; i++)
       m_reals[i] = a;
}

This can also be shortened to this:

RealBox::RealBox(int s, float a) : m_boxsize(s), m_reals(new float[s])
{
   std::fill( m_reals, m_reals + s, a);
}

The std::fill and std::copy are defined in the <algorithm> header.


Once you fix this, the assignment operator is very simple:

#include <algorithm>
//...
RealBox& RealBox::operator =(RealBox rhs)
{
    std::swap(rhs.m_reals, m_reals);
    std::swap(rhs.m_boxsize, m_boxsize);
    return *this;
}

Usage of the copy/swap idiom makes this all possible. Simple as simple can be.

What is the copy-and-swap idiom?

Community
  • 1
  • 1
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
0

sorry for short answer ...

first add a member to save the table size inside the costructor

RealBox::RealBox(int s, float a)
{
 m_reals = new float[s];
 for (int i=0; i < s; i++)
 {
  m_reals[i] = a;
 }
 m_realCount = s;
}

then replace :

RealBox::RealBox(const RealBox& rhs)
{
 m_reals = new float[m_boxsize];
 *this = rhs.m_reals; // << this is a big mistake
}

by :

RealBox::RealBox(const RealBox& rhs)
{
 this->m_reals = new float[rhs.m_realCount];
 memcpy(this->m_reals,rhs.m_reals,rhs.m_realCount*sizeof(float));
 this->m_realCount = rhs.m_realCount;
}

You may need to include stdlib.h

Garcia Sylvain
  • 356
  • 4
  • 10
  • Your copy constructor is wrong. You're not allocating memory for `this->m_reals`, and you're not copying the right amount (`memcpy` takes byte count, not element count) – Benjamin Lindley Feb 27 '15 at 02:53