Stack Overflow

[MatrixCode84]

Der Name ist Programm!

Mein XOX Programm läuft soweit und funktioniert, jedoch gefällt mir die Methode PlayerInput noch nicht so ganz.

Folgendes Problem:

Nehmen wir mal an es setzt sich ein Prof an diese Funktion und

• lässt sie durch einen Unittest laufen oder
• tippt 1000 mal eine Falsche Zahl ein (wir gehen hier mal davon aus das nur Intenger eingegeben werden)

dann habe ich in kürzester Zeit einen Stack Overflow

Ich bitte um eure Hilfe um eine vernünftige Lösung zu finden. Ich habe mir auch schon gedanken über eine while - Schleife gemacht, jedoch besteht hier auch bei falscher behandlung die Gefahr nicht mehr raus zu kommen. Und ich wollte den Code auch nicht weiter aufblähen. Sieht bis jetzt ganz nach meinem Geschmack aus.

Bin dankbar für jeden guten Tip!

Der Code-Snippet

static int PlayerInput()
{
    Console.Write(">");
    int input = Int32.Parse(Console.ReadLine()) -1;

    if(input > 8 || input < 0 || grid[input].Equals('x') || grid[input].Equals('o'))
    {
        Console.WriteLine("Field is out of range or already set!");
        Console.WriteLine("Try again:");
        return input = PlayerInput();
    }

    return input;
}

[JirkaDellOro]

Mein erster Tipp wäre zu schauen, was man nach den Regeln des CleanCode von einer Methode mit Namen "PlayerInput" erwartet.

[MatrixCode84]

Ok Name wurde geändert von PlayerInput zu ChooseField. Ändert leider nichts am Problem

[JirkaDellOro]

Was erwartet man von einer Methode "ChooseField"?

[MatrixCode84]

Das ein Feld gewählt wird. Ok, in dem Fall automatisch. Wenn ich sie PlayerChooseField nenne, erwarte ich dann das der Spieler das Feld auswählt.

[JirkaDellOro]

Hmm... das kommt in der Lektion zu CleanCode nicht richtig rüber, Ich trage mir selbst einen Issue ein.

Funktions- oder Methodennamen sollen immer eine Aktivität des Systems beschreiben!

(Letztlich ergibt sich das bei der Planung durch die Erstellung des Aktivitätsdiagramms)

[MatrixCode84]

Das System erwartet einen Input vom Spieler, dieser soll einen Wert zwischen 1 und 9 haben. Der Wert wird dann in einer anderen Methode weiter verarbeitet um ein Feld im Spielraster als 'X' oder 'O' zu setzten. Dann müsste ich die Methode GetInputFromPlayerToSetField nennen.

[JirkaDellOro]

Dann würden wir der Sache näher kommen. Ich habe aber den Eindruck, die Methode tut deutlich mehr und so stellt sich die Frage ob sie der Regel bezüglich der Abstraktionsebenen, dem SOC und dem Single Responsibilty Prinzip gerecht wird.

[MatrixCode84]

Ich verstehe dann packe ich die überprüfung in eine extra Methode die dann InputCheck heißt.

[JirkaDellOro]

Du hast auch noch Kommunikation mit dem Spieler dabei...

[MatrixCode84]

        static void CallToAction()
        {
            Console.WriteLine("Player " + player + " choose your Field");
            Console.Write(">");
        }
        static int PlayerChooseField()
        {
            return input = Int32.Parse(Console.ReadLine()) -1;
        }

        static void InputCheck()
        {
            if(input > 8 || input < 0 || grid[input].Equals('X') || grid[input].Equals('O'))
            {
                Console.WriteLine("Field is out of range or already set!");
                Console.WriteLine("Try again:");
                PlayerChooseField();
            }
        }

[MatrixCode84]

ok ich sehe gerade ich muss es noch weiter runterbrechen

[MatrixCode84]

        static void CallToAction()
        {
            Console.WriteLine("Player " + player + " choose your Field");
            Console.Write(">");
        }
        static int GetInputFromPlayerToSetField()
        {
            return input = Int32.Parse(Console.ReadLine()) -1;
        }

        static void InputCheck()
        {
            while(input > 8 || input < 0 || grid[input].Equals('X') || grid[input].Equals('O'))
            {
                ShowInputErrorMessage();
                GetInputFromPlayerToSetField();
            }
        }

        static void ShowInputErrorMessage()
        {
            Console.WriteLine("Field is out of range or already set!");
            Console.WriteLine("Player " + player + " try again:");
        }

Jetzt habe ich alles auf die kleinste Ebene herunter gebrochen. Mein Code ist zwar weiter gewachsen. Aber jetzt habe ich nur eine While-Schleife. Ich habe hier jetzt, denke ich, wirklich "Clean Code". Problem gelöst

[JirkaDellOro]

Schön, dass dich ein paar Grundregeln voran bringen. Das hatte ich gehofft. Inwiefern ist dein ursprüngliches Problem jetzt gelöst?

[MatrixCode84]

Ich habe keine Rekursion mehr sondern eine While-Loop. Ist, denke ich, von der Performance besser. Und ich bekomme keinen Stack Overflow

[JirkaDellOro]

Genau! 👍