Probleme mit Sichtbarkeitsmodifikatoren

Ist es sinnvoll die Methoden get_x() get_y() und get_z() einzubauen oder sollte man die member einfach public lassen und keine get und set Methoden verwenden?

Bashar

MisterX schrieb:

Ist es sinnvoll die Methoden get_x() get_y() und get_z() einzubauen oder sollte man die member einfach public lassen und keine get und set Methoden verwenden?

Die Klasse muss ihre Invariante sicherstellen können, auf deutsch gesagt, sie muss sicherstellen, dass niemand ihre Daten so durcheinanderwürfelt, dass die Klasse selbst damit nichts mehr anfangen kann. Daraus folgert man normalerweise, dass der Zugriff auf sämtliche Membervariablen geschützt wird, d.h. nur durch eine Auswahl von Methoden möglich ist.
Wenn es aber, wie hier, keine Invariante zu schützen gibt, weil schlicht und einfach jede mögliche Kombination von x, y, z auch gültig ist, kann man Felder IMHO auch public machen.

megaweber

MisterX schrieb:

inline Vector3D operator + (const Vector3D& a,const Vector3D& b)	
{return Vector3D(a.x + b.x, a.y + b.y, a.z + b.z);}

Mach das mal so :...

inline Vector3D operator + (const Vector3D& a)const	
{return Vector3D(a.x + x, a.y + y, a.z + z);}

Dann brauchst du auch nicht die friend deklaration.

megaweber

Bashar schrieb:

Die Klasse muss ihre Invariante sicherstellen können, auf deutsch gesagt, sie muss sicherstellen, dass niemand ihre Daten so durcheinanderwürfelt, dass die Klasse selbst damit nichts mehr anfangen kann. Daraus folgert man normalerweise, dass der Zugriff auf sämtliche Membervariablen geschützt wird, d.h. nur durch eine Auswahl von Methoden möglich ist.
Wenn es aber, wie hier, keine Invariante zu schützen gibt, weil schlicht und einfach jede mögliche Kombination von x, y, z auch gültig ist, kann man Felder IMHO auch public machen.

Oder man definiert einen [] operator. Finde ich auch sehr praktisch.

Bashar

megaweber schrieb:

Mach das mal so :...

inline Vector3D operator + (const Vector3D& a)const	
{return Vector3D(a.x + x, a.y + y, a.z + z);}

Oder mach operator+= als Member und operator+ als freie Funktion, die operator+= aufruft. Dann braucht man auch keine friend-Deklaration.

Hallo, ich habe mich jetzt entschieden kein set und get Methoden zu verwenden, da es ja wirklich keine zu schützende Invariante gibt.

#include <math.h>

#pragma once

class Vector3D
{
public:
	float x,y,z;
public:
	Vector3D(const float x_,const float y_,const float z_): x(x_), y(y_), z(z_){};

	Vector3D():x(0),y(0),z(0) {};

	Vector3D(const Vector3D& v): x(v.x), y(v.y), z(v.z) {} 

	~Vector3D(void){}

	inline Vector3D get_Vector() {return Vector3D(x,y,z);}

	inline void set_Vector(const float x_,const float y_,const float z_) {x=x_; y=y_; z=z_;}

    inline Vector3D& operator =  (const Vector3D& v)	{x = v.x; y = v.y; z = v.z; return *this;}
	inline Vector3D& operator += (const Vector3D& v)	{x += v.x; y += v.y; z += v.z; return *this;}
	inline Vector3D& operator -= (const Vector3D& v)	{x -= v.x; y -= v.y; z -= v.z; return *this;}
	inline Vector3D& operator *= (const Vector3D& v)	{x *= v.x; y *= v.y; z *= v.z; return *this;}
	inline Vector3D& operator *= (const float f)		{x *= f; y *= f; z *= f; return *this;}
	inline Vector3D& operator /= (const Vector3D& v)	{if ((v.x==0) || (v.y==0) || (v.z==0)){throw "Division duch 0 Klasse Vector3D Operator /= Vector3D";}x /= v.x; y /= v.y; z /= v.z; return *this;}
	inline Vector3D& operator /= (const float f)		{if (f==0){throw "Division duch 0 Klasse Vector3D Operator /= float";}x /= f; y /= f; z /= f; return *this;}

	inline Vector3D kreuz(const Vector3D b) {return Vector3D(y*b.z - z*b.y, z*b.x - x*b.z, x*b.y - y*b.x);}

	inline void Vector3D::normalize() {float laenge_Kehrwert =1 / Vector3D::length(); x*=laenge_Kehrwert; y*=laenge_Kehrwert; z*=laenge_Kehrwert;}

    inline float Vector3D::length() {return sqrt(x*x+y*y+z*z);}

    inline float Vector3D::sqrt_length() {return (x*x+y*y+z*z);}

};

	 inline bool operator == (const Vector3D& a, const Vector3D& b) {if(a.x != b.x) return false; if(a.y != b.y) return false; return a.z == b.z;}
     inline bool operator != (const Vector3D& a, const Vector3D& b) {if(a.x != b.x) return true; if(a.y != b.y) return true; return a.z != b.z;}

     inline Vector3D operator + (const Vector3D& a,const Vector3D& b)	{return Vector3D(a.x + b.x, a.y + b.y, a.z + b.z);}
   	 inline Vector3D operator - (const Vector3D& a,const Vector3D& b)	{return Vector3D(a.x - b.x, a.y - b.y, a.z - b.z);}
     inline Vector3D operator - (const Vector3D& a)					    {return Vector3D(-a.x, -a.y, -a.z );}
	 inline Vector3D operator * (const Vector3D& a, const Vector3D& b)	{return Vector3D(a.x * b.x, a.y * b.y, a.z * b.z);}
	 inline Vector3D operator * (const Vector3D& a, const float f)		{return Vector3D(a.x * f, a.y * f, a.z * f);}
	 inline Vector3D operator * (const float f, const Vector3D& a)		{return Vector3D(a.x * f, a.y * f, a.z * f);}
	 inline Vector3D operator / (const Vector3D& a, const Vector3D& b)	{if ((b.x==0) || (b.y==0) || (b.z==0)){throw "Division duch 0 Klasse Vector3D Operator /";}return Vector3D(a.x / b.x, a.y / b.y, a.z / b.z);}
	 inline Vector3D operator / (const Vector3D& a, const float f)		{if (f==0){throw "Division duch 0 Klasse Vector3D Operator /";}float f_Kehrwert=1/f; return Vector3D(a.x * f_Kehrwert, a.y * f_Kehrwert, a.z * f_Kehrwert);}

	inline float skalar_Produkt(const Vector3D v1,const Vector3D v2)
	{
		return v1.x*v2.x+v1.y*v2.y+v1.z*v2.z;
	}

	//Kreuzprodukt
	 inline	Vector3D    kreuz(const Vector3D a,const Vector3D b) {return Vector3D(a.y * b.z - a.z * b.y, a.z * b.x - a.x * b.z, a.x * b.y - a.y * b.x);}

	 //berechnet den Winkel in 360 Grad
	 inline float  winkel(Vector3D vec1, Vector3D vec2) 
	 {
		Vector3D help = vec1*vec2; 
		float help1 = help.x+help.y+help.z;
		float help2 = vec1.length()+vec2.length();
		float help3 = help1/help2;
		return acos(help3)/0.017453292519943295769f;
	 }

Ist an dieser Vectorklasse noch irgend was falsch, oder verbesserungsfähig?

MisterX schrieb:

Ist an dieser Vectorklasse noch irgend was falsch, oder verbesserungsfähig?

Den Deutsch/Englisch-Mix würde ich als verbesserungsfähig bezeichnen.

Und die fehlende const-correctness.

******* schrieb:

MisterX schrieb:

Ist an dieser Vectorklasse noch irgend was falsch, oder verbesserungsfähig?

Den Deutsch/Englisch-Mix würde ich als verbesserungsfähig bezeichnen.

Stimmt: Habe ich alle auf englisch geändert

******* schrieb:

Und die fehlende const-correctness.

Wo genau habe ich const falsch oder zu wenig benutzt?
(Ich kenn mich noch nicht so damit aus)

CStoll

MisterX schrieb:

******* schrieb:

Und die fehlende const-correctness.

Wo genau habe ich const falsch oder zu wenig benutzt?
(Ich kenn mich noch nicht so damit aus)

get_Vector() ist unnötigt (dafür hast du den Copy-Ctor), length() und sqrt_length() kannst du const setzen - und für das Kreuzprodukt reicht auch eine Version.

außerdem solltest du die globalen Operatoren durch die entsprechenden kombinierten Zuweisungen ausdrücken:

inline Vector3D operator+(const Vector3D& l, const Vector3D& r)
{
  return Vector3D(l)+=r;
}

PS: Und vernünftige Vektoren im mathematischen Sinn haben keine komponentenweise Multiplikation, sondern nur Skalar- und Kreuzprodukt (letzteres nur für 3D).