• Create Account

## How to get rid of this?

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

5 replies to this topic

### #1Vortez  Members

2705
Like
0Likes
Like

Posted 26 April 2014 - 02:48 AM

Hiya, i was wondering if someone could help me with a design flaw in my library's vector and matrice code. See, like i posted here, i always need to access the x,y,z part of the vector by the letter v, and m for matrices.

Another example would be

ApplyMatrixToVector(&p->ThrustVector.v, &p->JetMatrix.m);



It's kinda annoying to have those v and m letters appended everywhere, but im affraid to modify the code since it work perfectly well.

Also, I know the ApplyMatrixToVector() should be done by overriding the '*' operator but it's already overloaded for something else i think.

Here the source:

vector.h

vector.cpp

Matrix.h

Matrix.cpp

Thx

EDIT: Humm, i know the the header file look a bit strange, it was done that way to compile the library either normally or in dll for multiples languages, just don't mind the #defines (another thing to fix, i need to remove this, but that i dont need help with )

Edited by Vortez, 26 April 2014 - 02:53 AM.

### #2Bacterius  Members

13100
Like
7Likes
Like

Posted 26 April 2014 - 03:56 AM

POPULAR

That's because your class is designed to multiply a CMatrix with a Matrix, or add a CVector to a Vector, which isn't right. What you want is to be able to multiply a CMatrix with a CMatrix, or add a CVector with a CVector (and obtain a CMatrix or CVector in return respectively). Then only these multiplication/addition functions have to mess with the "v" and "m" fields, and you can safely make these fields private, as they should be.

It seems to me you've complicated your math library for no reason, by adding an extra composition layer between the raw data (Matrix, a structure of 16 floats) and the class that manipulates this data (CMatrix, which interprets it and adds/multiplies/etc..). Typically this wouldn't be "bad" (even though it is usually done internally, and the raw data structure (Matrix) is never exposed outside of its wrapper class (CMatrix) except through serialization/deserialization, if applicable) but in your case it is problematic because your class asks for the raw data to operate on, and hence cannot directly operate on itself, which makes it impossible to handle matrices through CMatrix alone - you have to constantly bounce back and forth between the data (by accessing the "m" field) and the class that operates on matrices (by calling a function that takes this field as a parameter).

If that's not clear enough, what you've basically written is a class that doesn't really contain anything. Up to a few details, all of the useful functions in CMatrix could be made static (or free functions) and the code reduced to moving Matrix variables around and calling CMatrix static functions on them, which is essentially what you are doing now, even if it somewhat obfuscated. You need to decide on whether you want CMatrix to simply contain functions that work on Matrix variables and not actually hold a matrix themselves, or whether you want CMatrix to represent an individual matrix equipped with functions that can operate on it and other matrices (the second approach is the more popular one). Again, both can be made to work, but in your case your code seems to exist somewhere between the two approaches, and as a result is encountering severe design and consistency problems. Once you've decided on what you want your class to actually do, you should know what to modify.

The same goes for CVector and so on, naturally.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

### #3Vortez  Members

2705
Like
0Likes
Like

Posted 26 April 2014 - 04:09 AM

Thx for the answer bac, this clear things up a bit. Great explanation btw, i'll look into it.

### #4Vortez  Members

2705
Like
1Likes
Like

Posted 28 April 2014 - 11:36 AM

Ok, so i was speaking with Kathar the other night in the chat about those classes and he did a wonderfull job helping me out. The thing is, im not very used to reference in c++, i always use pointers, but i understand what they are and, i almost understand when to use them but it's tripping me off sometime. Here's the code:

Vector.h

Vector.cpp

Matrix.h

Matrix.cpp

What i don't get is, should't this, for example

CMatrix _EXP_FUNC operator+(CMatrix m1, const CMatrix &m2);

be

CMatrix _EXP_FUNC operator+(const CMatrix &m1, const CMatrix &m2);

???

It mostly confuse me in operator overloading functions... maybe he just missed that, or am i wrong?

### #5Brother Bob  Moderators

10103
Like
3Likes
Like

Posted 28 April 2014 - 11:51 AM

What i don't get is, should't this, for example

CMatrix _EXP_FUNC operator+(CMatrix m1, const CMatrix &m2);

be

CMatrix _EXP_FUNC operator+(const CMatrix &m1, const CMatrix &m2);

???

It mostly confuse me in operator overloading functions... maybe he just missed that, or am i wrong?

Follow how the operators are implemented and how the arguments are used. The + operator is implemented as

CMatrix operator+(CMatrix m1, const CMatrix &m2)
{
return m1;
}

where m1.Add(m2) is equivalent to m1 += m2. If you pass m1 as a reference as well, you wouldn't be able to update it and then return it.

You can of course make m1 a const reference as well, but you would then have to create a temporary matrix to hold the result of the addition.

CMatrix operator+(const CMatrix &m1, const CMatrix &m2)
{
CMatrix res = Add(m1, m2); // pseudo-function that returns m1+m2
return res;
}

The non-reference variant uses m1 as that temporary variable.

### #6Vortez  Members

2705
Like
0Likes
Like

Posted 28 April 2014 - 12:18 PM

The non-reference variant uses m1 as that temporary variable.

I see, i think i understand better now.

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.