Structs unter C++, nicht reproduzierbarer Fehler

[Markus B. (rusticus)]

Hallo,

ich bin zurzeit daran ein C++ Buch durchzuarbeiten um einfach meine 
Kenntnisse mal wieder weiter zu bringen (die jetzt nicht der Burner 
sind), allerdings bekomme ich einen Fehler, den ich nicht reproduzieren 
kann.

Ich kann in dem Programm immer mindestens 1 Feld belegen, das geht 
immer, dann aber i-wann zwischen dem 2ten und dem 14ten fängt das 
Programm an, mir in der Ausgabe immer und durchgehend beliebige Auszüge 
aus dem Programm auszugeben.

Ich bin ziemlich ratlas und habe stellenweise auch den Code schon 
vereinfacht weil ich dacte da könnte der Fehler sein, aber er kommt 
einfach immer wieder.

Ich hoffe nun auf ein paar Tipps und eine Erklärung was ich falsch 
gemacht habe.

Danke schon mal im Vorraus!

[Rufus Τ. F. (rufus)]

Markus B. schrieb:
> i-wann

Gewöhn Dir das ab.

Markus B. schrieb:
> fängt das Programm an, mir in der Ausgabe immer und durchgehend
> beliebige Auszüge aus dem Programm auszugeben.

Was magst Du damit meinen?


Hast Du Dein Programm mal im Debugger laufen lassen und ihm auf die 
Finger gesehen?

[Markus B. (rusticus)]

Rufus Τ. Firefly schrieb:
> Markus B. schrieb:
>> i-wann
>
> Gewöhn Dir das ab.

Du meinst ich soll es in Zukunft lieber ausschreiben oder meinst du ich 
sollte die Aussage spezifizieren?

Und auf die Idee mit dem Debugger bin ich noch gar nicht gekommen, 
probiere ich morgen gleich mal aus

Danke trotzdem schon mal!

[D. I. (Gast)]

Das Problem habe ich nicht nachvollzogen, aber

sSpielfeld[PlayFieldX][PlayFieldY];

sollte IMO

sSpielfeld[PlayFieldY][PlayFieldX];

heißen, wenn du so

for (int y = 0; y < PlayFieldY; y++)
{
   for (int x = 0; x < PlayFieldX; x++)
   {

iterierst (Geschwindigkeisgründe aufgrund der Speicherorganisation eines 
2D-Arrays). Außerdem ist es üblich in 2D-Arrays erst die Zeile 
(y-Koordinate) und dann die Spalte (x-Koordinate) zu indizieren. Ist in 
IMO einfach intuitiver.

[Rolf Magnus (Gast)]

Du machst bei der Eingabe keinerlei Fehlerprüfung. Sobald da mal was 
falsches eingegeben wurde, geht dein cin in den Fehlerzustand, und dann 
landest du mit deinen Abfrageschleifen in einer Endlosschleife.
Was z.B. auch zum Fehler führt, ist die Antwort "nein" auf die Frage 
"Soll dort ein Gegner sein?". Da das zu lang ist, bleibt ein Teil der 
Buchstaben in der Eingabe stecken. Beim nächsten Abfragen einer Zahl 
geht der Stream dann auch auf Fehler.

[Markus B. (rusticus)]

@D.I.
Naja das XY ist für mich einfach von der Reihenfolge her so richtig, 
allerdings ist es, zumindest nach meinen Stand der Dinge, nicht möglich 
so ein Feld über

for (int x = 0; x < PlayFieldX; x++)
{
   for (int y = 0; y < PlayFieldY; y++)
   {

darzustellen


@Rolf Magnus:

Danke schon mal für die Antwort, ich schau es mir morgen gleich nochmal 
an
Allerdings mal so vorweg, ich frage doch das "Nein" über das "else" ab, 
oder sollte eher so was wie ein "cin.ignore()" dahin?

[Karl H. (kbuchegg)]

Grundregel.
Zumindest in der Entwicklungsphase lässt man sich alles, was das 
Programm einliest auch gleich wieder ausgeben. Einfach nur um sicher zu 
gehen, dass das Programm auch tatsächlich das erwartete eingelesen hat. 
Vor allen Dinge Sachen wie der zur Eingabe erforderliche Return machen 
einem nämlich gerne einen Strich durch die Rechnung, weil sie nicht 
richtig behandelt werden.

[Karl H. (kbuchegg)]

Und wenn du solche Sachen hast wie:

Du lässt den Benutzer Zahlen von 1 bis 5 eingeben, brauchst 
programmintern aber 0 bis 4 (wegen Array Index), dann korrigierst du die 
Zahl sofort nach der Eingabe

    x--;

anstatt dass du überall
   irgendwas[x-1]
schreibst.

Denn irgendwo vergisst du nämlich mit Sicherheit auf das -1. Und wenn 
nicht heute dann morgen oder nächste Woche, wenn du am Programm 
weiterarbeitest.

Rechne damit, dass du Fehler machst und gestalte dein Programm so, dass 
du nach Möglichkeit keine Leichtsinnsfehler machen kannst. Je weniger 
Sonderfälle (wie zb Arrayindizes die immer um 1 vermindert werden 
müssen) du dir merken musst, desto geringer ist die Chance, dass du 
diesen Sonderfall irgendwo vergisst anzuwenden.

[Klaus W. (mfgkw)]

Neben den schon gesagten habe ich auch noch ein paar Sachen...

(Das soll dich jetzt nicht frustrieren; prinzipiell ist das hier
eines der wenigen Einsteigerprogramme, die lesbar und
übersichtlich sind.)


Das:

  using namespace std;

wird sehr häufig gemacht, davon würde ich aber stark abraten.
namespaces sind nicht dafür erfunden worden, daß man sie
gleich wieder aushebelt.

struct sFieldData
{
...
};

Wenn schon C++, warum dann nicht gleich richtig?
Man könnte eine Klasse machen, die Innereien gleich mit dem
Konstruktor sauber initialisieren und den Inhalt über Methoden
ändern und abfragen.

Muß nicht gleich am Anfang sein, aber vielleicht im nächsten Schritt...

  int status;
  /*0 = Loch   ' '
    1 = Gras   '.'
    2 = Stein  'o'
    3 = Berg   'O'
    4 = Wald   'X'
    5 = Wasser '~'*/

Dafür wurde enum erfunden.

  char cName[30];

Was, wenn es mal mehr als 29 Zeichen sind?
Mit std::string geht es besser.

const int PlayFieldX = 5;
const int PlayFieldY = 5;

Die würde ich z.B. MaxPlayFieldX o.s.ä. nennen.

  char Enemy[5];

Was, wenn der Benutzer mehr als 4 Zeichen eingibt?
Auch hier lieber wieder std::string.

Das Problem mit der Pufferung der Eingabe wurde ja auch schon genannt.

  if (Enemy[0] == 'J' || Enemy[0] == 'j' )

oder:

  if ( std::toupper(Enemy[0]) == 'J' )

  bool enemy;
  char cName[30];

Die bool könnte eigentlich entfallen, wenn man einen leeren Namen
als Zeichen für enemy==false annimmt - eine ordentliche
Initialisierung des Strings vorausgesetzt.

        switch (sSpielfeld[x][y].status)
        {
          //0 = Loch   ' '
          case 0:
            {
              cout << " ";
            }break;
            //1 = Gras   '.'
          case 1:
            {
              cout << ".";
            }break;
            //2 = Stein  'o'
...

Mit enum und schönen sprechenden Namen wäre das auch ohne Kommentare 
selbsterklärend...

void deleteField()
{
...
      sSpielfeld[x][y].status = false;
...
}

Das ist unschön, weil status eine int ist (und besser eine enum
wäre :-), false aber eine bool.
Daß false "zufällig" deinem leeren Feld entspricht, mag sein - ist
aber nicht offensichtlich.

0 oder halt besser der passende enum-Wert wäre sinnvoller.

Angehängte Dateien:

    * Aufgabe_7.zip (205.6 KB, 19 Downloads)

Der wirklich genutzte Quelltext hätte auch gereicht; mit der exe
und den ungenutzten anderen Dateien kann niemand etwas anfangen.


Ansonsten sehe ich auch die evtl. zu knappen Felder und
ungeprüfte Eingaben als einzigen echten Fehler.

mfgkw

[Markus B. (rusticus)]

Vielen vielen Dank für die ganzen Tipps und Verbesserungsvorschläge!!

Ich werde es gleich nochmal durcharbeiten!

An das "enum" habe ich gar nicht gedacht, ist aber sicherlich die 
bessere Lösung, ich werde es mal versuchen umzuarbeiten, genauso muss 
ich eingestehen, das ich schlichtweg nicht darauf gekommen bin, "--x" zu 
machen anstatt das umständlliche.

Danke nochmal, hat mir schon sehr viel geholfen!!