Nur ein Return pro Methode? Die Sache des Stills?

Question

In einem schlauen Buch habe ich mal gelesen, dass eine gut durchdachte Methode nur einen einzigen Ausgangspunkt (return;) haben soll.
Ich habe nun mal eine Methode, die ziemlich lang ist (sie zu zerlegen bringt kaum was..) und da komme ich auf mind. 3 Return(s). Wie steuert man solche Abläufe? Oder das mit "nur 1 return pro Methode" soll nicht so ortodox angewandt werden?

private void btnCreateIni_Click(object sender, EventArgs e)
    {

      
      IniForm inifrm = new IniForm();
      inifrm.label1.Text = strINIFile();
      
      if (inifrm.ShowDialog() == DialogResult.Cancel)
        return; //return no. 1

      SQLInstallation inst = new SQLInstallation();
      string sqlversion = this.txtInstallationsTyp.ToString();
      sqlversion = sqlversion.Replace(" ", "");
      if (sqlversion.Length < 11)
      {
        MessageBox.Show("Die Installationsbezeichnung ist unzulässig.\nDie zulässigen Einträge sind z. B.: 2005 SE 32 SP4.\nKorrigieren Sie den DBView-Eintrag", "Fehlerhafte Eingabe", MessageBoxButtons.OK, MessageBoxIcon.Error);
        return; //return no. 2
      }
      string dirCopy=inst.LWKopieren(sqlversion.Substring(0, 8), sqlversion.Substring(6, 5), this.lblAnzeigeInstallFiles.Text.ToString());
      if (dirCopy != String.Empty)
      {
        SaveFileDialog saveFileDialog1 = new SaveFileDialog();
        StreamWriter sw;
        saveFileDialog1.Filter = "*.INI Dateien|*.ini";
        saveFileDialog1.Title = "INI-Datei speichern";
        saveFileDialog1.InitialDirectory = "E:\\SQL-Install\\unatended\\";

        saveFileDialog1.ShowDialog();

        try
        {
          if (saveFileDialog1.ShowDialog() == DialogResult.OK)
          {
            FileStream fs = new FileStream(saveFileDialog1.FileName, FileMode.OpenOrCreate);
            sw = new StreamWriter(fs);
            //string zusammengebaut? 
            sw.WriteLine(strINIFile());
            sw.Close();
            fs.Close();
            MessageBox.Show("Die INI-Datei wurde erfolgreich erstellt.", "INI erstellen", MessageBoxButtons.OK, MessageBoxIcon.Information);

            //alle Textfelder löschen - noch eine INI erstellen?
            //clearInputs();
            //Button INI-Erstellen deaktivieren, da INI erstellt..
            btnCreateIni.Enabled = false;

            inst.SQLInstallStarten(dirCopy, saveFileDialog1.FileName.ToString());
            //Summarydatei umbenennen..
            inst.RenameFile(this.txtInstanzName.ToString(), this.cmbSQLWahl.ToString());

            inst.ServicesStoppen(this.txtInstanzName.ToString());
            string dirSP = this.lblAnzeigeInstallFiles.Text.ToString() + "\\" + this.cmbSP.Text.ToString();
            inst.SQLServicePackInstall(dirSP, this.txtInstanzName.Text.ToString());

          }
        }
        catch (Exception b)
        {
          MessageBox.Show("Es gab folgendes Problem während der Installation: " + b.Message, "Installationsfehler", MessageBoxButtons.OK, MessageBoxIcon.Error);
          return; //return no. 3
        }
      }
      else
      {
        MessageBox.Show("Das Installverzeichnis ist falsch oder leer.");
        return;
      }

    }

Danke für Eure Hilfe

Answer 1

servus,

mal abgesehen davon, das lang was anderes ist, bietet deine Methode jede Menge Optimierungspotential. Hier nur zwei konkrete Beispiele:

1. Anstatt

  if (inifrm.ShowDialog() == DialogResult.Cancel)
    return; //return no. 1

gehört hier her ein

  if (inifrm.ShowDialog() == DialogResult.OK)
  {
    // Restlicher Code
  }

Schon ein Return gespart. Das ganze wird durch die Refaktorisierung "Extract Method"

http://sourcemaking.com/refactoring/extract-method

angehübscht:

  if (inifrm.ShowDialog() == DialogResult.OK)
  {
    NewMethod()
  }

2. Das ist eine in sich geschlossene Prüfung auf eine Voraussetzung:

   SQLInstallation inst = new SQLInstallation();
   string sqlversion = this.txtInstallationsTyp.ToString();
   sqlversion = sqlversion.Replace(" ", "");
   if (sqlversion.Length < 11)
   {
    MessageBox.Show("Die .. DBView-Eintrag",
     "Fehlerhafte Eingabe",
      MessageBoxButtons.OK, MessageBoxIcon.Error);
    return; //return no. 2
   }

kann also ebenfalls durch "Extract Method" gelöst werden:

private bool CheckVersion()
{
  bool result = false;
  SQLInstallation inst = new SQLInstallation();
  string sqlversion = this.txtInstallationsTyp.ToString();
  sqlversion = sqlversion.Replace(" ", "");
  result = (sqlversion.Length < 11);
  if (result)
  {
   MessageBox.Show("Die .. DBView-Eintrag",
    "Fehlerhafte Eingabe",
    MessageBoxButtons.OK, MessageBoxIcon.Error);
  }
 return result;
}

und so geht es fast genauso weiter. btw, http://sourcemaking.com ist eine sehr schöne gemachte Seite, die viel must-knows gut aufbereitet.

---

Microsoft MVP Office Access
https://mvp.support.microsoft.com/profile/Stefan.Hoffmann

Answer 2

Hallo Stefan,

vielen Dank für die Tipps. Ich werde sie schnell umsetzen. Vielleicht entdecke ich "unterwegs" noch ein paar Möglichkeiten den Code klarer zu gestallten. Ich bin zwar ein Anfänger, muss ich aber (mit Euer Unterstützung) keine Anfängerfehler machen.. ;-)

Gruß

 

Answer 3

Hallo Ihrs,

ich finde mehrere Returns nicht unbedingt wild.

Es gibt Methoden (zB die selbstgebaute SFTP Libary) die nutzt Returns um am Anfang abzubrechen ohne Exeption

zB

void TueEtwas()
{
 if(!IstBereit())return;
 if(!istEingeloggt())return;
 //MACHE HIER WAS 
}

Answer 4

Hallo Pawel,

ja, diese (Deine) Methode (mit return bei Ausstiegskriterien) ist auch meiner Ansicht nach oft eher empfehlenswert. Man sollte immer beachten, dass es konkurriende Werte in Code-Entwicklung gibt!
Einer der Vorteile ist, dass man eine Verschachtelung von if-else vermeidet, was wiederum CleanCode Prinzipien folgt. Ebenfalls geht der weitere Code von einer reduzierten Bedingungs-Matrix aus, was ebenfalls die Verständlichkeit erleichtert. Es ist unbenommen - man hat Vorteile und Nachteile durch ein einiziges return. Man sollte seine Wahl halt letztlich möglichst gut begründen können.

BTW: die genannte Methoden-Extraktion ist natürlich u.a. auch ein gutes Mittel.
Es gibt aber weitere und P. kann hier sicher in der Benennung auch noch einiges herausholen.

Die IMHO etwas veraltete Regel mit einem einzigen Auststiegspunkt hat sich heutzutage etwas zugunsten mehrerer Ausstiegspunkte (wenn erforderlich) geändert, obwohl beide Standpunkte durchaus verständlich sind.

[language agnostic - Should a function have only one return statement? - Stack Overflow]
http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement
-> "So yes, I think it's fine to have multiple "exit points" from a function/method."

---

ciao Frank