-4

I try to learn C++ basis in my free time and follow exercises of a book. Firstly, when I enter 9 as row and 8 as column I get malloc freeing error. Secondly, I get 0 as output I can't see what I enter. I want to write the program because I can reinforce rule of three. It's certainly not homework. Besides, I think it is hard question and efficient question. If the question can be answered, it will be helpful. Because I've searched on google I can't find out halfway decent thing about solution. Also, can you check my copy constructor, assignment operator and destructor and tell me where my errors at?

Write a class TwoD that implements the two-dimensional dynamic array of doubles using ideas from this display in its constructors. You should have a private member of type pointer to double to point to the dynamic array, and two int (or unsigned int) values that are MaxRows and MaxCols. You should supply a default constructor for which you are to choose a default maximum row and column sizes and a parameterized constructor that allows the programmer to set maximum row and column sizes. Further, you should provide a void member function that allows setting a particu- lar row and column entry and a member function that returns a particular row and column entry as a value of type double. Remark: It is difficult or impossible (depending on the details) to overload [ ] so it works as you would like for two-dimensional arrays. So simply use accessor and mutator functions using ordinary function notation. Overload the + operator as a friend function to add two two-dimensional arrays. This function should return the TwoD object whose ith row, jth column element is the sum of the ith row, jth column element of the left-hand operand TwoD object and the ith row, jth column element of the right-hand operand TwoD object. Provide a copy constructor, an overloaded operator =, and a destructor. Declare class member functions that do not change the data as const members.

My effort

#include <iostream>
using std::cin;
using std::cout;
using std::endl;

class TwoD
{
public:
    TwoD();
    TwoD(int row, int column);
    void setRowCol();
    double getVal(int row, int column);
    friend const TwoD operator +(const TwoD& first, const TwoD& second);
    int getterRow() const;
    int getterCol() const;
    void setterRow(int row);
    void setterCol(int column);
    TwoD(const TwoD& object);
    TwoD& operator =(const TwoD& rightSide);
    void putArr() const;
    ~TwoD();
    static TwoD constructFromUserInput();
private:
    int MaxRows;
    int MaxCols;
    double **arr;
};

int main(int argc, char const *argv[])
{
    cout << "All size of TwoD object must be same\n\n";

    TwoD arr1 = TwoD::constructFromUserInput();
    TwoD arr2 = TwoD::constructFromUserInput();

    TwoD arr3;

    arr3 = arr1 + arr2;

    TwoD arr4(arr3);
    arr1.putArr();
    arr2.putArr();
    arr3.putArr();
    arr4.putArr();

    return 0;
}
void TwoD::setRowCol()
{
    int r_user;
    int c_user;
    cout << "Enter num of rows => ";
    cin  >> r_user;
    cout << "Enter num of columns => ";
    cin  >> c_user;

    MaxRows = r_user;
    MaxCols = c_user;

    TwoD(MaxRows,MaxCols);

}
TwoD::TwoD(int row, int column)
: MaxRows(row), MaxCols(column)
{
    arr = new double*[row];
    for (int i = 0; i < row; i++)
    {
        arr[i] = new double[column];
    }

    for (int i = 0; i < MaxRows; i++)
    {
        for (int j = 0; j < MaxCols; j++)
        {
            cout << "Enter for " << i << j << "=> ";
            cin  >> arr[i][j];
        }
    }
}
TwoD::TwoD()
: MaxRows(2), MaxCols(2)
{
    arr = new double*[2];
    for (int i = 0; i < 2; i++)
    {
        arr[i] = new double[2];
    }
}
double TwoD::getVal(int row, int column)
{
    return arr[row][column];
}
const TwoD operator +(const TwoD& first, const TwoD& second)
{
    TwoD sum;
    for (int i = 0; i < first.MaxRows; i++)
    {
        for (int j = 0; j < first.MaxCols; j++)
        {
            sum.arr[i][j] = first.arr[i][j] + second.arr[i][j];
        }
    }
    return sum;
}
TwoD::TwoD(const TwoD& object)
{
    MaxRows = object.MaxRows;
    MaxCols = object.MaxCols;

    arr = new double*[MaxRows];
    for (int i = 0; i < MaxRows; i++)
    {
        arr[i] = new double[MaxCols];
    }

    for ( int i = 0; i < MaxRows; i++ )
    {
        for ( int j = 0; j < MaxCols; j++ )
        {
            arr[i][j] = object.arr[i][j];
        }
    }
}
TwoD::~TwoD()
{
    for (int i = 0; i < MaxRows; i++)
        delete [] arr[i];
    delete [] arr;
}
TwoD& TwoD::operator =(const TwoD& rightSide)
{
    if (this == &rightSide)
    {
        return *this;
    }

    for (int i = 0; i < MaxRows; i++)
        delete [] arr[i];
    delete [] arr;

    arr = new double*[rightSide.MaxRows];
    for (int i = 0; i < rightSide.MaxRows; i++)
    {
        arr[i] = new double[rightSide.MaxCols];
    }

    MaxRows = rightSide.MaxRows;
    MaxCols = rightSide.MaxCols;

    for (int i = 0; i < MaxRows; i++)
    {
        for (int j = 0; j < MaxCols; j++)
        {
            arr[i][j] = rightSide.arr[i][j];
        }
    }
    return *this;
}
void TwoD::putArr() const
{
    for (int i = 0; i < MaxRows; i++)
    {
        for (int j = 0; j < MaxCols; j++)
        {
            cout << arr[i][j] << " ";
        }
        cout << endl;
    }
}
int TwoD::getterRow() const
{
    return MaxRows;
}
int TwoD::getterCol() const
{
    return MaxCols;
}
void TwoD::setterRow(int row)
{
    MaxRows = row;
}
void TwoD::setterCol(int column)
{
    MaxCols = column;
}
TwoD TwoD::constructFromUserInput()
{
    int r_user;
    int c_user;
    cout << "Enter num of rows => ";
    cin  >> r_user;
    cout << "Enter num of columns => ";
    cin  >> c_user;

    // Construct an object.
    TwoD obj(r_user, c_user);

    // Return the object
    return obj;
}
newbie
  • 31
  • 5
  • 5
    We already have a good Q&A about [how the rule of 3](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) is implemented and applied correctly. – πάντα ῥεῖ Sep 02 '15 at 16:54

2 Answers2

2
  1. your copy constructor creates the arrays, but never copies the contents. The copy assignment operator does this correctly. For that matter, you have loads of duplicated new[] loops, a couple of delete[] loops, and you should have two copy loops. Factor these out into functions, and you only need to get them right once.

  2. sum creates a default-sized object and then, probably, overruns both row and column dimensions. The usual (easy and correct) implementation is to create a local copy of the left-hand argument (using the copy ctor you fixed above) and forward to operator+=. That operator is much easier to get right.

  3. you have a way of printing these matrices out. Do it! Do it everywhere! This makes it easy to see which stage things went wrong at.

  4. this book doesn't sound very good. It's teaching poor practice for no obvious reason (the operator+ trick is standard, you should use unique_ptr if you can't use vector, etc. etc.)


Edits: I'll reply to some comments here because they're in danger of turning into a chat session, and I don't want to spend more time on this.

  • I have fixed copy constructor is it right?

    This is a good time to figure out how to test and debug code. Add print statements, use a debugger, split things out into functions you can test and then write tests for them.

  • what you mean a couple of delete[] loops and having two copy loops

    Look for loops in your code. I can see 3 loops calling new[], all doing essentially the same thing. I can see 2 loops calling delete[], one in the destructor and one in the copy assignment operator. Just search your own code for the for keyword!

  • I really don't find a way to print out matrices

    so where did TwoD::putArr come from? Admittedly ostream& operator<<(ostream&, TwoD const &) would be a better name for it, but you wrote it so you might as well use it.

  • I can't get destructor into main()

    you can't stop the destructor being called when main finishes - your objects go out of scope then. Just stick a breakpoint in the destructor and it will be called.

  • Because arr is private how can I test?

    you can still see it in the debugger, you can still print the contents (as above), you can still call getVal from a unit test, you can just make it public until you've figured out your problem(s)

Useless
  • 64,155
  • 6
  • 88
  • 132
  • which book you advise? It's absolute c++ walter savitch – newbie Sep 02 '15 at 17:08
  • I'm afraid book recommendations are OT, and anyway I wouldn't know what level to pitch. Just start with the rule of 3 link in the comment, and do your own research. – Useless Sep 02 '15 at 17:16
  • for first specification I have fixed copy constructor is it right ? also one thing I don't understand what you mean a couple of `delete[]` loops and having two copy loops. Would you mind explaining? I have destructor already. – newbie Sep 02 '15 at 17:34
  • You tear down the arrays in your dtor and in the assignment operator, duplicating code. Move it to a function and call from both places. Same for allocating the arrays, same for copying values. – Useless Sep 02 '15 at 17:36
  • Oh, and _is it right?_ Test it! Print the result, step through in a debugger, see if it does the right thing. – Useless Sep 02 '15 at 17:38
  • and I really don't find a way to print out matrices likewise calling in constructor – newbie Sep 02 '15 at 17:38
  • I can't get destructor into main(). Because arr is private how can I test ? and I can't test the copy constructor because the filling the array is problem. I filled but shows result as 0 – newbie Sep 02 '15 at 17:42
0

Problem with your code:

void TwoD::setRowCol()
{
    int r_user;
    int c_user;
    cout << "Enter num of rows => ";
    cin  >> r_user;
    cout << "Enter num of columns => ";
    cin  >> c_user;

    MaxRows = r_user;
    MaxCols = c_user;

    ///
    /// This constructs a new TwoD object but does not change
    /// object on which the function was invoked.
    ///
    TwoD(MaxRows,MaxCols);    
}

Ways to solve the problem:

Update the member data

void TwoD::setRowCol()
{
    int r_user;
    int c_user;
    cout << "Enter num of rows => ";
    cin  >> r_user;
    cout << "Enter num of columns => ";
    cin  >> c_user;

    // Delete the current memory
    for (int i = 0; i < MaxRows; i++)
    {
       delete [] arr[i];
    }

    delete [] arr;

    MaxRows = r_user;
    MaxCols = c_user;

    // Allocate new memory.
    arr = new double*[MaxRows];
    for (int i = 0; i < MaxRows; i++)
    {
       arr[i] = new double[MaxCols];
    }

    // Now read the data.
    for (int i = 0; i < MaxRows; i++)
    {
        for (int j = 0; j < MaxCols; j++)
        {
            cout << "Enter for " << i << j << "=> ";
            cin  >> arr[i][j];
        }
    }
}

Create a function that returns a brand new object

I recommend this method.

First, update the constructor that takes the row and the column so that it does not have any code to read data from user input.

TwoD::TwoD(int row, int column)
: MaxRows(row), MaxCols(column)
{
    arr = new double*[row];
    for (int i = 0; i < row; i++)
    {
        arr[i] = new double[column];
    }
}

Second, add a static member function to construct an object from user input.

static TwoD constructFromUserInput();

and implement it as:

TwoD TwoD::constructFromUserInput()
{
    int r_user;
    int c_user;
    cout << "Enter num of rows => ";
    cin  >> r_user;
    cout << "Enter num of columns => ";
    cin  >> c_user;

    // Construct an object.
    TwoD obj(r_user, c_user);

    // Now read the data.
    for (int i = 0; i < r_user; i++)
    {
        for (int j = 0; j < c_user; j++)
        {
            cout << "Enter for " << i << j << "=> ";
            cin  >> obj.arr[i][j];
        }
    }

    // Return the object
    return obj;
}

Third, use the new function from main.

int main(int argc, char const *argv[])
{
   cout << "All size of TwoD object must be same\n\n";

   TwoD arr1 = TwoD::constructFromUserInput();
   TwoD arr2 = TwoD::constructFromUserInput();

   TwoD arr3;

   arr3 = arr1 + arr2;
   arr1.putArr();
   arr2.putArr();
   arr3.putArr();

   return 0;
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • thank you after applying your advised way, I have also tried to copy constructor I get memory error. `TwoD arr4(arr3);`. Would you mind helping ? I updated the code. – newbie Sep 02 '15 at 19:08
  • @newbie, The line `for ( int i = 0; i < MaxRows + 1; i++ )` is wrong. You end up using the array out of bounds. It needs to be `for ( int i = 0; i < MaxRows; i++ )`. – R Sahu Sep 02 '15 at 19:27
  • @newbie, Trying to debug your code with comments and responses is not very productive. Put `cout` statements liberally in your code to track down the problem(s); – R Sahu Sep 02 '15 at 20:01
  • I have used and get error in `const TwoD operator +(const TwoD& first, const TwoD& second)`. I can't find. If there is paradise on the world, it is Hındistane. – newbie Sep 02 '15 at 20:10