2010-12-30 1 views
1

Ich Refactoring einige Code, der nicht von mir geschrieben wurde. Dieser Block setzt den Wert $val, aber ich möchte es ein wenig aufräumen. Natürlich kann ich den Tertiäroperator hier nicht verwenden. Welche anderen Möglichkeiten, um diesen Code sauberer zu machen?Refactoring dieses Blocks

if (isset($vars[$input])) { 
     $val = $vars[$input]; 
    } elseif (isset($this->getI['io'])) { 
     $val = $this->getI['io']; 
    } elseif (isset($vars[5])) { 
     $val = $vars[5]; 
    } else { 
     $val = 10; 
    } 
+5

Ich bin verwirrt: Was ist mit diesem Code falsch? –

+0

yup das ist sinnlos refactoring wenn du mich fragst. Vielleicht könntest du das in eine switch-Anweisung schreiben, aber das würde es nicht sauberer machen! – Alfred

Antwort

1

Ich fürchte, ich weiß nicht, PHP. Ich gehe davon aus, dass, wenn Sie (sagen wir) $ vars [$ input] an eine Funktion übergeben würden, bis zu der Zeit, als es ein Parameter für die Funktion war, die set-ness des Parameters wahr wäre (falls das nicht der Fall ist) Ich würde versuchen, eine Funktion zu schreiben, die isset() auf ihrem Parameter testet und, wenn ja, $ val setzt. Ich finde, dass elseif Komplexität hinzufügt; Ich versuche, sie zu vermeiden. In diesem Fall würde ich eine Funktion schreiben, die den Wert zurückgibt; dann können alle meine elseifs klar werden, wenn's geht.

Und selbstverständlich, in Ihrer aufrufenden Funktion, $ val dem Ergebnis dieser Funktion zuweisen.

1

Meiner Meinung nach ist Ihr Beispiel so sauber wie es nur geht. Sicher, Sie es als ein riesiger Einzeiler mit dem ternären Operator schreiben konnten:

$val = isset($vars[$input]) ? $vars[$input] : isset($this->getI['io'] ? $this->getI['io'] : isset($vars[5]) ? $vars[5] : 10; 

Aber das ist natürlich viel schwieriger zu lesen und zu halten, so dass das ursprüngliche Beispiel auf jeden Fall sauberer ist (obwohl es einige fehlen könnte Bemerkungen).

2
$val = 10; 
if (isset($vars[$input])) { 
    $val = $vars[$input]; 
} elseif (isset($this->getI['io'])) { 
    $val = $this->getI['io']; 
} elseif (isset($vars[5])) { 
    $val = $vars[5]; 
} 

Dies ist ungefähr so ​​einfach wie es geht, ohne den Code zu verschleiern. Ich würde eher versuchen, die Logik zu vereinfachen, es ist ein bisschen schwer zu verstehen, warum der Wert an so vielen verschiedenen Orten gesucht wird.

1

Ich weiß es nicht ... es scheint ziemlich prägnant zu sein, wie es ist.

Wenn Sie wissen, was es tut, es tut es gut und es ist sauber genug, dass Sie es in der Zukunft wieder herausfinden können, ich sage, es nicht anfassen.

1

Während Sie gerade dabei sind, herauszufinden, was es tut, und fügen Sie einige Kommentare hinzu.

z.B. wieso weise es der magischen Zahl 10 zu? vielleicht kann der Kontext des Restes etwas Licht werfen.

Soweit Code geht, werden Sie es nicht einfacher als das bekommen.