Stilfrage: Zuweisungen in if-Klammern



  • Hallo,
    ich habe da mal eine kleine Frage:
    Sind Zuweisungen in if-Klammern stilistisch in Ordnung oder eher schlechter Stil?
    Meine damit konkret z.B.

    int Get(void) {return 5;}
    
    int x;
    
    if((x=Get())==5)
    {
    	//...
    }
    

    oder doch eher so:

    int Get(void) {return 5;}
    
    int x=Get();
    
    if(x==5)
    {
    	//...
    }
    

    Gruß
    E-the-Real



  • Ich find das zweite schöner und lesbarer, aber ist wohl Geschmackssache.



  • Variante2 ist schöner



  • Ich bevorzuge die zweite Variante. Von mir aus können deswegen alle zu mir sagen "das machst du nur, weil du es nicht checkst".



  • Bei mir kommt es auf den Kontext an, welche Variante ich schöner finde.

    Kommt drauf an, ob ersichtlich ist was gemacht wird. Sowas z.B. kann mitunter missverstanden werden:

    if(cond1 && (cond2 || cond3 = true)){...}
    

    Also: "er soll nur reinlaufen wenn cond1 true ist, und falls cond2 false ist noch cond3 auf true setzen."



  • hmmm, aber imho ist die erste Varinate flexibler und schöner, was die Einsatzmöglichkeiten angeht. Folgendes Beispiel:

    if((second=m_Room->RoomItems.Find(in))!=-1)		//search in RoomItems
    {
    m_QuestManager.Evaluate(&m_Inventory,first,&m_Room->RoomItems,second);
    return m_ID;
    }
    else if((second=m_Room->Exits.Find(in))!=-1)	//search in Exits
    {
    	m_QuestManager.Evaluate(&m_Inventory,first,&m_Room->Exits,second);
    	return m_ID;
    }
    else		//not found
    {
    	cout<<"There is no "<<in<<"!\n";					return m_ID;	
    }
    

    gegenüber

    second=m_Inventory.Find(in);	//search in inventory
    if(second!=-1)			//success->...
    {	
    	m_QuestManager.Evaluate(&m_Inventory,first,&m_Inventory,second);
    }
    else 		//if the search in the inventory did not succeed...
    {
    	second=m_Room->RoomItems.Find(in);	//...search in the room
    	if(second==-1)			//not found->abort
    	{
    	cout<<"There is no "<<in<<"!\n";					return m_ID;							}								else
    	{
    	m_QuestManager.Evaluate(&m_Inventory,first,&m_Room->RoomItems,second);
    	}
    }
    

    So zieht diese zweite Möglichkeit eine weitere Verzweigunstiefe der if-Abfragen mit sich (was für ein Satz 😉 ), wohingegen bei der ersten Möglichkeit alle ifs/elses schön auf eine Ebene liegen.
    Was zieht man da vor?



  • If benutzt Zuweisungen eigentlich nicht in if Konstrukten jedoch in while. Bei mir findest du häufig:

    istream in;
    int c;
    while((c=in.get())!=EOF){
    
    }
    


  • upps, vielleicht sollte ich noch erklären, worum es geht 😉
    Also, es wird nach einem Item gesucht, das anhand der ID, die in "in" steht, gefunden werden soll.
    Dazu habe ich eine Find()-Methode implementiert, die -1 bei Misserfolg und ansonsten die Position des Items in dem entsprechenden vector zurückgibt.
    Zuerst wird im vector "RoomItems" gesucht.
    Wird es dort nicht gefunden, wird im vector "Exits" gesucht.
    Ist es dort auch nicht, kommt die Meldung, dass es nicht gefunden werden konnte.
    Hoffe, das ist so verständlich 🙂



  • Sowas also:

    int foo(){
      if( /*error*/ )
        return -1;
      else
        return 1;
    }
    //...
    int a;
    if((a=foo())!=-1){
      //...
    }
    

    Sowas löst man aber mit exceptions:

    int foo(){
      if( /*error*/ )
        throw runtime_error("Ups");
      else
        return 1;
    }
    //...
    try{
      int a=foo();
      //...
    }catch(runtime_error&excep){}
    


  • if (unsigned a = foo())
        //...
    
    if (A* p = dynamic_cast<A*>(b))
        //...
    

    alles andere ist ein schlechter Stil...



  • Shlo schrieb:

    if (unsigned a = foo())
        //...
    

    alles andere ist ein schlechter Stil...

    nö, das kann man leicht verwechseln mit:

    if (a == foo())
        //...
    

    deshalb ist _das_ schlechter stil



  • Wenn du das verwechselst, solltest du mal zum Augenarzt gehen.



  • net schrieb:

    deshalb ist _das_ schlechter stil

    Da müsstest aber ziemlich blind sein, eine Definition mit einem Vergleich zu verwechseln, also nix da.

    Des Weiteren siehe Kapitel 6.3.2.1 des Buches "Die C++ Programmiersprache" von Bjarne Stroustrup. Dort wird erklärt warum, eine Deklaration in einer Bedingung kein schlechter Stil ist.



  • Bashar schrieb:

    Wenn du das verwechselst, solltest du mal zum Augenarzt gehen.

    ich sagte man 'kann' es verwechseln.
    wenn man den code nur überfliegt z.b.



  • Bashar schrieb:

    Wenn du das verwechselst, solltest du mal zum Augenarzt gehen.

    quatsch, das ist leicht zu verwechseln, deshalb ist es schlechter stil. Nach einer Zuweisung separat, also nicht in der if-Anweisung, auf einen Gültigen Wert zu prüfen ist außerdem Ausdrucksstärker.

    int *f () { ... }
    
    int *x;
    if (x=f()) //undeutlich
    x=f;
    if (x)	//deutlich
    

    ich schreibe meinen code so, das ich ihn praktisch in die menschliche Sprache übersetzen und verstehen kann. Wenn ich if (x=f()) schreibe, dann denke ich intuitiv etwas anderes. Es geht wohl auch anderen so. Je expliziter etwas ausgedrückt ist, desto verständlicher ist der Code.



  • randa schrieb:

    quatsch, das ist leicht zu verwechseln, deshalb ist es schlechter stil.

    Achsooo, dann gehörst du bestimmt zu denjenigen die auch sowas schreiben:

    int i;
    
    for ( i = 0; i < 2; i++)
        //...
    


  • Shlo schrieb:

    Des Weiteren siehe Kapitel 6.3.2.1 des Buches "Die C++ Programmiersprache" von Bjarne Stroustrup. Dort wird erklärt warum, eine Deklaration in einer Bedingung kein schlechter Stil ist.

    das ist seine meinung. muss ich die teilen?

    Shlo schrieb:

    Achsooo, dann gehörst du bestimmt zu denjenigen die auch sowas schreiben:

    int i;
    for ( i = 0; i < 2; i++)
        //...
    

    in c muss man das.
    aber in c++ das 'int' in die for-anweisung zu schreiben ist z.b. kein schlechter stil, weil eindeutig. die lesbarkeit verbessert sich sogar, wenn man die variablendeklaration in die for-klammer schreibt. allein wegen der gültigkeit von 'i' ist es besser in der for-anweisung aufgehoben



  • und du gehörst zu denen:

    if (x=(y=z?y==f():z=4)=p++) cout << "Hello World"
    else cout << "Verschissen";
    

    das ist seine meinung. muss ich die teilen?

    Er argumentiert plausibel und hat recht. Du musst nicht auf ihn hören, weil er "der Master" ist, sondern weil er in 99% der Fälle mit seinen Feststellungen recht hat.



  • randa schrieb:

    Er argumentiert plausibel und hat recht.

    Geiles Argument! 😃



  • net schrieb:

    ich sagte man 'kann' es verwechseln.
    wenn man den code nur überfliegt z.b.

    Man kann viel verwechseln, wenn man nur überfliegt. Aber bei normalem Lesen fällt auf, dass da eine Deklaration steht (am unsigned zu erkennen). Wenn da die Mustererkennung im Gehirn immer noch einen Vergleich sehen will ... wie gesagt, medizinischer Notfall.

    IMHO hilft es auf lange Sicht ungemein, wenn man seine 'Intuition' der Sprache anpasst, es nützt nichts ständig dagegen anzukämpfen.


Log in to reply