Verbesserungsvorschläge für Mathe-Templateklasse

[***]

Hi,

hat hier jemand paar gute vorschäge für verbesserungen?

Bitte kommt mir nicht mit Zusatzfunktionen, es geht nur um den Aufbau sowie die schon vorhandenen Funktionsdeklarationen.

template <typename T> class vector
{
public:
        // Constructors / Destructor
    vector  (void);
    vector  (T x_, T y_, T z_, T w_ = 1);
    vector  (const vector<T> &other);
    ~vector (void);

        // Produkte:
    __forceinline T dotProduct (const vector<T> &other);
    __forceinline vector<T> crossProduct (const vector<T> &other) const;

        // Längenwerte:
    __forceinline const T length (void);
    __forceinline const T lengthSqr (void);

        // Normalisierungsfunktion
    __forceinline void normalize (void);
    __forceinline void normalizeEx (void);

        // Setzt den Vector auf 0
    __forceinline void clear (void);

        // Alles |*(-1)
    __forceinline void negate (void);

        // Errechnet den Winkel in Grad zwischen diesem und einem anderen vector
    __forceinline T angle (const vector<T> &other) const;

        // Errechnet einen Zufallsvector
    __forceinline void random (T min, T max);

        // Interpolierungsfunktionen
    __forceinline vector<T> interpolateCoords (const vector<T> &other, T factor) const;
    __forceinline vector<T> interpolateNormal (const vector<T> &other, T factor) const;

        // Distanzwerte:
    __forceinline const T distance (const vector<T> &other);
    __forceinline const T distanceSqr (const vector<T> &other);

          // Minimal- und Maximalwerte raussortieren  
    __forceinline vector<T> min (const vector<T> &other) const;
    __forceinline vector<T> max (const vector<T> &other) const;

        // *(-1)
    __forceinline vector<T> operator- (void) const;

        // Vergleichsoperator
    __forceinline const vector<T>& operator= (const vector<T> &other) 
    { x = other.x; y = other.y; z = other.z;    return (*this); }

        // Grundlegende Operatoren
    __forceinline vector<T>& operator+= (const vector<T> &other)    
    { x += other.x; y += other.y; z += other.z; return (*this); }
    __forceinline vector<T>& operator-= (const vector<T> &other)    
    { x -= other.x; y -= other.y; z -= other.z; return (*this); }
    __forceinline vector<T>& operator*= (const vector<T> &other)    
    { x *= other.x; y *= other.y; z *= other.z; return (*this); }
    __forceinline vector<T>& operator/= (const vector<T> &other)    
    { x /= other.x; y /= other.y; z /= other.z; return (*this); }

    __forceinline vector<T>& operator+= (const T &value)    
    { x += value; y += value; z += value;       return (*this); }
    __forceinline vector<T>& operator-= (const T &value)    
    { x -= value; y -= value; z -= value;       return (*this); }
    __forceinline vector<T>& operator*= (const T &value)    
    { x *= value; y *= value; z *= value;       return (*this); }
    __forceinline vector<T>& operator/= (const T &value)    
    {   T l = 1/value; x *= l; y *= l; z *= l;  return (*this); }

    union 
    {
        struct { T x, y, z, w; };
        struct { T r, g, b, a; };
        struct { T values[4];  };
    };
};

Achja und ich benutze __forceinline, seht es einfach als "inline" an

[unterschied]

Und was soll der Unterschied zwischen normalize und normalizeEx sein?

[***]

dort wird noch ein Sicherheitswert von 0,00001 draufgerechnet bevor normalisiert wird.

[volkard]

template <typename T> class vector
verwirrend, genau den gleichen amen zu nehmen, wie std::vector.
besser wäre wohl vector4d oder sowas.

// Constructors / Destructor
nutzloser kommentar. es würde reichen, mit den leerzeichen zu gliedern.
was jeweils die sektionen für themen haben, ist ja offensichtlich.
kommentare heb dir für wichtige sachen auf.

vector (void);
das (void) schreibt man nicht, sondern nur ().

vector (const vector<T> &other);
läßt vermuten, daß der copy-ctor etwas zauberhaftes macht. ist aber wohl nicht der
fall. dann las ihn weg und lass den compiler seinen automatisch generieren. da
weiß der leser dann sofort, wie kopiert wird.

~vector (void);
der macht nix, gell? also weg.

__forceinline const T length (void);
__forceinline const T lengthSqr (void);
gute namen.

__forceinline void normalizeEx (void);
wie bereits durch die andere frage gezeigt, fehlt hier ein kommentar, was die funktion macht.

__forceinline void clear (void);
ist diese funktion notwendig? im sinne einer kompletten und minimalen schnittstelle sicher nicht.
wird sie in der programmierpraxis ausreichend oft verwendet, daß es besser ist, a.clear() zu erfinden,
als immer a=vector(0,0,0,0) zu schreiben?

__forceinline void negate (void);
ich schätze, es wird eh eine multipyWithScalar oder ein op*(vector,T) gebaraucht. zu strecken
und stauchen, was ich für nicht extrem ungewöhnlichen bedarf halte. dann würde a.negate() wegfliegen
müssen zugunsten von a*=-1;

__forceinline vector<T> interpolateCoords (const vector<T> &other, T factor) const;
wen die funktion das meint, was ich vermute, müßte huer stehen:
friend interpolateCoords(vector const& a,vector const& b,T factor);
das könnte auch die produkt-funktionen treffen.
und die distanzfunktionen.
und min, max.

// Minimal- und Maximalwerte raussortieren
__forceinline vector<T> min (const vector<T> &other) const;
__forceinline vector<T> max (const vector<T> &other) const;
namen ändern. min(a,b) ist nunmal a<b?a:b. aber vectoren haben keinen op<.

// *(-1)
__forceinline vector<T> operator- (void) const;
oh. nur ne andere schreibweise für negate. das ist was doppelt.

// Vergleichsoperator
__forceinline const vector<T>& operator= (const vector<T> &other)
{ x = other.x; y = other.y; z = other.z; return (*this); }
erstens ist das der zuweisungsoperator, also der kommentar lügt.
und zweitens müßte man den weglassen können un der compiler baut einen.
und drittens schreibt man den am besten zwischen die konstruktoren
und die destruktoren. denn er passt thematisch hervorragend dazu.

// Grundlegende Operatoren
__forceinline vector<T>& operator+= (const vector<T> &other)
{ x += other.x; y += other.y; z += other.z; return (*this); }
keine kalllern um return (). sieht ja wie ne funktion aus.

__forceinline vector<T>& operator*= (const T &value)
{ x *= value; y *= value; z *= value; return (*this); }
ah, hier isser ja. also fliegt negate sicher raus. das unäre - würde ich drinlassen,
weil praktisch. weil damit die schreibweise mehr der schreibweise in mathebüchern ähnelt,
wenn man -a schreiben darf. deswegen muss dotProduct auch zum friend werden.

union
{
struct { T x, y, z, w; };
struct { T r, g, b, a; };
struct { T values[4]; };
};
die attribute am besten an den anfang der klasse schreiben, denn wenn man die schnell liest,
weiß man über die klasse sofort viel mehr, als wenn man erst die funktionen liest.

übrigens ist es schön, echt kleine triviale inlinefunktionen direkt in den code zu schreiben,
wie du das machst. ist besser, als jeder kommentar (und lügt garantiert nicht).

[***]

@volkard
Vielen dank für die Hinweise! aber was macht das friend da drin? welchen speziellen Sinn soll es haben?

Ja ich habe die klasse auch im namespace math:: nur nicht dahingeschrieben

[volkard]

*** schrieb:

@volkard
Vielen dank für die Hinweise! aber was macht das friend da drin? welchen speziellen Sinn soll es haben?

friend hat hier keinen sinn. da du die attribute erst am schluss hattest, konnte ich ja nicht ahnen, daß sie public sind.
ich nehme mal an.

class vector<T>{...
__forceinline vector<T> interpolateCoords (const vector<T> &other, T factor) const;
...}

ist etwa so implementiert:

__forceinline vector<T> vector::interpolateCoords (const vector<T> &other, T factor) const{
   return *this+factor*(other-*this)
}

ups, bin ich jetzt böse, daß ich auch nen op*(T,vector) damit verlange?

ich wünsche ne globale funktion

__forceinline vector<T> interpolateCoords (const vector<T> &a, const vector<T> b, T factor) const{
   return a+factor*(b-a)
}

denn ich will an anwender nicht

bullet.pos=me.pos.interpolateCoords(enemy.pos,now()-bullet.startTime);

, sondern

bullet.pos=interpolateCoords(me.pos,enemy.pos,now()-bullet.startTime);

schreiben.

[***]

hmmm so einen großen unterschied ist das zwar für mich nicht, aber okay

Weitere Kriterien?

[volkard]

*** schrieb:

Ja ich habe die klasse auch im namespace math:: nur nicht dahingeschrieben

nicht math!
willste etwa diese sachen auch noch in eine datei "math.h" stecken?

man beachte:

#include <math.h> //für __hypot2
#include "math.h" //für math::vector
#include <vector> //für std::vector

std::vector<math::vector<double> > transform(std::vector<math::vector<double> >, math::matrix m)

natürlich fällt dir erst nach 1000 zeilen auf, daß du in dieser datei auch ne transform brauchst und deswegen std::vector und hast vorher mit using namespace math gearbeitet.

kannste aber machen, wie du magst. als leitregel für guten stil möge die dienen: machst nicht wie die anderen sagen oder we es in büchern steht, sondern so, daß du am besten damit arbeiten können wirst. nimm natürlich begründungen, die in büchern stehen, als überleghilfe an.
und das verhindert auch gleich, daß du diese vielen unbegründeten (und oft gar nicht sinnvollen) stilregeln annimmst. stil soll ja helfen, und nicht wie ne heilige kuh umtanzt werden.

[volkard]

*** schrieb:

hmmm so einen großen unterschied ist das zwar für mich nicht, aber okay

da nicht.

aber du liest im mathebuch
c=a dot b. im computeralgebraprogramm dann schon c=dot(a,b). also ist man zum algrabraprog schonmal kompatibler.

bei klassen mit typumwandlungskonstruktor isses ein muss.

sei

class Bruchzahl 
{ 
int zaehler;
int nenner;
priblic:
Bruch(int z,int n=1)
...
}

und
Bruchzahl Bruchzahl::operator*(Bruchzahl other);
es wird c=Bruchzahl(1,2)*5; gut gehen, denn die 5 wird nach Buchzahl konvertiert.
aber c=5*Bruchzahl(1,2); geht nicht, denn int hat keinen op* mit bruchzahl.

schreibste aber Bruchzahl operator*(Bruchzahl a, Bruchzahl b);, dann gehen beide aufrufe. deswegen schafft bei solchen klassen immer die operationen, wo beide objekte gleichberechtigt sind, aus der klasse raus.
und da man das bei solchen klassen immer tut, wäre es ein fehler, es bei den anderen nicht zu tun. denn bei deiner klasse wäre es nicht nachteilhaft. aber sie wäre konsistenter mit den ganen anderen klassen, die einen typumwandlungskonstruktor haben. und konsistenz bringt's.

und frag weiter immer nach, wenn du eine begründung nicht siehst. wenn dann keine weitere erläuterung kommt oder eine, die man nicht einsehen kann, dann hat der autor erstmal unrecht.

[Gast]

Ich finde diese hundert __forceinline ganz leicht übertrieben. Ein normales inline reicht völlig aus, dass du es in den Header schreiben kannst, es ist schöner lesbar und inlinen ist außerdem Aufgabe des Compilers und nicht deine.

Ich sag ja nicht mal was, wenn du anhand von Messungen begründen kannst, dass diese Funktion nicht geinlined wurde und besser geinlined werden sollte. Aber meistens wird das der Compiler ganz gut machen und dein Source bleibt damit standardkonform.

Im übrigen ist eine fette .exe nicht so ganz ohne Nachteile, wenn der Rechner wegen dir dauernd hin und her cachen muss, hast du nichts gewonnen.

[volkard]

[quote="Optimizer"]Ich finde diese hundert __forceinline ganz leicht übertrieben. Ein normales inline reicht völlig aus, dass du es in den Header schreiben kannst, es ist schöner lesbar und inlinen ist außerdem Aufgabe des Compilers und nicht deine.[quote]
kann ich bestätigen. mir ist es einfach nicht gelungen, gescheit mit __forceinline__ zu arbeiten. die dicken brummer hat der compiler stets entgegen meinen wünschen doch outline gemacht.

Ich sag ja nicht mal was, wenn du anhand von Messungen begründen kannst, dass diese Funktion nicht geinlined wurde und besser geinlined werden sollte. Aber meistens wird das der Compiler ganz gut machen und dein Source bleibt damit standardkonform.

das mache man mit #define INLINE __forceinline__ und auf ner anderen plattform mit nem anderen define.

Im übrigen ist eine fette .exe nicht so ganz ohne Nachteile, wenn der Rechner wegen dir dauernd hin und her cachen muss, hast du nichts gewonnen.

das allerdings ist eine these, die erst bewiesen werden müßte.
dazu lese man aufmerksam http://www.cs.umass.edu/~emery/pubs/berger-pldi2001.pdf

die haben in der tat mehr performance, weil sie kleine funktionen machen und den compiler entscheiden lassen. im wpc habe ich auch die selbe beobachtung gemacht.
am meisten speed bekommt man im allgemeinen, wenn man dem compiler erlaubt, selber zu entscheiden, und im speziellen hab ich noch keine ausnahme gefunden, ich habe aber auch nicht gesucht.

[b7f7]

also ich würd viel der operatoren grundsätzlich aus der klasse lassen.
die elemente x,y,z,w würden mich ja dazu hinreissen die Klasse für projektive Koordinaten zu benutzen, aber auch für Quarternions da sind halt die operatoren unterschiedlich definiert.
also erst in mit den abgelittenen klassen solche ops() dazu.
und ein operator*(vector) const ist nicht eindeutig
v*v geht so nicht es geht nur v*vT oder vTv das eine hat als return ne matrix das ander ein skalar.
soll es also elementeweise multiplizieren?
das ist aaber Mv , M <-Matrix.
das ist dan aber kein operator von vector sonder eigentlich von Matrix oder global.

trotzdem
negate() und operator-(...)const machen ja unterschiedliche dinge ob negate nötig ist ist ansichtssache.

[volkard]

b7f7 schrieb:

v*v geht so nicht es geht nur v*vT oder vT*v das eine hat als return ne matrix das ander ein skalar.

was ist T und was bedeutet der satz?

[otze]

hier geht es um vectoren, und nicht um matritzen.

[b7f7]

das vektoren auch nur matrizen sind.

v*v
[x1]   [x2]
[y1] * [y2]  geht so nicht
[z1]   [z2]

aber das geht

v*vT vT<- Transponierter Vektor

[x1]                 [x1*x2] [x1*y2][x1*z2]
[y1] * [x2][y2][z2]= [y1*x2] [y1*y2][y1*z2] eine 3x3 Matrix
[z1]                 [z1*x2] [z1*y2][z1*z2]

vT*v
               [x2]
[x1][y1][z1] * [y2]= Skalarprodukt
               [z2]

[volkard]

b7f7 schrieb:

das vektoren auch nur matrizen sind.

warum frag ich überhaupt. sowas musst ja kommen.

[b7f7]

was ist denn dein Problem damit?
es ist nicht intuitiv klar Welche Operation gemeint ist.
es geht halt nur v.transp()*vector oder vector*vector.transp()
es geht def. nicht vector*vector.
die Probleme wird man alle los wenn man das als Matrix operationen betrachtet.

M*v ist definiert für eine Matrx(n,m)und ein Vektor(m,1) oder eine Matrix(m.p)
das ergebnis ist eine Matrix.
Scalierung geht damit

[sx][ 0][ 0]
[ 0][sy][ 0]
[ 0][ 0][sz]

Roation

[r00][r01][r02]
[r10][r11][r12]
[r20][r21][r22]

usw.

[Da Guru]

@Optimizer
er kann doch sowas schreiben:

#if !defined(_MSC_VER)
#define __forceinline inline
#endif

problem gelöst und sogar sehr schön

[rüdiger]

@Der Guru
nö, nicht sehr schön. Das eine ist nicht das andere und das andere Problem, was Optimizer zu recht angesprochen hat existiert noch.

[Da Guru]

wieso net schön?