Torpedó vizsgálat hiba Console Application C#

Címkék
Torpedó vizsgálat hiba Console Application C#
2012-07-09T16:38:03+02:00
2012-07-12T15:10:39+02:00
2022-11-26T07:30:41+01:00
Marcicerus
Sziasztok!
Egy torpedó játékot írok C#-ban, de a hajók elhelyezésénél valami galiba van, nemegyszer egymásba csúsznak a hajók(még csak a gép rakod).
using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace Battleship_v2._0 { class Program { static int[,] fp; static int[,] fc; static int[] optf; static int[] opts; static int[] fffuuu; static int f = 0; static int s = 0; static int length = 0; static Random m = new Random(); static void Main(string[] args) { Start(); } static void Start() { fp = new int[12, 12]; fc = new int[12, 12]; PosComp(); } static void PosComp() { for (int i = 0; i < 5; i++) { optf = new int[0]; opts = new int[0]; length = Length(i); f = m.Next(1, 11); s = m.Next(1, 11); bool chk = chk33(1); if (chk == true) { Options(); chkfull(1); if (fffuuu.Length > 0) { int pos = m.Next(0, fffuuu.Length); PlaceComp(fffuuu[pos]); } else i--; } else i--; Console.Clear(); } for (int r = 0; r < fc.GetLength(0); r++) { for (int j = 0; j < fc.GetLength(1); j++) Console.Write(fc[r, j] + " "); Console.WriteLine(); } Console.ReadKey(); } static void PlaceComp(int q) { int f1 = f; int s1 = s; int f2 = optf[q]; int s2 = opts[q]; if (f2 < f1) { int other = f1; f1 = f2; f2 = other; } if (s2 < s1) { int other = s1; s1 = s2; s2 = other; } for (int i = f1; i < f2 + 1; i++) for (int j = s1; j < s2 + 1; j++) fc[i, j] = 2; } static void chkfull(int a) { int[,] fo; if (a == 0) fo = fp; else fo = fc; fffuuu = new int[0]; for (int q = 0; q < optf.Length; q++) { bool k = true; for (int i = f - 1; i < optf[q] + 1; i++) for (int j = s - 1; j < opts[q] + 1; j++) { if (fo[i, j] == 2) { k = false; } } if (k == true) { Array.Resize(ref fffuuu, fffuuu.Length + 1); fffuuu[fffuuu.Length - 1] = q; } } } static void Options() { if (f - length > -1) { Array.Resize(ref optf, optf.Length + 1); optf[optf.Length - 1] = (f - length + 1); Array.Resize(ref opts, opts.Length + 1); opts[opts.Length - 1] = s; } if (f + length < 12) { Array.Resize(ref optf, optf.Length + 1); optf[optf.Length - 1] = f + length - 1; Array.Resize(ref opts, opts.Length + 1); opts[opts.Length - 1] = s; } if (s - length > (-1)) { Array.Resize(ref opts, opts.Length + 1); opts[opts.Length - 1] = s - length + 1; Array.Resize(ref optf, optf.Length + 1); optf[optf.Length - 1] = f; } if (s + length < 12) { Array.Resize(ref opts, opts.Length + 1); opts[opts.Length - 1] = s + length - 1; Array.Resize(ref optf, optf.Length + 1); optf[optf.Length - 1] = f; } } static bool chk33(int a) { int[,] fo; if (a == 0) fo = fp; else fo = fc; for (int i = f - 1; i < f + 2; i++) for (int j = s - 1; j < s + 2; j++) if (fo[i, j] == 2) return false; return true; } static int Length(int i) { switch (i) { case 0: i = 5; break; case 1: i = 4; break; case 2: i = 3; break; case 3: i = 3; break; case 4: i = 2; break; default: i = 0; break; } return i; } } }
Szerintetek mi lehet a hiba?
Üdv: Marcicerus
Mutasd a teljes hozzászólást!
Töröld ki az OptionSecond metódusból a felfelé és a balra elhelyezett hajókat.
static void Optionsecond() { if (first + length < 12) { Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first + length - 1; Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second; } if (second + length < 12) { Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second + length - 1; Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first; } }

Gondold csak végig, hogy a Checkfullfield metódusban mekkora területet ellenőriznél egy balra vagy felfelé elhelyezett hajónál.
Tehát az ellenőrző ciklusban azt is figyelni kéne, hogy a kezdőponttól merre van a hajó végpontja, és ez alapján kéne a ciklus intervallumát meghatározni.
De mivel a jobbra vagy balra elhelyezett hajó ugyan úgy néz ki, ezért az egyiket nyugodtan elhagyhatjuk.

Másrészt a ciklus intervallumának meghatározása a jobbra vagy lefele elhelyezett hajóknál is hibás.
for (int i = first - 1; i < optionsfirst[q] + 2; i++) for (int j = second - 1; j < optionssecond[q] + 2; j++)

Tehát így + 2 -nek kellene szerepelnie benne a + 1 helyett.

Egy-két további észrevétel.
1. A Length metódust le kéne cserélni egy konstans tömbre. Ez így elég hülyén néz ki.
2. Tömböket használsz az adatok tárolására és ha beleakarsz tenni valamit, akkor folyton átméretezed. Használj helyette inkább listákat, azoknak is van length (vagyis count) metódusa amivel megtudhatod, hogy mennyi elem van benne, és sokkal egyszerűbb használni őket.
3. Használj összetett típusokat. A first/second és a optionfirst/optionsecond ugye koordinátákat jelölnek, ezeket egyrészt jobb lenne X és Y -nek hívni, másrészt kezelhetnéd őket együtt mondjuk egy Point objektummal. És akkor nem kéne két tömböt karbantartani.
4. Console.Clear utasítást a hajó elhelyező ciklusba tetted, miért?
5. Eszméletlenül túlbonyolítottad a programot.
Ez egy viszonylag egyszerű feladat, de sikerült annyira túlbonyolítani, hogy alig lehet átlátni.
Egy valóban bonyolult feladatnál, hogy fog kinézni a programod?

A programodból látszik (ha te csináltad), hogy van érzéked a programozáshoz, de az is látszik, hogy eléggé kezdő vagy még (ami persze nem baj).
Mutasd a teljes hozzászólást!

  • Nos, őszintén szólva fogalmam sincs miért nem műküdök a programod, de a kódoddal kapcsolatban lenne néhány apróbb megjegyzésem.
    (kérlek ezt most ne trollkodásnak vedd, hanem építő jellegű kritikának!)

    1. A C# egy OO nyelv, és mint ilyen érdemes osztályokat használni a programozáshoz. A programod jelen állapotában akár C-ben is íródhatott volna.

    2. Érdemes dokumentálni, leírni, hogy melyik függvény, változó mire használatos.

    3. A dokumentáció egy részét megspórolhatod, ha beszédes neveket használsz. Többek között pl. a fffuuu változódról és a check33 függvényedről sem lehet tudni, hogy mire jó.

    4. Amikor kérdezel jó lenne ha leírnád, hogy milyen logika mentén gondolkodtál, amikor a programot megírtad, így a válaszolónak könnyebb lenne kitalálnia, hogy mit is szeretnél csinálni, és, hogy az miért nem működik.

    Most mennem kell, később még benézek. Ha addig nem lesz meg a válasz, megpróbálom kibogarászni a kódodból, de sokat segítene, ha megfogadnád a tancsaimat (nem csak ebben az esetben, hanem bármikor, amikor valamilyen programot írsz, idővel majd te is rájössz a hasznára)
    Mutasd a teljes hozzászólást!


  • Én se trollkodok, csak leírom a véleményem:

    1. A C# egy OO nyelv, és mint ilyen érdemes osztályokat használni a programozáshoz. A programod jelen állapotában akár C-ben is íródhatott volna.


    A C# azért támogat OOt, hogy ha szükséges ki tudd használni. Kevés 177 soros programban van értelme OOzni. Szerintem kifejezetten a könnyű refaktorálhatóság a C# legnagyobb erőssége, vagyis az én tapasztalatom szerint itt a legegyszerűbb újrastruktúrálni a kódod, ha a mérete miatt túlbonyolódna. Szerintem bőven elég akkor osztályokat bevezetni, ha túl sok függvényed van ahhoz, hogy rendesen átlehessen látni a kódot. Ha korábban csinálod, abból nagyon könnyen
    OverEngineering
    lesz. Olyan osztályokat vezetsz be, amik nem optimálisak. Ezzel a kóddal nem a függvények száma a gond.



    2. Érdemes dokumentálni, leírni, hogy melyik függvény, változó mire használatos.


    Ezzel megint nem tudok egyetérteni egy ekkora kódnál. Nekem mondjuk egyébként is az a tapasztalatom, hogy igazán szép C# kód, konvencióknak megfelelően használt változó/függvény/osztály nevekkel, megfelelő nyelvi eszközök (foreach, lambdák, propertiek stb.) kihasználásával 99%ban öndokumentáló. Legalábbis addig mindenképp amíg 1-2 ember ír/használ egy projektet, és szabadon lehet kódminőségre optimalizálni.


    3. A dokumentáció egy részét megspórolhatod, ha beszédes neveket használsz. Többek között pl. a fffuuu változódról és a check33 függvényedről sem lehet tudni, hogy mire jó.


    Ezzel mélységesen egyetértek. Felesleges (és C# kód konvenciókkal ellentétes) a változóneveket rövíditeni, az IntelliSense miatt csak az első begépelésnél spórolsz meg néhány karaktert. Hivatkozásnál mindig sokkal egyszerűbb a teljes névre hivatkozni. Nyugodtan használj
    Field
    ,
    Hits
    ,
    Ships
    stb. változóneveket. Akkor se félj kiírni, ha ennél jóval hosszabb a legtalálóbb név amit ki tudsz találni. Persze itt is felesleges túl sokat agyalni egy néven, bármikor átnevezheted ha kell, de
    fc,fp,optf,opts
    mind egyértelműen rossz választás, és már ennek a nagyon rövid kódnak az értelmezését is jelentősen megnehezítik.



    Normális elnevezéseken túl még funkcionális programozás használatával tudnál jelentősen javítani ennek a kódnak az átláthatóságán, meg fejlettebb adatstruktúrák használatával. Tömböt használni pl. szinte sosem optimális kódminőség szempontjából.
    De igazából kezdőként teljesen elég amit használsz. Ha megfelelően átnevezed a függvényeket és a változókat szerintem adni fogja magát a probléma.
    Mutasd a teljes hozzászólást!
  • Na átszerkesztettem a kódban a változóneveket.
    using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace Battlesecondhip_v2._0 { class Program { static int[,] fieldplayer; static int[,] fieldcomputer; static int[] optionsfirst; static int[] optionssecond; static int[] optionsindex; static int first = 0; static int second = 0; static int length = 0; static Random m = new Random(); static void Main(string[] args) { Start(); } static void Start() { fieldplayer = new int[12, 12]; fieldcomputer = new int[12, 12]; PositionComputer(); } static void PositionComputer() { for (int i = 0; i < 5; i++) { optionsfirst = new int[0]; optionssecond = new int[0]; length = Length(i); first = m.Next(1, 11); second = m.Next(1, 11); bool chk = Check3_3field(1); if (chk == true) { Optionsecond(); Checkfullfield(1); if (optionsindex.Length > 0) { int posecond = m.Next(0, optionsindex.Length); PlaceComp(optionsindex[posecond]); } else i--; } else i--; Console.Clear(); } for (int r = 0; r < fieldcomputer.GetLength(0); r++) { for (int j = 0; j < fieldcomputer.GetLength(1); j++) Console.Write(fieldcomputer[r, j] + " "); Console.WriteLine(); } Console.ReadKey(); } static void PlaceComp(int q) { int first1 = first; int second1 = second; int first2 = optionsfirst[q]; int second2 = optionssecond[q]; if (first2 < first1) { int other = first1; first1 = first2; first2 = other; } if (second2 < second1) { int other = second1; second1 = second2; second2 = other; } for (int i = first1; i < first2 + 1; i++) for (int j = second1; j < second2 + 1; j++) fieldcomputer[i, j] = 2; } static void Checkfullfield(int a) { int[,] fieldother; if (a == 0) fieldother = fieldplayer; else fieldother = fieldcomputer; optionsindex = new int[0]; for (int q = 0; q < optionsfirst.Length; q++) { bool k = true; for (int i = first - 1; i < optionsfirst[q] + 1; i++) for (int j = second - 1; j < optionssecond[q] + 1; j++) { if (fieldother[i, j] == 2) { k = false; } } if (k == true) { Array.Resize(ref optionsindex, optionsindex.Length + 1); optionsindex[optionsindex.Length - 1] = q; } } } static void Optionsecond() { if (first - length > -1) { Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = (first - length + 1); Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second; } if (first + length < 12) { Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first + length - 1; Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second; } if (second - length > (-1)) { Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second - length + 1; Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first; } if (second + length < 12) { Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second + length - 1; Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first; } } static bool Check3_3field(int a) { int[,] fieldother; if (a == 0) fieldother = fieldplayer; else fieldother = fieldcomputer; for (int i = first - 1; i < first + 2; i++) for (int j = second - 1; j < second + 2; j++) if (fieldother[i, j] == 2) return false; return true; } static int Length(int i) { switch (i) { case 0: i = 5; break; case 1: i = 4; break; case 2: i = 3; break; case 3: i = 3; break; case 4: i = 2; break; default: i = 0; break; } return i; } } }
    Talán így már egy kicsit átláthatóbb
    Mutasd a teljes hozzászólást!
  • Pár név hülyén maradt, mert quick replacéval cseréltem a neveket,és nem vettem észre mindent, amit az s cseréje okozott, de már nem tudok szerkeszteni
    Akkor leírom, hogy hogyan kéne működnie a programnak:
    két field nevű mátrix 12*12 a keret nem fog kirajzolódni, csak azért kell, hogy az indexek miatt ne dobjon Exceptiont a szélső cellákra. a firstek és secondok mindig a lövések indexét tartalmazza .a két options-t úgy tekintem, mintha x*2-es mátrix lenne. na akkor a lényeg: a ciklusban a length a hajó hosszát adja meg, utána randomizálok két számot 1-10-ig, majd megvizsgálom, hogy a tömb ezen koordinátával rendelkező pont és környéke szabad-e(Check3_3field). ha nem, akkor újra generálom. ha igen, akkor megkeresem az összes olyan lehetőséget, aminél nem lóg ki a hajó a pályáról(Options)(optionsfirst és optionssecond tömbbe a végpont koordináták) majd ezekre nézem meg, hogy melyik lehetőség nem ütközik másik hajóval(Checkfullfield), és amely indexnél nem ütközött, azt eltárolom az optionsindexben. ezután generálok egy indexet az optionsindexhez, majd a PlaceComputer-nek átadom az indexhzez tartozó indexet, és az optionsfirts és optionssecond megfelelő értékével elhelyezem a hajót. Elméletileg így működik,de mégis sokszor összecsúsznak a hajók, átfedésbe is
    Mutasd a teljes hozzászólást!
  • Senkinek sincs ötlete?
    Mutasd a teljes hozzászólást!
  • Egyszer ha a prog.hu-ra látogatna Steve Balmer tuti, hogy ő is ilyen kérdést tenne fel, hogy "operációs rendszert csináltunk, de néha fagy, hol lehet a hiba?" és ezek után becopipésztelné a windows forráskódját, buzgón várva a megoldásra....
    Mutasd a teljes hozzászólást!
  • visszaemlékszem, amikor még kedvenc spectrumomat toltam assemblyben.
    Ha valami nem ment, kockás papír elő és lépésről lépésre debuggolni. Külön kis négyzetbe írtam a regisztereket :)

    ehez képest ami ma rendelekzésre áll... :)
    Mutasd a teljes hozzászólást!
  • Szerintem senki nem fogja a konkrét kódodat debug-olni, többek közt én sem.

    Mi lenne, ha átgondolnád az alapelvet és újrakódolnád? Annak idején még Pascal-ban én is írtam torpedót, és nem mondhatnám, hogy különösebben bonyolult lett volna. Kb. annyit kell csinálnod, hogy mindent kódokkal jelölsz a táblán, pl. 0 - üres, 1 - hajó, 2 - a tábla kerete, vagy egyéb foglalt rész. Gyakorlatilag csak annyit kell tenned, hogy az első hajó felrakása után körberajzolod a foglalt kóddal (jelen példában 2-es), és a következő hajó kezdőpozíciójánál megvizsgálod, hogy elegendő üres (jelen példában 0-ás) hely áll-e rendelkezésre a hajó számára. Ha a vizsgálat során az derül ki, hogy nincs hely, akkor természetesen ne kezdd el lerakni a hajót!

    Ezután már csak a lépéseket ismétled: hely ellenőrzés, hajó felrakás, hajó körberajzolása foglalt kóddal, egészen addig, míg minden hajó fel nem lesz helyezve.

    Ezzel az egyszerű módszerrel elkerülhető, hogy egymásra csússzanak a hajók.
    Mutasd a teljes hozzászólást!
  • Töröld ki az OptionSecond metódusból a felfelé és a balra elhelyezett hajókat.
    static void Optionsecond() { if (first + length < 12) { Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first + length - 1; Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second; } if (second + length < 12) { Array.Resize(ref optionssecond, optionssecond.Length + 1); optionssecond[optionssecond.Length - 1] = second + length - 1; Array.Resize(ref optionsfirst, optionsfirst.Length + 1); optionsfirst[optionsfirst.Length - 1] = first; } }

    Gondold csak végig, hogy a Checkfullfield metódusban mekkora területet ellenőriznél egy balra vagy felfelé elhelyezett hajónál.
    Tehát az ellenőrző ciklusban azt is figyelni kéne, hogy a kezdőponttól merre van a hajó végpontja, és ez alapján kéne a ciklus intervallumát meghatározni.
    De mivel a jobbra vagy balra elhelyezett hajó ugyan úgy néz ki, ezért az egyiket nyugodtan elhagyhatjuk.

    Másrészt a ciklus intervallumának meghatározása a jobbra vagy lefele elhelyezett hajóknál is hibás.
    for (int i = first - 1; i < optionsfirst[q] + 2; i++) for (int j = second - 1; j < optionssecond[q] + 2; j++)

    Tehát így + 2 -nek kellene szerepelnie benne a + 1 helyett.

    Egy-két további észrevétel.
    1. A Length metódust le kéne cserélni egy konstans tömbre. Ez így elég hülyén néz ki.
    2. Tömböket használsz az adatok tárolására és ha beleakarsz tenni valamit, akkor folyton átméretezed. Használj helyette inkább listákat, azoknak is van length (vagyis count) metódusa amivel megtudhatod, hogy mennyi elem van benne, és sokkal egyszerűbb használni őket.
    3. Használj összetett típusokat. A first/second és a optionfirst/optionsecond ugye koordinátákat jelölnek, ezeket egyrészt jobb lenne X és Y -nek hívni, másrészt kezelhetnéd őket együtt mondjuk egy Point objektummal. És akkor nem kéne két tömböt karbantartani.
    4. Console.Clear utasítást a hajó elhelyező ciklusba tetted, miért?
    5. Eszméletlenül túlbonyolítottad a programot.
    Ez egy viszonylag egyszerű feladat, de sikerült annyira túlbonyolítani, hogy alig lehet átlátni.
    Egy valóban bonyolult feladatnál, hogy fog kinézni a programod?

    A programodból látszik (ha te csináltad), hogy van érzéked a programozáshoz, de az is látszik, hogy eléggé kezdő vagy még (ami persze nem baj).
    Mutasd a teljes hozzászólást!
  • Köszi, a ciklusnál, a végpontnál levő +1 helyett +2 és a PlaceComp metódusból a csere (hogy a ciklus kezdőpontja ne legyen nagyobb, mint a végpont)(és így benn maradt a balra és fel) beszúrása után tökéletesen működik :) .
    1-3 jogos észrevétel, eszembe se jutott :D
    4 valószínűleg véletlen maradt ott akkorról, mikor hajónként rajzoltattam ki
    5 ezt bóknak veszem :D . Csak ezeknek a metódusoknak a nagy részét fogom felhasználni a játékos hajóinak elhelyezésekor
    Természetesen én csináltam(nem iskolamunka, csak magamnak írom unalmas perceimre :D )
    Köszi a dicséretet, iskolai szinten tanulom egy éve, de sajnos nagyon lassan haladtunk, hogy mindenki megértse rendesen az anyagot (10.-et fogom kezdeni)
    Mutasd a teljes hozzászólást!
Címkék
Tetszett amit olvastál? Szeretnél a jövőben is értesülni a hasonló érdekességekről?
abcd