Revize kódu Hanojských věží – .NET – Fórum – Programujte.com

Ahoj, dostal jsem po Pacmanovi za úkol zpracovat "hru" Hanojské věže. Mělo to být udělané procedurálně bez oop a neměl jsem koukat na algoritmy na netu (dodržel jsem). Rád bych zdejší znalejší poprosil o revizi toho mého výtvoru. Funguje jak má. Otázkou je, je napsaný efektivně? Za každých pár vět, které zkritizují nějaký nešvar, budu vděčný. 

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace Veze
{
    class Program
    {
        static Stack<int> vezA = new Stack<int>();
        static Stack<int> vezB = new Stack<int>();
        static Stack<int> vezC = new Stack<int>();
        static Stack<int> prac = new Stack<int>();
        static bool konec = false;
        static int pocetTahu = 0;

        static void Main(string[] args)
        {
            // příprava základní věže
            vezA.Push(5);
            vezA.Push(4);
            vezA.Push(3);
            vezA.Push(2);
            vezA.Push(1);
            int ber;
            int poloz;
            Vypis();
                       
            while (!konec)   // herní smyčka
            {
                do           // cyklus ověřuje vstup - pouze čísla 1,2,3
                {
                    Console.WriteLine();                               
                    Console.WriteLine("Přesunout kotouč z věže: ");
                    if (int.TryParse(Console.ReadLine(), out ber) && ber > 0 && ber < 4)
                    {
                        break;
                    }
                    Console.WriteLine("Nesprávné zadání!");
                    Console.WriteLine("Nesprávné zadání! Pokračuj stisknutím libovolné klávesy.");
                    Console.ReadKey();
                    Vypis();
                }
                while (true);

                do          //// cyklus ověřuje vstup - pouze čísla 1,2,3
                {
                    Console.WriteLine("Polož kotouč na věž: ");
                    if (int.TryParse(Console.ReadLine(), out poloz) && poloz > 0 && poloz < 4)
                    {
                        break;
                    }
                    Console.WriteLine("Nesprávné zadání! Pokračuj stisknutím libovolné klávesy.");
                    Console.ReadKey();
                    Vypis();
                }
                while (true);

                if (Legalni(ber, poloz))    //volám metodu, abych ověřil, je li tah podle pravidel
                {
                    ProvedTah(ber, poloz);  // pokud ano, provede se tah
                    pocetTahu++;
                    Vypis();                // aktualizace obrazovky
                }
                else
                {
                    Console.WriteLine("Neplatný tah! Pokračuj stisknutím libovolné klávesy.");
                    Console.ReadKey();
                    Vypis();
                }                    
            }
            Console.WriteLine("Vyhrál jsi, potřeboval jsi {0} tahů.", pocetTahu);   //hráč splnil podmínky, herní smyčka končí
        }

        //***********************************************************************************************//
        //***********************************************************************************************//

        static bool Legalni(int ber, int poloz)     //metoda posoudí tah podle pravidel 
        {                                           //V jednom tahu lze přemístit pouze jeden kotouč
            bool tahJeOK = false;                   //Vždy se bere vrchní kotouč z některé věže a pokládá se na vrchol jiné věže
            int kamen = 0;                          //Nesmí se položit větší kotouč na menší
            switch (ber)
            {
                case 1:
                    if (vezA.Count > 0)
                    {
                        kamen = vezA.Peek();
                        tahJeOK = true;
                    }
                    break;

                case 2:
                    if (vezB.Count > 0)
                    {
                        kamen = vezB.Peek();
                        tahJeOK = true;
                    }
                    break;

                case 3:
                    if (vezC.Count > 0)
                    {
                        kamen = vezC.Peek();
                        tahJeOK = true;
                    }
                    break;
            }

            if (!tahJeOK)
                return false;

            switch (poloz)
            {
                case 1:
                    if (vezA.Count == 0)
                        tahJeOK = true;
                    else
                       if ((vezA.Peek() > kamen) && (vezA.Count + kamen < 6))
                    {
                        tahJeOK = true;
                    }
                    else tahJeOK = false;
                    break;
                case 2:
                    if (vezB.Count == 0)
                        tahJeOK = true;
                    else
                    if ((vezB.Peek() > kamen) && (vezB.Count + kamen < 6))
                    {
                        tahJeOK = true;
                    }
                    else tahJeOK = false;
                    break;
                case 3:
                    if (vezC.Count == 0)
                        tahJeOK = true;
                    else
                    if ((vezC.Peek() > kamen) && (vezB.Count + kamen < 6))
                    {
                        tahJeOK = true;
                    }
                    else tahJeOK = false;
                    break;
            }
            if (tahJeOK) return true;
            else return false;
        }

        static void ProvedTah(int ber, int poloz)           //hráčův záměr je legální, provedeme tedy tah
        {
            int kamen = 0;
            switch (ber)
            {
                case 1:
                    {
                        kamen = vezA.Pop();
                        break;
                    }
                case 2:
                    {
                        kamen = vezB.Pop();
                        break;
                    }
                case 3:
                    {
                        kamen = vezC.Pop();
                        break;
                    }
            }
            switch (poloz)
            {
                case 1:
                    vezA.Push(kamen);
                    break;
                case 2:
                    vezB.Push(kamen);
                    if (vezB.Count == 5)
                        konec = true;
                    break;
                case 3:
                    vezC.Push(kamen);
                    if (vezC.Count == 5)
                        konec = true;
                    break;
            }
        }

        static void Vypis()                                 //vykreslí konzoli
        {
            int[] poleA = new int[5] { 0, 0, 0, 0, 0 };     //stacky věží převedu na pole abych mohl pracovat s objekty s pevným počtem prvků
            int[] poleB = new int[5] { 0, 0, 0, 0, 0 };
            int[] poleC = new int[5] { 0, 0, 0, 0, 0 };
            string strVezA = "          ";
            string strVezB = "          ";
            string strVezC = "          ";
            string mezera = "";
            string mezerab = "";
            string mezerac = "";
            string radek = "";
            int nasobek = 0;
            int j = 0;

            foreach (var item in vezA)                      //zde se to převádí na ty pole
            {
                poleA[j] = item;
                j++;
            }
            j = 0;
            foreach (var item in vezB)
            {
                poleB[j] = item;
                j++;
            }
            j = 0;
            foreach (var item in vezC)
            {
                poleC[j] = item;
                j++;
            }
            Array.Sort(poleA); Array.Reverse(poleA);        // srovnám pole podle velikosti a otočím, aby začínalo nejvyšší hodnotou
            Array.Sort(poleB); Array.Reverse(poleB);
            Array.Sort(poleC); Array.Reverse(poleC);

            Console.Clear();
            Console.WriteLine("Na věži A je {0} kamenů, na věži B je {1} kamenů a na Věži C je {2} kamenů. Počet legálních tahů: {3}", vezA.Count, vezB.Count, vezC.Count, pocetTahu);
            Console.WriteLine();
            Console.WriteLine("   Věž A      Věž B      Věž C");
            Console.WriteLine();

            for (int i = 4; i >= 0; i--)                    //cyklus vykresluje věže. Vypočítá, kolik mezer je třeba přidat z obou stran, aby věž byla symetrická
            {
                nasobek = (10 - (2 * poleA[i])) / 2;
                mezera = new string(' ', nasobek);
                nasobek = (10 - (2 * poleB[i])) / 2;
                mezerab = new string(' ', nasobek);
                nasobek = (10 - (2 * poleC[i])) / 2;
                mezerac = new string(' ', nasobek);

                strVezA = mezera + new string('█', 2 * poleA[i]) + mezera;
                strVezB = mezerab + new string('█', 2 * poleB[i]) + mezerab;
                strVezC = mezerac + new string('█', 2 * poleC[i]) + mezerac;

                radek = strVezA + " " + strVezB + " " + strVezC;
                Console.WriteLine(radek);
            }
        }
    }
}

#1 KARLOSCZ1979
já bych ta pole apd. do polí o velikosti 3, místo ..A ..B ..C  bych pak použil index, tím by se kód podstatně zkrátil

Já o tom přemýšlel, ale zkrátilo by se to max o tři čtyři řádky za cenu přehlednosti. Tak jsem se rozhodl pole 3*5 nepoužít. Jo kdyby tam těch věží bylo 20, to už by samozřejmě muselo jít přes jedno pole a indexování. Je to velká chyba?

#1 KARLOSCZ1979
Ne není to chyba. První co mne napadlo když jsem se podíval na Tvůj kód, bylo proč jsi to neudělal takto: 

// A B C názvy nejsou ideální, já vím... 
const int A=0, B=1, C=2; //pokud budeš potřebovat přistupovat k indexům, dáš vez[B] a nespleteš se
static var vez = new Stack<int>[] { new Stack<int>(), new Stack<int>(), new Stack<int>() };

        static bool Legalni(int ber, int poloz)     //metoda posoudí tah podle pravidel 
        {                                           //V jednom tahu lze přemístit pouze jeden kotouč
            ber--;
            poloz--; // zdůvodu indexování od nuly
            bool tahJeOK = false;                   //Vždy se bere vrchní kotouč z některé věže a pokládá se na vrchol jiné věže
            int kamen = 0;                          //Nesmí se položit větší kotouč na menší

            if(vez[ber].Count > 0)
            {
                kamen = vez[ber].Peek();
                tahJeOK = true;
            }

            if (!tahJeOK) return false;


            if (vez[poloz].Count == 0) tahJeOK = true;
            else
            {
                if ((vez[poloz].Peek() > kamen) && (vez[poloz].Count + kamen < 6))
                {
                    tahJeOK = true;
                }
            }

            return tahJeOK;
        }

        static void ProvedTah(int ber, int poloz)           //hráčův záměr je legální, provedeme tedy tah
        {
            int kamen = vez[ber-1].Pop();
            vez[poloz-1].Push(kamen);
        }

kód se tím zkrátil tak o dvacet řádků, tím by jsi měl méně práce. (Uvedený kód může mít chybu, nezkoušel jsem to)

S těmi poli a stringy je to stejné... Všímal jsem si pouze vizuální stránky kódu, zkoumat jak pracuje a co vše je vněm nutné, na to jsem líný.

Tak tohle jsem tam neviděl - díky moc!

#5 KARLOSCZ1979
Čekal jsem, že to zhodnotí nějaký profík, zatím nikdo nic nepsal, tak ještě přidám celkové hodnocení.
Nelíbí se mi to provedení celkově, dávat ty položky do stacku, pak to kopírovat do pole, řadit a pak ještě převracet...
Vem si kdyby jsi dělal něco náročnějšího, kolik by jsi tímto zaplácal paměti a výkonu..

Proc mas tak slozity kod?
- Funkce1: Na zacatku potrebujes naplnit veze (pole). Bud jednu 1-n, nebo vsechny 3, podle zadani (seznam poli s hodnotami).
- Funkce2: Zjisti, zda je tah z veze na vez mozny, tahCheck(a, b).
- f3: tahProved(a, b)
- f4: zobraz stav
- f5: test ukonceni, zda na vybrane vezi je pozadovana sestava

A, pokud to ma delat automaticky, tak ten algoritmus je znamy a opakuje se stale dokola
tahProved(0, 1)
tahProved(0, 2)
tahProved(1, 2) nebo je to nejak jinak, uz nevim, google.
Tak potrebujes jen casovac a zobrazovat to prubezne. idealne s nejakou animaci, aby clovek postrehl, odkud kam putuje disk z veze na vez. nebo pridat blikajici sipky a pak presunout.
 

Pokod to nema byt v Oop, proc to mas obalne v class?
 

Proč je to dělené do funkčních celků? Protože všichni autoři, které jsem kdy četl, vždycky zdůrazňovali dělit logiku podle funkčních celků. Když nastane chyba, snadno dopátráš, v které metodě a tam máš daleko větší přehled. Nešpagetovat mi přijde jako naprostý základ. To obalení třídou nechápu co řešíš. VS tak nalinkuje prázdné řešení a všechno je v main nebo nad main. Žádný objekt ze třídy se tam nevytváří. Tak ještě jsem měl vstup od uživatele zabalit do metody a herní smyčka by se zkrátila na třetinu. Nemá to dělat automaticky, normálně to jde hrát. Kód je funkční, hod to do VS a pomocí 1,2,3 si zahraj:-)

#7 peter on to dělal tak, že hraje uživatel, ne PC!

#1 KARLOSCZ1979
a ještě jedna věc která mě napadla je: Věže označuješ jako A B C, ale zadáváš 1 2 3. To může být pro někoho matoucí. Doporučoval bych věže přejmenovat na Věž 1, Věž 2, Věž 3.