Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Ich habe mich heute mal einfach an die FizzBuzz Kata gesetzt und nach TDD gelöst. FizzBuzz ist meiner Meinung nach die einfachste Kata, die ich hier verwenden möchte um mein Anliegen kund zu tun.

Ich möchte an dieser Stelle nicht alle Schritte erläutern sondern einfach die Frage in den Raum stellen, wann der Code einer Komponente als “clean” gilt.

Um alle Anforderungen der FizzBuzz Kata abzudecken habe ich die folgenden Testmethoden implementiert.

[TestMethod]
public void Fizz_Is_Returned_If_multiple_of_3()
{
    // arrange
    var fizzBuzz = new FizzBuzz();
    var expected = "Fizz";
    // act
    var valueForThree = fizzBuzz.StringFor(3);
    var valueOfSix = fizzBuzz.StringFor(6);
    //assert
    Assert.AreEqual(expected,valueForThree);
    Assert.AreEqual(expected,valueOfSix);
}
 
[TestMethod]
public void Buzz_Is_Returned_If_multiple_of_5()
{
    // arrange
    var fizzBuzz = new FizzBuzz();
    var expected = "Buzz";
    // act
    var valueForFive = fizzBuzz.StringFor(5);
    var valueForTen = fizzBuzz.StringFor(10);
    // assert
    Assert.AreEqual(expected,valueForFive);
    Assert.AreEqual(expected,valueForTen);
}
 
[TestMethod]
public void FizzBuzz_Is_Returned_If_multiple_of_3_and_5()
{
   // arrange
    var fizzBuzz = new FizzBuzz();
    var expected = "FizzBuzz";
   // act
    var valueFor15 = fizzBuzz.StringFor(15);
    var valueFor45 = fizzBuzz.StringFor(45);
   // assert
    Assert.AreEqual(expected, valueFor15);
    Assert.AreEqual(expected,valueFor45);
}
 
[TestMethod]
[ExpectedException(typeof(ArgumentOutOfRangeException))]
public void Throws_Exception_if_number_lower_than_1()
{
    var fizzBuzz = new FizzBuzz();
    fizzBuzz.StringFor(0);
}
 
[TestMethod]
[ExpectedException(typeof(ArgumentOutOfRangeException))]
public void Throws_Exception_if_number_is_more_than_100()
{
    var fizzBuzz = new FizzBuzz();
    fizzBuzz.StringFor(101);
}

 

Hierbei bin ich wie bei TDD gewohnt Red-Green-Refactor vorgegangen, so dass ich schnell zu einem “Gesamtgrünen” Ergebnis kam, welches wie folgt implementiert ist.

 

class FizzBuzz
{
  private const String Fizz = "Fizz";
  private const String Buzz = "Buzz";
  
  public string StringFor(int number)
  {
      if (number < 1 || number > 100)
        throw new ArgumentOutOfRangeException("number", number, 
             "Number out of Range (1-100)");
      if (number % 3 == 0 && number % 5 == 0)
        return Fizz + Buzz;
      if(number %3 == 0)
        return Fizz;
      if (number % 5 == 0)
        return Buzz;
      return number.ToString();
  }
}

 

Eigentlich wäre ich nun an einem Punkt wo ich sagen würde, dass mein Code funktioniert und sauber ist. Doch man kann natürlich nun alle Operationen wie die Prüfung ob die Number innerhalb der Range ist oder die Modulo-Vergleiche aus der eigentlichen Methode StringFor herausziehen.

Bei der Modulo-Operation sehe ich dass ja noch ein, wodurch sich folgendes ergibt:

 

class FizzBuzz
{
    private const String Fizz = "Fizz";
    private const String Buzz = "Buzz";
 
    public string StringFor(int number)
    {
        if (number < 1 || number > 100)
            throw new ArgumentOutOfRangeException("number", number, 
               "Number out of Range (1-100)");
        if (IsDivisibleByThree(number) && IsDivisibleByFive(number))
            return Fizz + Buzz;
        if(IsDivisibleByThree(number))
            return Fizz;
        if (IsDivisibleByFive(number))
            return Buzz;
        return number.ToString();
    }
 
    private static bool IsDivisibleByThree(int number)
    {
        return number%3 == 0;
    }
 
    private static bool IsDivisibleByFive(int number)
    {
        return number%5 == 0;
    }
}

 

Ist mein Code jetzt “clean” oder sollte ich eine Methode IsDivisibleBy(int number, int divideBy) implementieren im Stil

private static bool IsDivisibleBy(int number, int divideBy)
{
   return number % divideBy == 0;
}

 

Sollte man eventuell noch die Range-Validierung auslagern in eine ValidateNumber(int number) Methode im Sinne von

private void ValidateNumber(int number)
{
    if(number<1)
        throw new ArgumentOutOfRangeException("number",number,"Number have to be larger than 0");
    if(number > 100)
        throw  new ArgumentOutOfRangeException("number",number,"Number have to be less than 101");
}

 

Führt das nicht zu weit?

Wann sollte man mit dem Refactoring aufhören? Wann sind die Operationen atomar genug? Klar kann man alles in eigene Methoden kapseln aber was ist mit YAGNI ??

 

Für ein paar Ideen oder Anregungen wäre ich sehr dankbar. Ich denke nicht, dass ich der einzige bin, der hier diese schwammige Grenze nicht richtig ziehen kann. Vielmehr sehe ich hier ein menschliches Problem denn wer zieht schon gerne und sicher Grenzen wenn eigentlich keine da sind.

 

Technorati-Tags: ,,,
DotNetKicks-DE Image
Published Montag, 26. Juli 2010 17:44 von ThorstenHans
Abgelegt unter: ,

Kommentare

# re: Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Montag, 26. Juli 2010 17:51 von Rainer Hilmer

Meine ganz persönliche Meinung: number % 3 lässt sich genau so gut lesen wie IsDivisibleByThree. Letzteres ist also too much und besser wartbar wird der Code dadurch auch nicht. Im Gegenteil, ich finde er wird dadurch sogar unflexibler. Was ist, wenn du beim nächsten mal prüfen muss ob etwas ohne Rest durch 7 teilbar ist und nicht mehr durch 3? Dann müsstest du eine neue Methode schreiben und die alte in die Tonne treten. Diese ganzen Minimethden basen den Code nur unnötig auf und dadurch wird das Gesamtbild sogar schlechter lesbar weil unübersichtlicher.

# re: Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Montag, 26. Juli 2010 18:57 von Albert Weinert

Anstatt IsDivisibleByThree() nimmt man IsFizz() und entsprechend IsBuzz() als Methodennamen. Somit wird ein Schuh draus.

Der Methodenname sagt aus was er macht. Nicht wie er was macht.

# re: Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Mittwoch, 28. Juli 2010 11:32 von Michael Ertelt

Vermutlich ist kein Code jemals clean. Wir können verschiedene Praktiken anwenden und irgendwann zufrieden sein, oder eben nicht. Diese Einschätzung wird subjektiv bleiben, ebenso wie Ansichten ob es an einem realen Ort sauber genug ist. Ich frage mich, wer etwas von der jeweiligen Refaktorisierung hat und welcher Aufwand dahinter steht. Momentan höre ich auf, wenn das Gefühl von "clean Code" möglichst ausgeprägt ist und sich das YAGNI Gefühl meldet.

# re: Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Dienstag, 3. August 2010 01:54 von Rainer Schuster

Es geht aber nicht um die subjektive Freude, sondern die Objektive Messbarkeit. Und die heißt, explizit ist besser als implizit. Wenn ein Mindmapping beim lesen eines Quellcodes passiert, weil nicht das steht was gemacht wird (sonder wie), dann ist das messbar. Code wird 70-80% der Zeit gelesen. Wenn ich etwas nicht verstehen kann oder 50 statt 5 Minuten brauche, dann ist das messbar.

Wenn ich aufgrund des unleserlichen Codes Fehler bei der Implementierung mache, oder falsche Annahmen mache, dann ist das messbar.

Mein Grundregeln lauten:

1. Ist es funktional?

2. ist es automatisiert testbar?

3. ist es explizit/verstehe ich das in einer Woche, einem Monat, in einem Jahr noch? Oder gar der Kollege.

4. Kann ich es leicht anpassen? (Zeitverbrauch und Sicherheit beim ändern der Komponente)

Werden alle Belange mit ja beantwortet. Wirst du sicherlich die ganze Kunst des TDD auf Codeebene erledigt haben. Denn, so bin ich mir sicher, wenn die Fragen mit ja beantwortet wurden, sollten auch die Prinzipien und Regeln von CCD eingehalten worden sein. Ist es nicht der Fall, wird wohl eine Frage mit nein beantwortet worden sein.

# re: Wann ist mein Code clean? – Exemplarisch anhand der FizzBuzz Kata

Mittwoch, 4. August 2010 08:37 von Michael Ertelt

@Rainer: Objektive Messbarkeit, wie soll das gehen? Ich stimme zu das Code mehr gelesen wird und deshalb lesbar sein sollte, auch testbar sollte er sein. Der Code kann aber (wie oben im FizzBuzz Beispiel lesbar sein) und trotzdem kann man immer weiter daran arbeiten den Code "clean" zu machen. Die Frage war, wann hört man auf.

Dein Regeln (ausgenommen Regel 2) kann ich auch nur subjektiv beantworten. Ich würde es gut finden objektive Messbarkeit zu erreichen. Vielleicht sollten wir an den Regeln noch etwas arbeiten.

Kommentar abgeben

(verpflichtend) 
(verpflichtend) 
(optional)
(verpflichtend)