Stilfrage: Zuweisungen in if-Klammern

[volkard]

ethereal schrieb:

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;    
}

wohingegen bei der ersten Möglichkeit alle ifs/elses schön auf eine Ebene liegen.
Was zieht man da vor?

was für ne frage?
da kannste genausogut anhand von zerbies code nachweisen, daß oo nix taugt oder anhand
von camarcks code nachweisen, daß goto gut ist.
also erstmal überlege ich, welchen sinn es macht, daß return m_ID dreimal da steht.

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

dann versuche ich zaghaft für die beiden fast identischen dinge eine inlinefunktion
zu bauen:

bool findAndEvaluate(Container* cont,Item const& item)
{
	int second=Container->Find(item);
	if(second==-1)
		return false;
	m_QuestManager.Evaluate(&m_Inventory,first,&m_Room->RoomItems,second);
	return true;
}
...
if(findAndEvaluate(&Room->RoomItems,in))
	return m_ID;
if(findAndEvaluate(&Room->Exits,in))
	return m_ID;
cout<<"There is no "<<in<<"!\n";                    
return m_ID;

oder je nach gusto auch

if(findAndEvaluate(&Room->RoomItems,in) || findAndEvaluate(&Room->RoomItems,in))
	return m_ID;
cout<<"There is no "<<in<<"!\n";                    
return m_ID;

aber die short-cirquit-auswertung in so seiteneffektigen aufrufen zu verwenden,
finde ich nicht fein.
oder

if(!findAndEvaluate(&Room->RoomItems,in))
	if(!findAndEvaluate(&Room->Exits,in))
		cout<<"There is no "<<in<<"!\n";                    
return m_ID;

da hat man ja furchtbar viele möglichkeiten. man sollte sich überlegen, ob die -1 eine
gute wahl war, um anzuzeigen, daß da nix gefunden wurde. warum wurden keine zeiger
verwandt?

merke: gute spieleprogrammierer haben keinen stil.

[Bashar]

interpreter schrieb:

Dass dieser Stil in der Vergangenheit wohl oft zu schwer auffindbaren Fehlern geführt hat, sieht man doch am Design neuer Sprachen. Genau _das_ ist der Grund, warum z.B. in Java oder C# sowas _nicht_ mehr erlaubt ist:
if(i = foo())

Ist das per se nicht erlaubt, oder ist es nicht erlaubt weil es keine implizite int-nach-boolean-Konvertierung gibt?

Ansonsten würde ich Java/C# nicht unbedingt als Maßstab nehmen ... die Designentscheidungen in diesen Sprachen wurden maßgeblich von Überlegungen, wie man möglichst viele Anfänger und Manager von sich überzeugt, geprägt.

[Vellas]

Hi!

Bashar schrieb:

Ist das per se nicht erlaubt, oder ist es nicht erlaubt weil es keine implizite int-nach-boolean-Konvertierung gibt?

So wie er es geschrieben hat ist es nicht erlaubt, da hier eine <Typ> nach boolean-Konvertierung stattfand. In C# muss immer explizit mit einem Wert verglichen werden, sofern es sich nicht um den Typ bool handelt, da geht dann sehr wohl dies:

bool richtig = true;
if(richtig) Console.WriteLine("Richtig");
else Console.WriteLine("Falsch");

Code-Hacker

[Bashar]

Also dann: Wer unbedingt will, dass die Zuweisung eine Anweisung und kein Ausdruck ist, kann sich ja mal im Pascal-Lager(+Ada, +Eiffel) umsehen.

[DrGreenthumb]

Bei

if(!(x = foo()) ...

lese ich, "Wenn foo() nicht funktioniert, mach dies und das". Gut.
Wenn dort aber steht

x = foo();

if(!x) ...

muss ich viel mehr denken. Erstmal wird x der Rückgabewert von foo() zugewiesen. Dann erst sehe ich, dass x evtl. ungültig sein kann und darauf reagiert wird.

IMHO will man aber meistens nicht auf ein ungültiges x prüfen, sondern darauf ob foo() funktioniert hat oder nicht. Das finde ich viel natürlicher und flüssiger zu lesen.

[Helium]

Bei

if(!(x = foo()) ...

lese ich, "Wenn foo() nicht funktioniert, mach dies und das". Gut.

Vielleicht lesen andere daraus: Wenn die zuweisung an x nicht funktioniert.

[ethereal]

volkard schrieb:

dann versuche ich zaghaft für die beiden fast identischen dinge eine inlinefunktion
zu bauen:

bool findAndEvaluate(Container* cont,Item const& item)
{
	int second=Container->Find(item);
	if(second==-1)
		return false;
	m_QuestManager.Evaluate(&m_Inventory,first,&m_Room->RoomItems,second);
	return true;
}
...
if(findAndEvaluate(&Room->RoomItems,in))
	return m_ID;
if(findAndEvaluate(&Room->Exits,in))
	return m_ID;
cout<<"There is no "<<in<<"!\n";                    
return m_ID;

oder je nach gusto auch

if(findAndEvaluate(&Room->RoomItems,in) || findAndEvaluate(&Room->RoomItems,in))
	return m_ID;
cout<<"There is no "<<in<<"!\n";                    
return m_ID;

hmm, ich finde ehrlich gesagt folgendes lesbarer als deine Idee mit der neuen Inline-Fkt....

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

	return m_ID;

Die drei returns waren in der Tat redundant und resultierten aus Cpy&Paste
Und hier finde ich auch die Zuweisung innerhalb der if-Klammern durchaus sinnvoll. Aber die Idee mit den Zeigern war auch gut. Mal sehen, was sich da machen lässt...
edit: cpp-Code formatiert

[Irgendwer]

IMHO will man aber meistens nicht auf ein ungültiges x prüfen, sondern darauf ob foo() funktioniert hat oder nicht. Das finde ich viel natürlicher und flüssiger zu lesen.

Wie ich bereits gesagt habe soll foo eine Ausnahme werfen oder einen gültigen Wert zurück geben und somit stellen sich solche Probleme erst gar nicht.

[Bashar]

Wenn es sich nicht um eine außergewöhnliche Situation handelt, sollte man auch keine Ausnahme einsetzen.

[volkard]

ethereal schrieb:

Aber die Idee mit den Zeigern war auch gut. Mal sehen, was sich da machen lässt...

naja, vorteil ist, daß der ungültige zeiger nach false auswertet, was erlaubt:

if(Bla* bla=find(...))
   tuwas(bla);

vortiel von integern ist aber die leichte relokierbarkeit der daten. also wenn dein array mal wachsen muss und deswegen woanders platz nimmt, sind alle langlebigen zeiger ins array rein kaputt. die indizes aber sind alle noch ok.
da könnte man sich noch überlegen, ne klasse Index zu basteln. und der op[] nimmt nicht mehr size_t oder int, sondern Index. Index hat natürliche nen umwandlungkonstruktor von int und size_t, damit normale schleifen auch nch gehen und man keinen unterschied merkt. aber Index könnte nen operator bol haben, der nur bei -1 false ausgibt. also nur im falle von "dieser index ist ungültig".

[otze]

erstmal zu den exceptions: ich bin dagegen, bei jedem funktionsfehlschlag den programmfluss mittels exception zu unterbrechen, stellt euch mal vor,in der std::map würde find ne exception werfen, wenn das element nicht gefunden wurde,anstatt dem üblichen end-iterator. was ich damit sagen will:

std::map<int,int> a;
try{
    (*a.find(5))=6;
}
catch(std::end& b){//ein typ der die exception auswerfende map liefern kann
    b->map.insert(5,6);
}

wer sowas gut findet hat echt ein medizinisches problem .
disclaimer: etwaige syntax fehler dürfen ausgeschnitten und in ein album geklebt werden, mir wurscht.

nun zum hauptthema:
for mit if zu vergleichen ist imho kindisch, weil die beiden strukturen sogut wie garnichts gemeinsam haben. If hat ein anwendungsgebiet, for ein andres, und je nach anwendungsgebiet sind die schreibweisen natürlich anders.
Im ersten argument einer for schleife würde ich nie einen vergleich schreiben, dafür aber jederzeit eine zuweisung, bei nem if ist es halt genau umgekehrt.

so nun mal ans problem:

x=foo()

das ist uns allen klar, das ist eine zuweisung

if(foo())

testet, ob foo() korrekt verläuft

if(x)

testet ob der wert von x korrekt ist

if(!(x = foo()))

das ist imho nicht eindeutig:
entweder foo liefert nicht den korrekten wert und die zuweisung läuft gut ab,
oder die zuweisung versagt
oder beides ist korrekt.
auf jedenfall kann man im fehlerfall bei komplexen datentypen nicht davon ausgehen, dass wenn der if block übersprungen wurde, auch foo() versagt hat.
das programm wäre also, wenn man den fehelrfall in einem elseblock abfangen würde,und nur von fehlern bei foo() ausgeht, nach der bearbeitung im ernstfall in einem inkonsistenten zustand.

das ist natürlich worst case, aber immerhin ein weiterer punkt,den es zu beachten gilt.

ps: und wenn ihr jetzt mit exception in der zuweisung kommt: JUHU der perfekte schlangencode

try{
    if(!(x = foo()){

    //Setzt hier 40 zeilen code ein

    }
    else
    {

    //hier nochmal 20
    }
catch(bad_alloc& a){
    //usw
}

dagegen ist das schon viel schöner:

try{
x=foo();
}
catch(bad_alloc& a){
    //JUHU ich kann direkt und übersichtlich direkt auf den fehler reagieren
}
if(x){
    //boah ich komm hier sogar in den if block rein, trotz exception
}
else
{
}

[Bashar]

Wie kommt ihr eigentlich darauf (ist ja jetzt schon der zweite), dass eine Zuweisung fehlschlagen kann und dann einen symbolischen Ungültig-Wert zurückgibt? Handelsübliche Zuweisungsoperatoren geben *this per Referenz zurück, wie soll das also funktionieren?

Fehlschlagen eines Zuweisungsoperators ist fast gleichbedeutend mit dem Fehlschlagen eines Konstruktors und sollte durch eine Exception angezeigt werden.

[otze]

lies weiter, da hab ich nochwas zu exceptions geschrieben

und ja, zuweisungen können jederzeit fehlschlagen, überall dort, wo ein new fehlschlägt,oder sonstwas unvorhergesehenes passiert.
und es ist fakt, dass man in if(x=foo()!=0) nicht mehr rechtzeitig auf diesen fall reagieren kann, der if block ist vorbei und man hat den salat.

[Bashar]

Hmm? Eben sagtest du noch, if (!(x=foo()) könne auch so interpretiert werden, dass auf Fehlschlag der Zuweisung geprüft wird. Jetzt widersprichst du dem und wiederholst eigentlich genau was ich gesagt habe. Sinn?

Fakt ist, dass Fehlschlagen von Zuweisungen nicht so wie oben angezeigt wird, so dass der Code auch nicht mißverstanden werden kann.

[net 0]

Bashar schrieb:

Handelsübliche Zuweisungsoperatoren geben *this per Referenz zurück

bei einem 'new'. aber sonst geben handelsübliche zuweisungsoperatoren das zurück, was im 'return' -statement steht oder eben der ausdruck rechts vom =.

Bashar schrieb:

Fehlschlagen eines Zuweisungsoperators ist fast gleichbedeutend mit dem Fehlschlagen eines Konstruktors und sollte durch eine Exception angezeigt werden.

gerade wenn man exceptions verwendet, kann eine zuweisung fehlschlagen. beispiel:

int foo (int a)
{
	if (a == 0)
		throw 1;
	return 0x5678;
}

if (x = foo(0)) 
{  
   ...

die zuweisung schlägt fehl. ohne 'throw' wär das nicht passiert (es sei denn, die funktion stürzt ab)

[interpreter]

net schrieb:

bei einem 'new'.

?

aber sonst geben handelsübliche zuweisungsoperatoren das zurück, was im 'return' -statement steht

Und das ist nun mal *this

oder eben der ausdruck rechts vom =.

???
Was gibt das hier wohl zurück:

Integer i;
i = 2;

einen int oder einen Integer?

[otze]

falsch, ich hab gesagt, dass man nicht herausbekommen kann, was fehlgeschlagen ist, da ist ein großer unterschied. der rückgabewert von dem op= ist etweder das, was bei foo rauskommt, oder irgendein krummer fehlerwert(natürlich nur, wenn keine exception geworfen wird).

das beispiel ansich ist verschärft:
einerseits sollten operatoren im fehlerfall exceptions werfen-soweit sind wir uns einig. Andererseits darf in diesem speziellen fall KEINE exception geworfen werden, weil wir sonst keine chance mehr hätten, an den if block zu kommen. Das exceptionhandling könnte also nicht das leisten, was es eigentlich sollte-und zwar das programm in einem guten zustand zu halten.

nun haben wir 3 möglichkeiten:
1.foo() wirft beim fehlschlag auch eine exception,und gibt keinen fehlercode zurück. Bei manchen funktionen klappt das-bei funktionen wie find nicht, weil der fehlschlag dort nicht unbedingt eine ausnahme sein muss. Diese version wäre imho durchführbar,würde aber zu enorm häßlichem und aufgeblähten code führen
2. es wird weder vom operator noch von foo eine exception geworfen.
die if abfrage behandelt nur das, was komplett richtig ist, der rest wird zur weiterverarbeitung und aufdröselung in den else teil verschoben.
Ist eigentlich nur ein c-like exception handling. Manchmal bietets vorteile, manchmal nicht, kommt sehr stark auf die implementation an.

3. man testet nur x und zieht den rest aus der if abfrage raus. Ok meine favorisierte lösung,man ist was rückgabewerte/exceptions angeht absolut frei, im idealfall kann man das programm normal weiterführen, also ich finds gut^^

[Bashar]

net: *Mööp* Sie sind raus!

otze schrieb:

falsch, ich hab gesagt, dass man nicht herausbekommen kann, was fehlgeschlagen ist, da ist ein großer unterschied. der rückgabewert von dem op= ist etweder das, was bei foo rauskommt, oder irgendein krummer fehlerwert(natürlich nur, wenn keine exception geworfen wird).

Nein, der Rückgabewert ist nie ein krummer Fehlerwert. Falls ja, soll der Schreiber des op= 20 Stockschläge bekommen.

das beispiel ansich ist verschärft:
einerseits sollten operatoren im fehlerfall exceptions werfen-soweit sind wir uns einig. Andererseits darf in diesem speziellen fall KEINE exception geworfen werden, weil wir sonst keine chance mehr hätten, an den if block zu kommen.

Was wollen wir im if-Block? Wenn die Zuweisung fehlschlägt, ist die Programmausführung in einem fehlerhaften Zustand, der erstmal bereinigt werden muss. Und zwar irgendwo in einem catch, nicht im if.

Wenn du die Exception lokal behandeln willst, setzt du die Zuweisung halt allein vor die if-Bedingung und schließt sie in try..catch ein. Das ist aber nicht die Regel, normalerweise behandelt man Exceptions nichtlokal.

[net 0]

interpreter schrieb:

net schrieb:

bei einem 'new'.

?

ach nee, auch nicht. bei einem 'new' bekommt man einen pointer auf das neu erzeugte objekt zurück. aber auch keinen *this.

interpreter schrieb:

aber sonst geben handelsübliche zuweisungsoperatoren das zurück, was im 'return' -statement steht

Und das ist nun mal *this

int f()
{
   return 1234;
}

was hat die rückgabe mit *this zu tun?

interpreter schrieb:

Was gibt das hier wohl zurück:

Integer i;
i = 2;

einen int oder einen Integer?

schwer zu sagen. was ist 'Integer'? eine klasse? ein typedef? ein #define?
müsste ein einfacher datentyp sein, wegen der zuweisung mit der 2. aber das = könnte auch ein überladener operator sein... keine ahnung

[Bashar]

net schrieb:

ach nee, auch nicht. bei einem 'new' bekommt man einen pointer auf das neu erzeugte objekt zurück. aber auch keinen *this.

Falls du's nicht gemerkt hast, es geht um Zuweisungsoperatoren und was sie zurückgeben, nicht was new zurückgibt.

net schrieb:

int f()
{
   return 1234;
}

was hat die rückgabe mit *this zu tun?

Ist das ein Zuweisungsoperator?

net schrieb:

keine ahnung

Stell das erstmal ab, bevor du die Diskussion durcheinander bringst. Sehr fruchtbar ist es jedenfalls nicht, wenn man immer zwischendurch deine Missverständnisse ausbügeln muss.