Register FAQ / Rules Forum Spy Search Today's Posts Mark Forums Read
Go Back   MacRumors Forums > Apple Systems and Services > Programming > Mac Programming

Reply
 
Thread Tools Search this Thread Display Modes
Old Apr 16, 2011, 11:55 PM   #1
quanganhct
macrumors member
 
Join Date: Dec 2010
C++ new statement. Help me

Hi,
Apparently I write a little code which use "new" inside of my function, but it appear that it do not work well.
Code:
#include "iostream"
#include "cstdlib"
#include "iomanip"

using namespace std;

void inputMatrix(double **M,int& line,int& col){
	cout << "Input number of lines and columns : ";
	cin>>line>>col;
	
	M=new double*[line];
	for (int i=0; i<line; i++) {
		M[i]=new double[col];
	}
	
	cout << "Input your matrix :"<<endl;
	for (int i=0; i<line; i++)
		for (int j=0; j<col; j++) cin>>M[i][j];
	
}

void killMatrix(double **M,int line,int col){
	cout<<"DD";
	for (int i=0; i<line; i++) delete[] M[i];
	delete[] M;
}

int main(){
	int line1,col1,line2,col2;
	double **M1,**M2,**result;
	
	inputMatrix(M1,line1,col1);
	cout<<line1<<" "<<col1<<endl;
	cout<<M1[0][1]<<endl;
//	inputMatrix(M2,line2,col2);
//	cout<<line2<<" "<<col2<<endl;
//	result=add2Matrix(M1,M2,line1,col1,line2,col2);
//	outputMatrix(result,line1,col1);
	killMatrix(M1,line1,col1);
//	killMatrix(M2,line2,col2);
//	killMatrix(result,line1,col1);
	
	return 0;
	
}
and it return "Segmentation fault"
I guess "new" do not work properly.
Does anyone have any ideas ?

Thanks in advance.
quanganhct is offline   0 Reply With Quote
Old Apr 17, 2011, 02:04 AM   #2
jiminaus
macrumors 65816
 
Join Date: Dec 2010
Location: Sydney
You need extra level of indirection for M in inputMatrix.
Code:
void inputMatrix(double ***M,int& line,int& col){
	cout << "Input number of lines and columns : ";
	cin>>line>>col;
	
	(*M)=new double*[line];
	for (int i=0; i<line; i++) {
		(*M)[i]=new double[col];
	}
	
	cout << "Input your matrix :"<<endl;
	for (int i=0; i<line; i++)
		for (int j=0; j<col; j++) cin>>(*M)[i][j];
	
}
And then in main
Code:
	inputMatrix(&M1,line1,col1);

Or alternatively, don't use an out-style parameter for M, instead pass it back as the result.

Code:
double **inputMatrix(int& line,int& col){
	cout << "Input number of lines and columns : ";
	cin>>line>>col;
	
	double **M=new double*[line];
	for (int i=0; i<line; i++) {
		M[i]=new double[col];
	}
	
	cout << "Input your matrix :"<<endl;
	for (int i=0; i<line; i++)
		for (int j=0; j<col; j++) cin>>M[i][j];
	
        return M;
}
And then in main:
Code:
	M1=inputMatrix(line1,col1);

But (assuming you're just using C++ as a better C) you should create a matrix struct that binds the pointer, col, and line together.
Code:
#include <iostream>
#include <cstdlib>
#include <iomanip>

using namespace std;

struct matrix {
    double **values;
    int line;
    int col;
};

matrix inputMatrix(){
    matrix m;
    
    cout << "Input number of lines and columns : ";
    cin>>m.line>>m.col;
	
    m.values =new double*[m.line];
    for (int i=0; i<m.line; i++) {
        m.values[i]=new double[m.col];
    }
	
    cout << "Input your matrix :"<<endl;
    for (int i=0; i<m.line; i++)
        for (int j=0; j<m.col; j++) cin>>m.values[i][j];
    
    return m;
}

void outputMatrix(const matrix &m) {
    for (int i=0; i<m.line; i++) {
        for (int j=0; j<m.col; j++)
            cout << m.values[i][j] << ' ';
        cout << endl;
    }
}

void killMatrix(matrix &m){
    for (int i=0; i<m.line; i++) delete[] m.values[i];
    delete[] m.values;
    m.values = NULL;
}

int main(){
    matrix M1, M2, result;
	
    M1 = inputMatrix();
    // cout<<m1.line<<" "<<m1.col<<endl;
    // cout<<m1.values[0][1]<<endl;
    //	M2 = inputMatrix();
    //	cout<<m2.line<<" "<<m2.col<<endl;
    //	result=add2Matrix(M1,M2);
    outputMatrix(M1);
    killMatrix(M1);
    //	killMatrix(M2);
    //	killMatrix(result);
	
    return 0;	
}
But, surely there are ready-made, tested and optimised matrix implementations out there. For example, there's Robert Davies' Newmat C++ matrix library which can be downloaded from http://www.robertnz.net/download.html.

Last edited by jiminaus; Apr 17, 2011 at 02:13 AM.
jiminaus is offline   0 Reply With Quote
Old Apr 17, 2011, 02:19 AM   #3
gnasher729
macrumors G5
 
gnasher729's Avatar
 
Join Date: Nov 2005
Quote:
Originally Posted by quanganhct View Post
Hi,
Apparently I write a little code which use "new" inside of my function, but it appear that it do not work well.
In that function, where do you use the value M that is passed in? You just overwrite its value. That looks very, very suspicious. The value of M1 never gets defined.
gnasher729 is offline   0 Reply With Quote
Old Apr 17, 2011, 06:39 AM   #4
Sander
macrumors 6502
 
Join Date: Apr 2008
Quote:
Originally Posted by jiminaus View Post
But (assuming you're just using C++ as a better C) you should create a matrix struct that binds the pointer, col, and line together.
Hear hear. I see this all the time, but a program doesn't progress from C to C++ by changing its file extension from .c to .cpp and changing malloc/free into new/delete. I guess C++ being compatible with C is both a blessing and a curse...

To the OP: I assume you are writing this matrix stuff as a learning exercise. It's a popular exercise to learn OO. You should make your matrix class "self-contained", and have it clean up after itself. You will run into a bunch of problems on the way: How do you copy matrices? Do you copy its contents? Do you hand out references instead? Who is then responsible for cleaning it up, and how do you prevent other code from accessing zombie matrices?
Sander is offline   0 Reply With Quote
Old Apr 17, 2011, 07:03 AM   #5
jiminaus
macrumors 65816
 
Join Date: Dec 2010
Location: Sydney
Quote:
Originally Posted by Sander View Post
Hear hear. I see this all the time, but a program doesn't progress from C to C++ by changing its file extension from .c to .cpp and changing malloc/free into new/delete. I guess C++ being compatible with C is both a blessing and a curse...
Actually it does precisely by changing .c to .cpp or gcc to g++. I can't remember in which book I learnt this phrase, but "C++ as a better C" is (or was before C99) a legitimate usage of C++. A C++ program does not have to be object-oriented to be a legitimate use of C++.

If this is an OOP exercise, than the OP is totally failing. Actually I picked it up as being a FORTRAN port. But maybe that was just because of the by-reference function arguments.
jiminaus is offline   0 Reply With Quote
Old Apr 17, 2011, 08:16 AM   #6
quanganhct
Thread Starter
macrumors member
 
Join Date: Dec 2010
well in fact I haven't learnt about OPP yet. I wrote this program 'cause I just had a lesson about Matrix.

anyway, thanks alot . I'll test it out and report later.
quanganhct is offline   0 Reply With Quote
Old Apr 17, 2011, 02:01 PM   #7
SidBala
macrumors 6502a
 
Join Date: Jun 2010
Always use a single dimension array for rectangular multidimensional arrays. It is so much easier, faster and error-free.
SidBala is offline   0 Reply With Quote
Old Apr 18, 2011, 02:34 AM   #8
Sander
macrumors 6502
 
Join Date: Apr 2008
Quote:
Originally Posted by jiminaus View Post
Actually it does precisely by changing .c to .cpp or gcc to g++. I can't remember in which book I learnt this phrase, but "C++ as a better C" is (or was before C99) a legitimate usage of C++. A C++ program does not have to be object-oriented to be a legitimate use of C++.
Of course you are right, jiminaus. Perhaps I should have written "good C++ program" or (to be more politically correct) "idiomatic C++ program".

What I was trying to get at is that when people struggle with a problem in C which is easier solved in C++, it's not going to be by substituting "new" for "malloc"
Sander is offline   0 Reply With Quote
Old Apr 18, 2011, 10:20 AM   #9
quanganhct
Thread Starter
macrumors member
 
Join Date: Dec 2010
OK, here I go.
Code:
/*
 *  matrix.cpp
 *  
 *
 *  Created by NGUYEN Quang Anh on 4/17/11.
 *  Copyright 2011 __MyCompanyName__. All rights reserved.
 *
 */

#include "iostream"
#include "cstdlib"
#include "iomanip"
#include "exception"
using namespace std;
struct matrix {
	double **value;
	int line;
	int col;
};

matrix inputMatrix(int& line,int& col){
	cout << "Input number of lines and columns : ";
	cin>>line>>col;
	matrix M;
	M.value=new double*[line];
	for (int i=0; i<line; i++) {
		M.value[i]=new double[col];
	}
	
	cout << "Input your matrix :"<<endl;
	for (int i=0; i<line; i++)
		for (int j=0; j<col; j++) cin>>M.value[i][j];
	M.line=line;
	M.col=col;
	return M;
	
}

void killMatrix(matrix M){
	for (int i=0; i<M.line; i++) delete[] M.value[i];
	delete[] M.value;
}

void outputMatrix(matrix M){
	for (int i=0; i<M.line; i++) {
		for (int j=0; j<M.col; j++) cout << setw(3) << M.value[i][j] ;
		cout << endl;
	}
}

matrix add2Matrix(matrix M1,matrix M2){
	bool notCorrect=((M1.line != M2.line)||(M1.col != M2.col)); 
	if (notCorrect) throw 10;
	
	
	matrix result;
	result.line=M1.line;
	result.col=M1.col;
	result.value=new double*[result.line];
	
	for (int i=0; i<result.line; i++) {
		result.value[i]=new double[result.col];
	}
	
	for (int i=0; i<result.line; i++) 
		for (int j=0; j<result.col; j++)
			result.value[i][j]=M1.value[i][j]+M2.value[i][j];
		
	return result;
}

matrix multi2Matrix(matrix M1,matrix M2){
	bool notCorrect=(M1.col != M2.line);
	if (notCorrect) throw 10;
	
	matrix result;
	result.line=M1.line;
	result.col=M2.col;
	
	result.value=new double*[result.line];
	for (int i=0; i<result.line; i++) {
		result.value[i]=new double[result.col];
	}
	
	for (int i=0; i<result.line; i++) {
		for (int j=0; j<result.col; j++) {
			result.value[i][j]=0;
			for (int k=0; k<M1.col; k++) result.value[i][j]+=M1.value[i][k]*M2.value[k][j];
		}
	}
	
	return result;
}

int main(){
	int line,col;
	matrix M1,M2,result;
	
	M1=inputMatrix(line,col);
	M2=inputMatrix(line,col);
	try {
	result=multi2Matrix(M1,M2);
	cout << "Your Result :"<< endl;
	outputMatrix(result);
	killMatrix(M1);
	killMatrix(M2);
	killMatrix(result);
		
	}
	catch (int i){
		if (i==10) cerr << "Cannot execute operation" <<endl;
		killMatrix(M1);
		killMatrix(M2);

	}
	
	return 0;
	
}
It went pretty well. Thanks a lot guys Now I try to write a function calculate matrix inverse
quanganhct is offline   0 Reply With Quote
Old Apr 18, 2011, 01:49 PM   #10
flashy-cat
macrumors member
 
Join Date: Apr 2007
Location: UK
Using more than 2 levels of indirection is poor practice and asking for trouble. It's difficult to read and error prone.
flashy-cat is offline   0 Reply With Quote
Old Apr 18, 2011, 05:55 PM   #11
jiminaus
macrumors 65816
 
Join Date: Dec 2010
Location: Sydney
Quote:
Originally Posted by flashy-cat View Post
Using more than 2 levels of indirection is poor practice and asking for trouble. It's difficult to read and error prone.
Agreed, I had to diagram it out. Actually I wanted to pass a reference to a the double pointer, but I couldn't get the syntax for that. I didn't try very hard, because the out-style 2d array parameter wasn't a good idea to begin with.


Quote:
Originally Posted by quanganhct View Post
OK, here I go.

It went pretty well. Thanks a lot guys Now I try to write a function calculate matrix inverse
quanganhct you should refactor your code by isolating out the code that allocates a matrix.

Let me show you how your example would look in object-oriented C++ that sandar was advocating.

Code:
#include <iostream>
#include <cstdlib>
#include <iomanip>
#include <exception>

using namespace std;


class matrix {
    
public:
    
    static matrix inputMatrix();
    
    matrix(int aLine, int aCol); // Constructor
    matrix(const matrix& aSrcM); // Copy constructor
   ~matrix(); // Destructor
    
    int line() const { return theLine; }
    int col() const { return theCol; }
    
    double get(int aLine, int aCol) const;
    void set(int aLine, int aCol, double aValue);
    
private:
    
    double** theValue;
    int theLine;
    int theCol;

};

matrix operator +(const matrix& M1, const matrix& M2);
matrix operator *(const matrix& M1, const matrix& M2);
ostream& operator <<(ostream& os, const matrix& M);


matrix::matrix(int aLine, int aCol)
{
    // TODO: Bounds-check aLine and aCol
    theLine = aLine;
    theCol = aCol;
    theValue = new double*[theLine];
    for (int i=0; i<theLine; i++) {
        theValue[i] = new double[theCol];
    }
}


matrix::matrix(const matrix& M)
{
    theLine = M.line();
    theCol = M.col();
    theValue = new double *[theLine];
    for (int i=0; i<theLine; i++) {
        theValue[i] = new double[theCol];
        // TODO: Replace inner loop with memcpy
        for (int j=0; j<theCol; j++) {
            set(i, j, M.get(i, j));
        }
    }
}


matrix::~matrix()
{
    for (int i=0; i<theLine; i++) delete[] theValue[i];
    delete[] theValue;
}


matrix matrix::inputMatrix()
{
    int line, col;
    
    cout << "Input number of lines and columns : ";
    cin >> line >> col;
    // TODO: Check line and col are valid
    
    matrix M(line, col);
    
    cout << "Input your matrix :"<<endl;
    double v;
    for (int i=0; i<line; i++) {
        for (int j=0; j<col; j++) {
            cin >> v;
            M.set(i, j, v);
        }
    }
    return M;
}


ostream& operator <<(ostream& os, const matrix& M)
{
    for (int i=0; i<M.line(); i++) {
        for (int j=0; j<M.col(); j++) os << setw(3) << M.get(i,j);
        os << endl;
    }
    return os;
}


matrix operator +(const matrix& M1, const matrix& M2)
{
    bool notCorrect=((M1.line() != M2.line())||(M1.col() != M2.col()));
    // TODO: Replace throwing an int with throwing a std::exception
    // or a custom subcalss of it
    if (notCorrect) throw 10;
    
    
    matrix result(M1.line(), M1.col()); 
    for (int i=0; i<result.line(); i++) 
        for (int j=0; j<result.col(); j++)
            result.set(i, j, M1.get(i,j)+M2.get(i,j));
    
    return result;
}


matrix operator *(const matrix& M1, const matrix& M2)
{
    bool notCorrect=(M1.col != M2.line);
    // TODO: Replace throwing an int with throwing a std::exception
    // or a custom subcalss of it
    if (notCorrect) throw 10;
    
    matrix result(M1.line(), M2.col());
    
    for (int i=0; i<result.line(); i++) {
        for (int j=0; j<result.col(); j++) {
            result.set(i,j,0);
            for (int k=0; k<M1.col(); k++) {
                result.set(i, j, result.get(i,j) + M1.get(i,k)*M2.get(k,j));
            }
        }
    }
    
    return result;
}


int main()
{
    matrix M1 = matrix::inputMatrix();
    matrix M2 = matrix::inputMatrix();

    try {
        matrix result = M1 * M2;
        cout << "Your Result :" << endl
             << result;        
        // result will be automatically destructed by compiler at this point
    }
    // TODO: Replace throwing an int with throwing a std::exception
    // or a custom subcalss of it
    catch (int i){
        if (i==10) cerr << "Cannot execute operation" <<endl;        
    }
    
    return 0;
    
    // M1 and M2 will be automatically destructed by compiler at this point
}
It's been a while since I've coded C++. It's nice to feel the raw power in my fingers again.
jiminaus is offline   0 Reply With Quote
Old Apr 19, 2011, 11:14 AM   #12
quanganhct
Thread Starter
macrumors member
 
Join Date: Dec 2010
Quote:
Originally Posted by flashy-cat View Post
Using more than 2 levels of indirection is poor practice and asking for trouble. It's difficult to read and error prone.
It seems I ask silly question, but hey, what is "level of indirection", and "where did i use more than 2 levels of indirection ?

you mean 2 dimension array ?
quanganhct is offline   0 Reply With Quote
Old Apr 19, 2011, 03:33 PM   #13
jiminaus
macrumors 65816
 
Join Date: Dec 2010
Location: Sydney
Quote:
Originally Posted by quanganhct View Post
It seems I ask silly question, but hey, what is "level of indirection", and "where did i use more than 2 levels of indirection ?

you mean 2 dimension array ?
Levels of indirection is the number pointers you need to "hop" to get to the value itself.

In C, the number of stars in a type is the level of indirection. So int* has 1 level of indirection to the int, int** has 2 levels of indirection to the int, int*** had 3 level of indirection to the int, etc.

You didn't use more than 2 levels of indirection. You do use 2 levels of indirection for the 2 dimensional array.

The first solution I posted for inputMatrix increased the levels of indirection to 3, so that it could pass back the pointer to the 2 dimensional array which it allocates inside itself. The 2 dimensional array is double**, so a pointer to that is double***, which has 3 levels of indirection.
jiminaus is offline   0 Reply With Quote
Old Apr 20, 2011, 08:33 AM   #14
quanganhct
Thread Starter
macrumors member
 
Join Date: Dec 2010
Code:
    double get(int aLine, int aCol) const;
    void set(int aLine, int aCol, double aValue);
Could you please explain this 2 functions to me ? I didn't find where you defined these 2 functions.

Thanks alot
quanganhct is offline   0 Reply With Quote
Old Apr 20, 2011, 08:49 AM   #15
lee1210
macrumors 68040
 
lee1210's Avatar
 
Join Date: Jan 2005
Location: Dallas, TX
Quote:
Originally Posted by quanganhct View Post
Code:
    double get(int aLine, int aCol) const;
    void set(int aLine, int aCol, double aValue);
Could you please explain this 2 functions to me ? I didn't find where you defined these 2 functions.

Thanks alot
It doesn't look like those were included, but surely you can work out what the implementation should be? The data store in matrix is theValue, which is a double**. Surely given a line and column you could work out what double should be returned, and given a line, column, and double value you can work out what to set to that double value. In both cases you'd want to compare to theLine and theCol to make sure you're not over or understepping a dimension, but that should be pretty easy.

-Lee
lee1210 is offline   0 Reply With Quote
Old Apr 20, 2011, 09:13 AM   #16
quanganhct
Thread Starter
macrumors member
 
Join Date: Dec 2010
Quote:
Originally Posted by lee1210 View Post
It doesn't look like those were included, but surely you can work out what the implementation should be? The data store in matrix is theValue, which is a double**. Surely given a line and column you could work out what double should be returned, and given a line, column, and double value you can work out what to set to that double value. In both cases you'd want to compare to theLine and theCol to make sure you're not over or understepping a dimension, but that should be pretty easy.

-Lee
Well, i ask that question, 'cause I don't know for sure if he missed the definition of those 2 functions. I did do some searching on internet before asking, and I found out that "get" and "set" are key-words. And if those "get" and "set" are in this context , then I'm beat, and I need some explanations.

Oh ya, and another question :
Code:
matrix::matrix(const matrix& M)
{
    theLine = M.line();
    theCol = M.col();
    theValue = new double *[theLine];
    for (int i=0; i<theLine; i++) {
        theValue[i] = new double[theCol];
        // TODO: Replace inner loop with memcpy
        for (int j=0; j<theCol; j++) {
            set(i, j, M.get(i, j));
        }
    }
}
What does that comment mean ? 'cause when I search for "memcpy", there aren't anything like your syntax

http://www.cplusplus.com/reference/c...string/memcpy/

Sorry for asking lot of questions It can't be help since I'm a totally newbie. I just learned about C++ 6 months ago, and since I want to learn it myself, alot of new things appear in front of me and I'm speechless lol :d
quanganhct is offline   0 Reply With Quote
Old Apr 20, 2011, 10:19 AM   #17
lee1210
macrumors 68040
 
lee1210's Avatar
 
Join Date: Jan 2005
Location: Dallas, TX
Quote:
Originally Posted by quanganhct View Post
Well, i ask that question, 'cause I don't know for sure if he missed the definition of those 2 functions. I did do some searching on internet before asking, and I found out that "get" and "set" are key-words. And if those "get" and "set" are in this context , then I'm beat, and I need some explanations.
"get" and "set" are not C++ keywords. Where did you read that? Wherever it is, either you misunderstood so you should re-read, or it is wrong. There is a STL container called "set", but this isn't a reserved word.

Quote:
Originally Posted by quanganhct View Post
Oh ya, and another question :
Code:
matrix::matrix(const matrix& M)
{
    theLine = M.line();
    theCol = M.col();
    theValue = new double *[theLine];
    for (int i=0; i<theLine; i++) {
        theValue[i] = new double[theCol];
        // TODO: Replace inner loop with memcpy
        for (int j=0; j<theCol; j++) {
            set(i, j, M.get(i, j));
        }
    }
}
What does that comment mean ? 'cause when I search for "memcpy", there aren't anything like your syntax
It wouldn't be like this syntax, the comment is that this syntax should be replaced with that usage instead. Instead of looping and using get to grab a double at a time, you'd have, say, a double *getRow(int rowNum) method that would return the address of one row, then you'd:
Code:
double *sourceRow = M.getRow(i);
memcpy(theValue[i],sourceRow,theCol*sizeof(double));
This would be much faster than looping to grab a bunch of contiguous bytes of memory sizeof(double) bytes at a time, then sticking that value somewhere else over and over again. Using memcpy takes advantage of our data all being contiguous in memory and allows a faster copy in larger chunks than a single double at a time.

-Lee
lee1210 is offline   0 Reply With Quote
Old Apr 20, 2011, 03:35 PM   #18
jiminaus
macrumors 65816
 
Join Date: Dec 2010
Location: Sydney
Quote:
Originally Posted by quanganhct View Post
Code:
    double get(int aLine, int aCol) const;
    void set(int aLine, int aCol, double aValue);
Could you please explain this 2 functions to me ? I didn't find where you defined these 2 functions.

Thanks alot
Sorry, I did forget them. They were defined inline in the class at one stage. I removed them from there intending to defined them later with TODO comments about bounds checking the parameters.
jiminaus is offline   0 Reply With Quote

Reply
MacRumors Forums > Apple Systems and Services > Programming > Mac Programming

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Similar Threads
thread Thread Starter Forum Replies Last Post
If-else statement not evaluating alexatwater iPhone/iPad Programming 2 Mar 13, 2014 01:38 PM
applescript if statement wild4life2013 Mac Programming 2 Aug 5, 2013 11:16 PM
Why doesnt the else statement work in this? bigMAC28 Mac Programming 2 Aug 4, 2013 04:44 PM
Can someone explain this statement DMB15 iPhone 6 Sep 12, 2012 05:41 PM

Forum Jump

All times are GMT -5. The time now is 02:25 AM.

Mac Rumors | Mac | iPhone | iPhone Game Reviews | iPhone Apps

Mobile Version | Fixed | Fluid | Fluid HD
Copyright 2002-2013, MacRumors.com, LLC