if-else welche Variante ist besser?

Kolumbus

Hallo Gemeinschaft,

ich hab' heut meinen Spiel- und Experimentier-Tag... *g* Welche Variante ist besser:

// Variante 1
	(Entscheidungsvariable ? RadBtnB1 : RadBtnA1)->Enabled= false;
	(Entscheidungsvariable ? RadBtnB1 : RadBtnA1)->Checked= false;
	(Entscheidungsvariable ? RadBtnB2 : RadBtnA2)->Enabled= false;
	(Entscheidungsvariable ? RadBtnB2 : RadBtnA2)->Checked= false;
	Entscheidungsvariable ? bRadBtnBEnabled= false : bRadBtnAEnabled= false;

// Variante 2
	if(Entscheidungsvariable ){
		RadBtnA1->Enabled= false;
		RadBtnA1->Checked= false;
		RadBtnA2->Enabled= false;
		RadBtnA2->Checked= false;
		bRadBtnAEnabled= false;
	}
	else{
		RadBtnB1->Enabled= false;
		RadBtnB1->Checked= false;
		RadBtnB2->Enabled= false;
		RadBtnB2->Checked= false;
		bRadBtnBEnabled= false;
	}

Ich würde denken Variante 1 ist WESENTLICH langsamer, wegen der vielen if-else-Konstrukte. Allerdings ist der Code so schön kurz... welche Variante würdet ihr bevorzugen?

MfG

_matze

Variante 1 dürfte so doch gar nicht funktionieren. Kriegst du das kompiliert?

Abgesehen davon sollte die Lesbarkeit an erster Stelle stehen, wenn es sich nicht gerade um ganz zeitkritische Geschichten handelt. Außerdem glaube ich nicht, dass eine der Varianten mit optimiertem Code schneller als die andere wäre (wenn es sich kompilieren ließe)...

Kolumbus

_matze schrieb:

Variante 1 dürfte so doch gar nicht funktionieren. Kriegst du das kompiliert? [...]

Ja! Und es macht genau das, was es soll...

_matze schrieb:

[...] Abgesehen davon sollte die Lesbarkeit an erster Stelle stehen, wenn es sich nicht gerade um ganz zeitkritische Geschichten handelt. [...]

Ich kann das lesen. Was ist da nicht lesbar?

(Entscheidungsvariable ? RadBtnB1 : RadBtnA1)->Enabled= false;

Ist die Entscheidungsvariable wahr, deaktiviere RadioButtonB1, ist die Entscheidungsvariable falsch, deaktiviere RadioButtonA1.

_matze schrieb:

[...] Außerdem glaube ich nicht, dass eine der Varianten mit optimiertem Code schneller als die andere wäre [...] ...

Sowas wollte ich hören.

Killer-Kobold

Na, ich weiß ja nicht. Bei Variante 2 wird nur ein Vergleich durchgeführt, das könnte tatsächlich schneller sein. Vielleicht meßbar, aber definitiv nicht spürbar.
Ich find Variante 2 auch lesbarer. Da sieht man auf den ersten Blick was Sache ist. Bei Version 1 muss man jede Zeile einzeln betrachten.

Just my 2 cents.

Gruß KK

Kolumbus

Killer-Kobold schrieb:

Na, ich weiß ja nicht. Bei Variante 2 wird nur ein Vergleich durchgeführt, das könnte tatsächlich schneller sein. Vielleicht meßbar, aber definitiv nicht spürbar.
[...]

Mit 2 clock_t-Variablen und Zeitmessung per clock() aus time.h konnte ich bei beiden Varianten keine Zeitdifferenz ermitteln...

Killer-Kobold schrieb:

[...]Ich find Variante 2 auch lesbarer. Da sieht man auf den ersten Blick was Sache ist. Bei Version 1 muss man jede Zeile einzeln betrachten.

Eigentlich habt ihr schon recht... ich muss vielleicht einfach mal von meinem "so-kurz-wie-möglich"-Wahn wegkommen...

Killer-Kobold schrieb:

[...] Just my 2 cents.

Danke dafür!

audacia

Viel wichtiger als "so kurz wie möglich" ist "so wenig Redundanz wie möglich". In diesem Sinne würde ich auf deine Initialfrage auch mit "sie sind beide gleich schlecht" antworten Du solltest dein Problem möglichst anders lösen.

(Noch kurz zu den Geschwindigkeitsunterschieden: Variante 2 ist sicherlich effizienter. Aber darüber denkst du nur nach, bis du weißt, was ein Aufruf von TRadioButton::SetChecked() oder TControl::SetEnabled() kostet. Wenn es dich interessiert, kannst du ja mal im Debug-Modus in den VCL-Code steppen.)

_matze

Kolumbus schrieb:

Ja! Und es macht genau das, was es soll...

Oh tatsächlich! Sorry, ich dachte immer, solche Konstrukte dürfen nur in Ausdrücken verwendet werden und nicht alleine stehen. Keine Ahnung, wie ich darauf komme...

EDIT: Ok, wenn ich das als C-Code kompilieren wll, spuckt er einen Fehler aus.

b?i=1:i=0;  //C++-only
i=b?1:0;    //geht natürlich immer

Ich wusste doch, dass da was war...

Kolumbus

audacia schrieb:

Viel wichtiger als "so kurz wie möglich" ist "so wenig Redundanz wie möglich". In diesem Sinne würde ich auf deine Initialfrage auch mit "sie sind beide gleich schlecht" antworten Du solltest dein Problem möglichst anders lösen.[...]

Sehr weise, aber leider etwas unkonkret... ich bin etwas überfragt, wie ich die geposteten Codeschnipsel besser/anders umsetzen soll!?

audacia schrieb:

[...](Noch kurz zu den Geschwindigkeitsunterschieden: Variante 2 ist sicherlich effizienter. Aber darüber denkst du nur nach, bis du weißt, was ein Aufruf von TRadioButton::SetChecked() oder TControl::SetEnabled() kostet. Wenn es dich interessiert, kannst du ja mal im Debug-Modus in den VCL-Code steppen.)

Alter Schwede!!!

witte

Kolumbus schrieb:

Sehr weise, aber leider etwas unkonkret... ich bin etwas überfragt, wie ich die geposteten Codeschnipsel besser/anders umsetzen soll!?

Ich habe mich aber auch gefragt wozu Du die beiden booleans benötigst, zumal Du mit zwei boolean-Variablen zusammen zwei Zustände darzustellen scheinst. Hängen denn noch mehr Controls von "Entscheidungsvariable" ab? Dann könntest Du Dir mal das Statusmuster anschauen.

audacia

Kolumbus schrieb:

Sehr weise, aber leider etwas unkonkret...

Ebenso
Für diese Entscheidung sind mir "Entscheidungsvariable", "RadBtnB1" und "RadBtnA1" zu abstrakt. Falls du auf dieser Abstraktion beharrst, so bekommst du auch nur allgemeine Ratschläge:

Wäre es nicht vielleicht sinnvoller, zwei RadioGroups zu benutzen und diese wechselseitig zu deaktivieren?
Wie witte sagte: warum speicherst du einen Zustand (bRadBtnAEnabled, bRadBtnBEnabled), den bereits die entsprechende Komponente für dich speichert? (Und bevor du fragst, die entsprechende Operation ist nicht einmal teuer:

// Controls.pas, Z. 4844ff
function TControl.GetEnabled: Boolean;
begin
  Result := FEnabled;
end;

;))

Kolumbus

Ich melde mich demnächst in diesem Thread zurück... ich habe leider gerade ein anderes Problem!

Kolumbus

witte schrieb:

[...]Ich habe mich aber auch gefragt wozu Du die beiden booleans benötigst, zumal Du mit zwei boolean-Variablen zusammen zwei Zustände darzustellen scheinst.

Der Teil des Formulars auf dem sich die RadioButtons und noch viele weitere Controls befinden, wird zwischenzeitlich komplett deaktiviert und ich muss hinterher noch wissen, welche Controls aktiv waren ;o)

witte schrieb:

[...] Hängen denn noch mehr Controls von "Entscheidungsvariable" ab? Dann könntest Du Dir mal das Statusmuster anschauen.

ja, von meiner "Entscheidungsvariablen" hängen noch mehr Controls ab! Was ist Statusmuster?

Edit: Die Forumsuche spuckt zu "Statusmuster" nur diesen Thread hier aus... Herzlichen Glückwunsch für die 1. Verwendung dieses Wortes hier im Forum.

witte

O.K. ich habe Zustand ungeschickt übersetzt:
http://de.wikipedia.org/wiki/Zustand_(Entwurfsmuster)

Ich benutze das Muster häufig, wenn das Form in verschiedene Zustände versetzt werden muß Readonly->Anfügen->Bearbeiten oder bei Implementierung von Benutzerrechten. Du mußt mal prüfen wieweit der Implementierungsaufwand sich lohnt.

Kolumbus

witte schrieb:

[...]http://de.wikipedia.org/wiki/Zustand_(Entwurfsmuster)
[...] Du mußt mal prüfen wieweit der Implementierungsaufwand sich lohnt.

Schon dabei, das mal abzuschätzen. Wirklich interessante Herangehensweise... Das Programm wird zwar dadurch größer, aber um Einiges schneller. Sollte man GoF gelesen haben?

audacia schrieb:

Wäre es nicht vielleicht sinnvoller, zwei RadioGroups zu benutzen [...]

Danke für den Tip - das habe ich übersehen

audacia schrieb:

Viel wichtiger als "so kurz wie möglich" ist "so wenig Redundanz wie möglich". In diesem Sinne würde ich auf deine Initialfrage auch mit "sie sind beide gleich schlecht" antworten Du solltest dein Problem möglichst anders lösen.

Dazu nochmal: hattest du es im Endeffekt auch auf witte's Vorschlag abgesehen?

Kolumbus

Entwurfsmuster... für jeden komplexen Zustand einer Komponente eine eigene Komponente... Ich denke ich werde es mal testen... Da ich die meisten Controls auf Panels angeordnet habe, lässt sich das relativ unkompliziert handhaben.

audacia

Kolumbus schrieb:

audacia schrieb:

Viel wichtiger als "so kurz wie möglich" ist "so wenig Redundanz wie möglich". In diesem Sinne würde ich auf deine Initialfrage auch mit "sie sind beide gleich schlecht" antworten Du solltest dein Problem möglichst anders lösen.

Dazu nochmal: hattest du es im Endeffekt auch auf witte's Vorschlag abgesehen?

Nein, ich hatte es auf nichts direkt abgesehen. Ich schrieb ja, daß mir dein Beispiel zu wenig konkret ist, als daß ich ein alternatives Design vorschlagen könnte. Aber das State-Pattern ist da sicherlich anwendbar.

witte

Kolumbus schrieb:

Entwurfsmuster... für jeden komplexen Zustand einer Komponente eine eigene Komponente...

Ich dachte jetzt eigentlich, dass das Form einen member bekommt, welches einen Zustand repräsentiert und die Komponenten des Form darauf einstellt. Wenn der Zustand sich ändert wird das Zustandsobjekt ausgetauscht und dann mithilfe des neuen Objektes die Komponenten eingestellt. Diese Zustandsobjekte sollten dann friends zur Formklasse sein.