Thread: Matrices library is now!!!

Originally Posted by ChaosEngine

First, well done on releasing your first project. next some constructive criticism:

• first you absolutely must include some more info in your header file. At the very least, you should put author, date, revision, copyright and warranty disclaimer. Theoretically I could use this code and if it didn't work, I could sue you. (it's not very likely but you should protect yourself). I'd recommend the boost licence, but there are others. to use it you just need to add
  
  Code:

  // Copyright Hussain Hani 2007.
  // Distributed under the Boost Software License, Version 1.0. (See
  // http://www.boost.org/LICENSE_1_0.txt)

• why is matrix restricted to floats? you could make the library a template and then users could have matrices of doubles/ints/complex numbers/whatever.
• what happens if I create a Matrix and use it without calling build? will it fail? if so, wouldn't it make more sense to require rows and columns as arguements to the constructor?
• how does print() work? the normal c++ convention would be to make an overloaded '<<' operator. that would work with printing and with files (and any other stream)
• if you're overloading the +, - and * ops it's customary to overload +=, -= and *= too. In fact you should overload those first and then op+ is simply
  
  Code:

  Matrix operator+(const Matrix& rhs)
  {
      Matrix result(*this);
      result += rhs;
      return result;
  }

• const correctness - print(), getr(), getc() and about() should be const. op+, - and * should return const Matrix. this prevents accidental mistakes like (m1 * m2 = m3), which is legal but useless in your code.

That's a few points from reading the header. If you want to post the source, I'll have a look at that too. Finally, wrt to boost, they won't accept a closed source library, so you'll have to provide the source. Plus I don't think the library is up to boosts standards yet.

Sorry if that's a bit harsh. Please take the criticism in the spirit in which it's intended, which is to help you improve the library. It's a good first attempt, but it needs work.

Answering your first question about the constructor that has to have arguments. I dont agree. When you just simply creat a matrice, the dimensions are zero and the pointers are null, and I tried to stick to original C++ library, like for example the vector
it's much cleaner to say :

Code:

Matrix matrix;
//than 
Matrix matrix(2,1);

//and 
vector<int> intvec;
//than 
vector<int> intvec(2,4);

Although you could inter some arguments in vector class.

for you question about "why didnt I overload the << operator to show the result instead of print() function)". The answer is that I wanted to keep the library in its OO boundries. I didnt want it to use the C++ built-int that hides it's and actual function. For example, I wanted it to look like this :

Code:

matrix.print();

so the user will know that he is dealing with library while using the dot operator.

Originally Posted by Hussain Hani

for you question about "why didnt I overload the << operator to show the result instead of print() function)". The answer is that I wanted to keep the library in its OO boundries. I didnt want it to use the C++ built-int that hides it's and actual function. For example, I wanted it to look like this :

Code:

matrix.print();

so the user will know that he is dealing with library while using the dot operator.

Kinda reminds me of these, plus or minus a few FAQs:
http://parashift.com/c++-faq-lite/in....html#faq-15.8
http://parashift.com/c++-faq-lite/in....html#faq-15.9

one popular one is GPL

I rarely see the GPL used for projects such as this, if ever. Libraries more commonly use the LGPL, whereas applications use the GPL.

Congrats on your library. More constructive (I hope) criticism.

• about() - This function has no parameters and returns no data. How does one go about getting info about the library if it does neither? Does it merely print it to stdout? If it does, I'd recommend not to - as an application developer I want to have control over how and what data gets outputted. Also, about() is a member function of Matrix, meaning I have to declare and allocate memory to a Martix just to get info on the library. You might want about to be static, since it has (as far as I can tell) no need to read/write any data in the class.
• Is there any way in gcc/MinGW to use .lib files? Otherwise, I can't test the library. (I'm thinking that g++ doesn't like MSVC's name mangling, but I could be way off here...) (And yes, I tried "g++ -o test.exe test.cpp matrix.lib" - undef'd references to Matrix::*)

Originally Posted by Hussain Hani

Answering your first question about the constructor that has to have arguments. I dont agree. When you just simply creat a matrice, the dimensions are zero and the pointers are null, and I tried to stick to original C++ library, like for example the vector
it's much cleaner to say :

Code:

Matrix matrix;
//than 
Matrix matrix(2,1);

//and 
vector<int> intvec;
//than 
vector<int> intvec(2,4);

Although you could inter some arguments in vector class.

Actually you didn't answer my question. I asked you what happens if I don't call build? does your library check for the null pointers? will it throw an exception? crash?

as for vector,

• vector is in a usable state when default constructed. you can perform push_back, reserve, resize, etc. Also a vectors purpose is to be a dynamically resizeable array. it grows as you need it. a matrix is a fixed size.
• vector has a "size constructor"

In general, it is bad design for a class that is default constructable to be in an invalid state after construction.

Originally Posted by Hussain Hani

for you question about "why didnt I overload the << operator to show the result instead of print() function)". The answer is that I wanted to keep the library in its OO boundries. I didnt want it to use the C++ built-int that hides it's and actual function. For example, I wanted it to look like this :

Code:

matrix.print();

so the user will know that he is dealing with library while using the dot operator.

sorry, that's just wrong. read the links Dave posted (esp the second one)