On 9 Aug 2004 19:44:26 -0400, ***@yahoo.com (KK) wrote:
This looks like a homework type question. Regardless...
since you could use some tips...
1. It is customary to prefix member variables with either m_, _, or
suffix them with an underscore.
2. The default ctor is ill-formed. It doesn't initialize the size, or
the pointer of elements.
3. There is a typo in return type of the the assignment operator.
The declaration returns a temporary, yet you are returning a reference
in the definition.
4. "register" is archaic. All modern C++ compilers make efficient use
of all CPU registers.
5. Transpose tries to assign elements, but does so without
either referring to elements, or using a overloaded operator() to do
it.
6. rows, and cols, are accessible publically, yet the size of the 2d
matrix depends on them. They should be made with getters/setters, so
that the element array can be re-dimensioned if they change.
7. Tradionally, lhs, and rhs, are used for the names of functions that
deal with unary, and binary operations (such as assignment or copy) on
the class.
8. Before writing template code, it is best to write (& debug!) it as
non-templatized. Once it is working, THEN convert it to a template.
This is because if you start off with templatized code, it still may
have compile errors if you never reference it. The non-templatized
version will have the compiler flag ALL errors.
9. Extra whitespace never hurts!!
10. When you see common operations throughout your codebase, create
functions for them. The point of programming is to create, manage,
and use BOTH the micro, and macro.
11. Did you intentionally want Transpose NOT to be a member function?
Since it deals with the internals it should be a member function (or a
friend). Furthermore, this type of operation is natural to have 2
ways of being called, since that would be helpfull to users of your
class.
12. Learn to read (& understand) other people's code -- you'll become
a better writer in the process. Everyone has their own coding style,
but I recommend you seperate your functions via comments, at the very
least.
13. I would recommend a few C++ books.
- The C++ Programming Language (Third Edition), by Stroustrup
- Thinking in C++, by Bruce Eckel
- All books by Scott Meyers, and Andrei Alexandrescu
- ARM (although quite old, it is still very usefull)
Here is the code with all the changes...
As a bonus, I've included the ability to compile it as as template, or
non-template code, so you can see how to minimize your conversions in
the future.
Cheers
// Matrix N x M _____________________________________________________
#include <iostream>
using std::cout;
// Uncomment for template version.
// Comment out for non-template version.
#define TEMPLATE
#ifdef TEMPLATE
#define TEMP_DEF template <typename TYPE = int>
#define TEMP template <typename TYPE>
#define SELF matrix<TYPE>
#else
#define TEMP_DEF
#define TEMP
#define SELF matrix
// Template Parameter/Argument name must match TEMP_DEF and
TEMP
#define TYPE double
#endif
TEMP_DEF
class matrix
{
int _nRows;
int _nCols;
TYPE **_aElement;
typedef TYPE by_value ;
typedef TYPE& by_ref ;
void _Create(const int & nRows, const int & nCols, const TYPE&
nFill )
{
_nRows = nRows;
_nCols = nCols;
_aElement = new TYPE* [nRows];
for( int y = 0; y < nRows; y++ )
{
_aElement[y] = new TYPE [_nCols];
for (int x = 0; x < nCols; x++ )
{
_aElement[y][x] = nFill;
}
}
}
// Elements are NOT initialized
void _Create(const int & nRows, const int & nCols )
{
_nRows = nRows;
_nCols = nCols;
_aElement = new TYPE* [nRows];
for( int y = 0; y < nRows; y++ )
{
_aElement[y] = new TYPE [_nCols];
}
}
void _Destroy()
{
for( int y = 0; y < _nRows; y++)
delete [] _aElement[y];
delete [] _aElement;
_aElement = NULL;
}
// assumes "this" is the same size as "rhs"
void _Copy(const matrix & rhs)
{
for( int y = 0; y < _nRows; y++ )
{
for (int x = 0; x < _nCols; x++ )
{
_aElement[y][x] = rhs._aElement[y][x];
}
}
}
public:
// ctor
matrix()
: _nRows( 0 )
, _nCols( 0 )
, _aElement( 0 )
{
}
matrix(const int& _nRows, const int& _nCols, const TYPE&
fillValue=0);
// copy-ctor
matrix(const matrix &rhs);
//dtor
~matrix();
// assignment
SELF& operator=(const SELF & rhs);
void Set( const int& x, const int & y, const TYPE& nNewElement
)
{
_aElement[y][x] = nNewElement;
}
TYPE& Get( const int& x, const int & y) const
{
return _aElement[y][x];
}
int GetRows() const
{
return _nRows;
}
int GetCols() const
{
return _nCols;
}
// Util - re-dimension
void SetRows( const int& nNewRows )
{
_Destroy();
_nRows = nNewRows;
_Create( _nRows, _nCols );
}
void SetCols( const int& nNewCols )
{
_Destroy();
_nCols = nNewCols;
_Create( _nRows, _nCols );
}
// Functionality
SELF& Transpose();
SELF& Transpose( const SELF & rhs );
// I/O
// TODO: overloading the output stream operator << is left as
an excersize for the reader.
void Print() const
{
for( int y = 0; y < _nRows; y++ )
{
for( int x = 0; x < (_nCols-1); x++ )
{
cout << _aElement[y][x] << ", ";
}
cout << _aElement[y][x] << "\n";
}
}
};
// ==================================================================
TEMP
SELF::matrix (const int& nRows, const int& nCols, const TYPE& nFill )
{
_Create( nRows, nCols, nFill );
}
// ==================================================================
TEMP
SELF::~matrix()
{
_Destroy();
}
// ==================================================================
TEMP
SELF::matrix(const SELF &rhs)
{
_Create( rhs._nRows, rhs._nCols );
_Copy( rhs );
}
// ==================================================================
TEMP
SELF& SELF::operator=(const SELF& rhs)
{
// are we not the same size as the rhs?
if ((_nRows != rhs._nRows) || (_nCols != rhs._nCols))
{
_Destroy();
_Create( rhs._nRows, rhs._nCols );
}
_Copy( rhs );
return *this;
}
// ==================================================================
TEMP
SELF& SELF::Transpose(const SELF & rhs)
{
_Destroy();
_Create( rhs._nCols, rhs._nRows ); // src ROWS & COLS
intentonally swapped
for( int y= -1; ++y < rhs._nRows; /* */ )
for( int x = -1; ++ x < rhs._nCols; /* */ )
_aElement[x][y] = rhs._aElement[y][x]; // dst
ROWS & COLS intentonally swapped
return *this;
}
TEMP
SELF& SELF::Transpose()
{
SELF orginal( *this );
_Destroy();
_Create( orginal._nCols, orginal._nRows ); // src ROWS & COLS
intentonally swapped
for( int y=-1; ++y < orginal._nRows; /* */ )
for( int x=-1; ++x < orginal._nCols; /* */ )
_aElement[x][y] = orginal._aElement[y][x]; //
dst ROWS & COLS intentonally swapped
return *this;
}
#undef TEMP_DEF
#undef TEMP
#undef SELF
#undef TYPE
// Test _____________________________________________________________
// ==================================================================
int main( int argc, char* argv[] )
{
#ifdef TEMPLATE
matrix<double> D(3,4,1);
#else
matrix D(3,4,1);
#endif
int i = 1;
for( int y = 0; y < D.GetRows(); y++ )
for( int x = 0; x < D.GetCols(); x++)
D.Set( x, y, (double) i++ );
#ifdef TEMPLATE
matrix<double> B( D );
matrix<double> T;
#else
matrix B( D );
matrix T;
#endif
cout << "Before Transpose\n";
D.Print();
T.Transpose( D );
cout << "\nAfter Transpose (copy)\n";
T.Print();
cout << "\nBefore Transpose (copy)\n";
B.Print();
cout << "\nAfter Transpose\n";
D.Transpose();
T.Print();
return 0;
}
// EOF
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]