3. Problem mit Software Architektur (C++)

Problem mit Software Architektur (C++)

  • I

    Zu den rvalues. Jeder der schon mal mit der DirectX Extension Library zu tun hatte, wird das Problem kennen 😉

    Viele DirectX Funktionen nehmen einen Parameter als Zeiger, welcher dann aber innerhalb der Funktion kopiert wird. Eine Referenz wäre hier IMHO eben schöner, aber eben, man verwendet ja auch Code, der nicht von einem selbst ist:

    Beispiel:

    D3DXVECTOR3 out;
D3DXMatrixLookAtLH(&out,&D3DXVECTOR3(0.0f,1.0f,0.0),&D3DXVECTOR3(0.0f,0.0f,0.0f),&D3DXVECTOR3(0.0f,1.0f,0.0f));

    Mit dieser Warnung aktiviert muss man dies dann folgendermassen umschreiben:

    D3DXVECTOR3 out;
D3DXVECTOR3 eye;
D3DXVECTOR3 at;
D3DXVECTOR3 up;
D3DXMatrixLookAtLH(&out,&eye,&at,&up);

    Das mag zwar auf den ersten Blick sogar schöner sein, aber nicht mehr, wenn man 100erte solcher Funktionen aufrufen muss.

    Parameter aus dem Interface rausschmeißen, wenn diese nicht gebraucht werden.

    Dazu sag ICH jetzt mal nichts....

    Aber wie gesagt, ich möchte mich hier nicht streiten. Es wäre ja noch was ganz anderes gewesen, wenn einfach jemand freundlich nach dem WARUM gefragt hätte und sich anschliessend erst mal meine Argumentation angehört hätte, bevor drauf los gewettert wird, aber na gut...

    0
  • Mod

    Es ist aber schon so, dass Warnungen in der Regel gleich einem Fehler sind oder zumindest gleich einem potentiellen Fehler. Man schaltet eine Warnung nur dann bewusst aus, wenn man genau verstanden hat, warum diese Warnung ausgegeben wird und sich ganz sicher ist, dass diese Gründe in diesem besonderen Fall (jeden Fall einzeln prüfen!) nicht vorliegen.

    Dein DirectX Interface ist ein schönes Beispiel, wo man die Warnung ruhigen Gewissens ausschalten darf, sofern man ganz sicher weiß, dass in der Funktion eine Kopie des Pointees erstellt wird, so dass man hinterher nicht auf nicht-existierende Objekte zeigt. In jedem anderen Fall deutet die Warnung aber auf einen schweren Fehler hin und man sollte unverzüglich den Grund für die Warnung abstellen und nicht die Warnung selbst.

    0
  • S

    Ishildur schrieb:

    Parameter aus dem Interface rausschmeißen, wenn diese nicht gebraucht werden.

    Dazu sag ICH jetzt mal nichts....

    Den Namen des Parameters rauswerfen:

    int foo(int a, char) {
  return a;
}

    und schon ist die warnung weg...

    0
  • I

    @SeppJ
    Ja ich stimme dir bei diesem Beitrag bei, aus diesem Grund habe ich dem Compiler auch gesagt, das Warnungen als Fehler geanded werden sollen. Aber du kannst dir vorstellen, dass es einfach extrem nervig ist, wenn der Code dann ständig wegen solchen Sachen (wo man ganz genau weiss, dass es in diesem Fall kein Problem ist) nicht mehr kompiliert. Daher habe ich diese ausgeschaltet und schalte sie erst wieder ein, wenn ein Modul abgeschlossen ist (und prüfe da jeden einzelen Fall noch einmal).

    @Shade of Mine
    Danek für den Tipp, das habe ich nicht gewusst. 🙂

    0
  • I

    @Shade of Mine
    Habe das schnell ausprobiert, funktioniert, aber findest du das wirklich schön?

    void MyInterfaceMethod(float32 A,float32,float32,uint16,int64 E){
}

    Vielfach hat man ja auch die Situation, dass man eine Funktion (bspw. fürs Prototyping noch nicht vollständig implementiert (spricht noch nicht alle Parameter interpretiert) oder dass man sich einen zusätzlichen Parameter definiert (reserviert), bei dem man noch nicht weis, wofür man ihn irgendwann benutzen könnte, aber dass man die Funktion eben erweitern könnte, ohne die Interfaces verändern zu müssen.

    0
  • I

    @Janjan

    Schlechter Stil. Immer komplette Pfade haben.

    float32 divide(float32 A,float32 B){
 if(B == 0.0f) throw DivisionException();
 else return A/B;
}

    Kannst du mir zeigen, wie du das so machst, dass da keine C4715 Warnung kommt?

    0
  • S

    Ishildur schrieb:

    Vielfach hat man ja auch die Situation, dass man eine Funktion (bspw. fürs Prototyping noch nicht vollständig implementiert (spricht noch nicht alle Parameter interpretiert) oder dass man sich einen zusätzlichen Parameter definiert (reserviert), bei dem man noch nicht weis, wofür man ihn irgendwann benutzen könnte, aber dass man die Funktion eben erweitern könnte, ohne die Interfaces verändern zu müssen.

    Und in so einen Fall weisst du schon dass der Parameter numberOfElements oder so heissen wird?

    Du lässt den Namen ja in der Implementierung weg - in der Interface Definition darf ja ruhig ein Name stehen...

    Und was du in der Implementierung machst ist ja dein privat vergnügen...

    0
  • S

    Ishildur schrieb:

    @Janjan

    Schlechter Stil. Immer komplette Pfade haben.

    float32 divide(float32 A,float32 B){
 if(B == 0.0f) throw DivisionException();
 else return A/B;
}

    Kannst du mir zeigen, wie du das so machst, dass da keine C4715 Warnung kommt?

    float32 divide(float32 A,float32 B){
 if(B == 0.0f) throw DivisionException();
 return A/B;
}
    0
  • N

    Ishildur schrieb:

    @Janjan

    Schlechter Stil. Immer komplette Pfade haben.

    float32 divide(float32 A,float32 B){
 if(B == 0.0f) throw DivisionException();
 else return A/B;
}

    Kannst du mir zeigen, wie du das so machst, dass da keine C4715 Warnung kommt?

    float32 divide(float32 A,float32 B)
{
    if(std::fabs(B) == 0.0f)
        throw DivisionException();

    return A/B;
}
    0
  • I

    Wo ist der Unterschied? Ausser dass das Beispiel ohne "else" meiner Meinung nach weniger intuitiv lesbar ist?

    0
  • N

    Ishildur schrieb:

    Wo ist der Unterschied?

    Es gibt keine Warnung.

    0
  • I

    Also schreibe ich weniger intuitiven Code, nur damit keine MS Compiler spezifische Warnung erscheint? :p

    0
  • S

    Ishildur schrieb:

    Also schreibe ich weniger intuitiven Code, nur damit keine MS Compiler spezifische Warnung erscheint? :p

    Das else ist unnötig. oder schreibst du auch

    if(error1) throw error1();
else if(error2) throw error2();
else if(error3) throw error3();

    ?

    nein.

    den code immer links halten. dadurch reduziert sich die komplexität des codes.

    fehler werden mit if abgefangen und die funktion beendet. der normale kontrollfluss der funktion findet also auf der "linkesten ebene" statt.

    0
  • C
    Mod 

    float32 divide(float32 a,float32 b){
  return b != 0 ? a / b : throw DivisionException();
}
    0
  • A

    Ishildur schrieb:

    @asc
    Welche Warnung meinst du konkret, die man nicht einfach ausschalten sollte?

    Wie ich sehe, hat dein Compiler einige starke Mankos (z.B. 4715). Einige andere Warnungen kann man dadurch los werden, in dem man bei seinen Code kleine Anpassungen macht, und damit auch verdeutlicht was gemeint ist. Unabhängig davon halte ich dennoch ein allgemeines Abschalten für grundsätzlich falsch, da man dann auch nicht die Fälle wahrnimmt, in denen die Warnungen durchaus berechtigt sind.

    Ishildur schrieb:

    #pragma warning(disable:4100) // unreferenced formal parameter [unused method parameters]

    Viele Interfaces erwarten Parameter, welche jedoch von einigen konkreten Implementierungen nicht genutzt werden.

    Wenn ich Parameter nicht benutze, und dies mit Absicht geschieht, sieht das bei mir in der cpp-Datei so aus:

    void foo(
    int /* paramname */)
{
}

    Damit umgehe ich Einerseits die Warnung, und weise auch darauf hin, das ich den Parameter bewusst nicht nutze.

    Ishildur schrieb:

    #pragma warning(disable:4127) // conditional expression is constant [while(true)]

    Oft hat man keine Ahnung, wie oft eine schleife durchlaufen wird, sie wird mit einem break verlassen.

    Oft? Ich kenne dies als Sonderfall, mag bei dir anders sein. Aber ich will auch die Fälle sehen in denen Unabsichtlich ein konstanter Ausdruck entsteht (Ist mir auch schon einmal nach einer Codeanpassung passiert).

    usw.

    Ich sehe jedenfalls weiterhin keinen einzigen wirklichen Grund, Warnungen und Fehler als "unsinnig" anzusehen, da es durchaus mal Gründe für eben jene gibt. Lieber einmal ein umschließendes pragma warning push/pop als Warnungen zu übersehen, an Stellen, an denen sie ihre Bewandtnis haben.

    Ishildur schrieb:

    Also schreibe ich weniger intuitiven Code, nur damit keine MS Compiler spezifische Warnung erscheint?

    Mit welcher Version bitte? Also die enum-Warnung kenne ich z.B. garnicht, obwohl ich grundsätzlich mit höchster Warnstufe arbeite. Und merkwürdigerweise muss ich in der Regel nur selten eine Warnung absichtlich (und sehr lokal) unterdrücken um keine Warnung am Schluss zu erhalten; Und in der Regel unterdrücke ich fast ausschließlich nur Warnungen in Kombination mit externen Bibliotheken.

    Verwende ich den MS-Compiler nun falsch, weil ich keine Probleme mit Warnungen habe? 😉

    0
  • I

    Oft? Ich kenne dies als Sonderfall,

    Praktisch jeder iterativ implementierte Suchalgorithmus...

    0
  • ?

    Ishildur schrieb:

    Wo ist der Unterschied? Ausser dass das Beispiel ohne "else" meiner Meinung nach weniger intuitiv lesbar ist?

    Ist das dein ernst?

    Ishildur schrieb:

    float32 divide(float32 A,float32 B){
 if(B == 0.0f) throw DivisionException();
 else return A/B;
}

    Es ist deutlich intuitiver, dass else wegzulassen. Du prüfst du auf einen Sonderfall, und wenn diese Sonderfall eintritt wird sofort eine Exception geworfen und die Funktion endet. Bei einer Sonderfall-Überprüfung, die auch gleich die Funktion endet, schreibt man kein else.

    Schönere Einrückung macht es deutlicher.

    float32 divide(float32 A, float32B)
{
  if (b == 0.0f)
    throw DivisionException();

  return A/B;
}
    0
  • I

    @Janjan
    Ja du hast recht (natürlich auch die Andern, die dieses Beispiel kritisiert haben, was ein dummes Beispiel)! 🙂

    Wieso funktioniert diese simple Abstrakte Fabrik nach Lehrbuch einfach nicht?

    #include <iostream>

class IResource{
public: virtual int GetValue(void) const = 0x00;
};

class IResourceFactory{
public: virtual IResource *NewInstance(void) = 0x00;
};

class IHeightmapResource:public virtual IResource{
public: virtual int GetValue2(void) const = 0x00;
};

class IHeightmapResourceFactory:public IResourceFactory{
public: virtual IHeightmapResource *NewInstance(void) = 0x00;
};

class HeightmapResource:public IHeightmapResource{
public:
 int GetValue(void)  const{return 5;}
 int GetValue2(void) const{return 10;}
};

class HeightmapResourceFactory:public IHeightmapResourceFactory{
public: HeightmapResource *NewInstance(void){return new HeightmapResource();}
};

int main(){
 IResourceFactory *pFac = new HeightmapResourceFactory();
 IResource        *pRsc = pFac->NewInstance();

 std::cout << pRsc->GetValue() << std::endl;
 std::cout << dynamic_cast<IHeightmapResource*>(pRsc)->GetValue2() << std::endl;
 std::cout << std::endl;

 delete pRsc;
 delete pFac;

 return 0;
}

    Resultat:
    Kompiliert anstandslos und stürzt anschliessend bei delete pRsc mit einer BlockInUse Assertion ab 😞

    0
  • N
     

    public: virtual int GetValue(void) const = 0x00;

    Hehe lol, was soll denn die 0x00? Ist dir einfach nur 0 nicht hübsch genug?
    Und (void) ist auch ein unnützer Anachronismus. Du machst es gern kompliziert und schreibst unnötiges Zeug, was? 🙂

    Ishildur schrieb:

    Kompiliert anstandslos und stürzt anschliessend bei delete pRsc mit einer BlockInUse Assertion ab 😞

    Ich sehe keinen Fehler im Code, bei Codepad läuft es auch durch.

    0
  • I

    Hehe lol, was soll denn die 0x00?

    Mein Gott fängt das wieder an. Ich schreibe bei long 45l, bei float 3.4f bei Zahlen 0,1,2,3,4 und bei Adressen 0x00, 0x01, 0x02 das ist mein Stil basta! :p

    Was ich gemerkt habe ist, dass wenn ich IHeightmapResource nicht virtuell von IResource erbe dann funktioniert es, aber wieso stürzt es bei virtueller Vererbung ab, das darf doch nicht wahr sein... 😮

    0
Anmelden zum Antworten