plan9soft / encarnium

Automatically exported from code.google.com/p/encarnium
0 stars 0 forks source link

Code Style: Verschachtelte If-Verzweigungen schlecht nachvollziehbar #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hallo allerseits. Ich schaue gerade euren Code durch und ich hoffe ihr habt 
nichts dagegen, dass ich ihn hier ein wenig kommentiere.

Eine Sache, die mir auffiel ist, dass ihr gerne mit verschachtelten 
If-Abzweigungen arbeitet. Grundsätzlich sind sie bis zu dem eigentlichen 
Ereignis immer lesbar, jedoch die Else-Fälle danach sind immer problematisch 
zu lesen.

Einfaches Beispiel in svn/trunk/includes/classes/Character.php, ab Zeile 124:

{{{
        public function checkLevel($char_id) {
                $current_level = $this->dao->getLevel($char_id);
                if($current_level["level"] != $this->ini["maxlevel"]) {
                        if($current_level["ep"] >= $this->ini["level"][$current_level["level"]+1]) {
                                if($this->dao->nextLevel($char_id)) {
                                        $this->level = $current_level["level"]+1;
                                        return true;
                                }
                                else return false;
                        }
                        else return false;
                }
                else return false;
        }
}}}

Hier sind die Else-Fälle relativ einfach und die True-Anweisung jeweils nur 2 
Zeilen. Problematischer wird es jedoch bei längerem Code über eine 
Bildschirmzeile hinweg oder bei längeren Else-Fällen.

Vorschlag: wieso nicht auf die Negativ-Fälle einzeln prüfen?

{{{
        public function checkLevel($char_id) {
                $current_level = $this->dao->getLevel($char_id);

        // prüfen, ob Charakter das Maxlevel erreicht hat
        if($current_level["level"] == $this->ini["maxlevel"]) {
            return false;
        }

        // prüfen, ob Charakter ..
                if($current_level["ep"] < $this->ini["level"][$current_level["level"]+1])
        {
            return false;
        }

        // prüfen, ob Charakter ..
        if(!$this->dao->nextLevel($char_id)) {
            return false;
        }

        $this->level = $current_level["level"]+1;
        return true;

        }
}}}

Wie Anfangs gesagt ist das keine Issue, sondern ich schreibe nur runter, was 
mir auffällt (bei Google Code kann ich als externer Ersteller nur leider die 
Prio und Bugtyp nicht auf niedrig und Vorschlag stellen).

Original issue reported on code.google.com by DirkSong...@googlemail.com on 19 Aug 2010 at 11:56

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago

Original comment by gilles86 on 22 Oct 2012 at 3:57