Wie ist mein Code?

Huhu,

letztens war hier ein post "Akzeptabler Programmierstil" und ich habe daraus einiges gelernt. Die idee den Code hier zu posten und von Profis vllt mal anschauen zu lassen fande ich sehr gut, und ich denke es ist gut wenn man c# sofort richtig lernt.

Ich habe ein Programm geschrieben, ein Gedächtnis-Trainer. Man lässt sich soviele Zahlen ausgeben wie man will, und muss sie danach der Reihe nach wieder aufzählen, man hat für jede Zahl 3 versuche, aber ihr werdet es ja selbst sehen.

Ich hoffe mal man versteht den Code, hab ihn so gut es geht kommentiert. Verbesserungen gerne, dazu ist der Thread ja da =).

mfg malte

Code:

using System;

namespace ZahlenMerken
{
    #region Main
    class Program
    {
        static void Main(string[] args)
        {
            #region Variablen
            int iCurrentNumber = 0;
            bool again = true;
            int isInt = 0;
            string readLine = "";
            #endregion

            Numbers NumberGame = new Numbers();

            #region Ausgabe

            //Einführung (Spielregeln)
            Console.WriteLine("\nDies ist ein Zahlen-Merk-Spiel, merke dir so viele Zahlen von 1-100 wie möglich.\nWenn du genug hast, drücke einfach ESC.");
            Console.ReadLine();
            Console.Clear();

            //Ausgabe (Zahlenausgabe)
            while (again)
            {
                iCurrentNumber = NumberGame.NumbersOut();
                Console.Write("\n{0}) {1}", NumberGame.Which, iCurrentNumber);
                if(Console.ReadKey().Key == ConsoleKey.Escape)
                {
                    again = false;
                }
                Console.Clear();
            }

            NumberGame.Amount = NumberGame.Which;
            NumberGame.Which = 1;

            #endregion

            #region Eingabe

            //Einführung (Zahlen aufzählen)
            Console.WriteLine("\nNun gebe die Zahlen in derselben Reihenfolge wieder ein.\nDu hast für jede Zahl 3 Versuche.");
            Console.ReadLine();
            Console.Clear();

            //Eingabe (Zahlen eingeben und überprüfen)
            do
            {
                Console.Write("\n{0}) ", NumberGame.Which);
                Int32.TryParse(Console.ReadLine(), out isInt);
                if (isInt != 0)
                    NumberGame.NumbersIn(isInt);
                else
                    continue;
            }
            while (NumberGame.Tries > 0 & NumberGame.Amount >= NumberGame.Which);

            Console.WriteLine("\nWow, du hast dich an {0}/{1} Zahlen erinnert, ich denke das war ein guter Anfang.", NumberGame.Which-1, NumberGame.Amount);
            Console.ReadLine();
            #endregion
        }
    }
    #endregion

    #region Numbers (Methoden)
    class Numbers
    {
        #region Klassenvariablen
        private int iTries = 3;
        private int iWhich = 0;
        private int iAmount = 0;
        private int[] arrNumbers = new int[100];
        #endregion

        Random rndNumbers = new Random();

        //Methode: Zahlenausgabe
        public int NumbersOut()
        {
            iWhich++;
            this.arrNumbers[iWhich] = rndNumbers.Next(1, 100);
            return this.arrNumbers[iWhich];
        }

        //Methode Zahleneingabe
        public void NumbersIn(int iCurrentNumber)
        {
            if (iCurrentNumber != this.arrNumbers[this.iWhich])
            {
                this.iTries--;
            }
            else
            {
                this.iTries = 3;
                this.iWhich++;
            }
        }

        //Eigenschaftsmethod "iTries"
        public int Tries
        {
            get { return this.iTries; }
        }

        //Eigenschaftsmethod "iWhich"
        public int Which
        {
            get { return this.iWhich; }
            set
            {
                if (value >= 0)
                {
                    this.iWhich = value;
                }
            }
        }

        //Eigenschaftsmethod "iAmount"
        public int Amount
        {
            get { return this.iAmount; }
            set
            {
                if (value >= 0)
                {
                    this.iAmount = value;
                }
            }
        }
    }
    #endregion
}

EDIT 1: Numbers-Objekt durch NumberGame ersetzt.

EDIT 2: public Variablen private gemacht und Eigenschaftsmethoden gemacht.

EDIT 3: Überprüfung ob Eingabe auch wirklich eine Zahl ist (TryParse)

hi,
die Zeile:

Numbers Numbers = new Numbers();

gefällt mir 🙂 🙂

mit richtigem SyntaxColoring schauts aber wahrschenlich besser aus!

Namen von Objektvariablen sollten niemals identisch sein mit den verwendeten Typen (Numbers). Siehe "Camel Casing".

ok, hab das mal geändert, danke 🙂

Numbers.NumbersIn(Convert.ToInt32(Console.ReadLine()));

wenn man einen buchstaben eintippt dann BUMMMM!!

Public Variablen finde ich schlecht.

Variable private machen und dafür ein Property über welches Du Zugriff auf die Variable erhälst.

Original von Programmierhans
Public Variablen finde ich schlecht.

Variable private machen und dafür ein Property über welches Du Zugriff auf die Variable erhälst.

okay, hab sie private gemacht und eigenschaftsmethoden zum zugriff eingefügt

Original von ApfeL
... und eigenschaftsmethoden zum zugriff eingefügt

Ich hoffe Du meinst Properties .... denn Zugriffsmethoden sind auch nicht wirklich state of the art.

Edit: habe gesehen dass Du es richtig gemacht hast....

Original von Programmierhans
Edit: habe gesehen dass Du es richtig gemacht hast....

gut =)

Das ganze nennt sich aber Properties statt Methoden (daher die Verwirrung)

Original von JAck30lena

Numbers.NumbersIn(Convert.ToInt32(Console.ReadLine()));  

wenn man einen buchstaben eintippt dann BUMMMM!!

wie überprüfe ich denn ob in einer eingabe NUR zahlen sind??

Guckst Du int.Parse / int.TryParse

okay, hab jetzt auch das mit tryparse eingebaut, funzt wunderbar =)

hmm meine meinung zu punkto programmierstiel.
Der Code im Main sollte übersichtlicher gestalltet sein.
klar du verwendest Region aber region ist nicht wirklich zum unterteilen einer Methode gedacht. Main ist ja im Prinzip auch nur eine Methode.
Wie Lagere Teile aus in andere Methoden.
zB.: die region Ausgabe/ Einführung könntest ruhig in eine andere Methode hauen.
da du eine Methode von Main aufrufst, vergiss nicht die neue methode ebenfalls als static zu deklarieren.

Ansonst gefällt mir der Code recht gut. Bis auf den punkt, dass ich für ne neue Klasse ein eigenes File verwenden würde.
Hat Semantisch mehr Sinn. Numbers ist sematisch keine unterklasse von Programm. Daher meine Empfehlung neue Klasse neues File. Class In-Class definitionen würde ich außer bei paar ausnahmen unterlassen.

Meistens gibt es ein paar Kleinigkeiten an der man eine gute programmier ausbildung erkennt aber ob du jetzt etwas so oder so schreibts ist meist Geschmakssache.

Ach übrigens bevor ich eines vergesse. Man sieht es zwar oft, schaut professionell aus ist es aber nicht. Ich mein ich tendiere hin und wieder auch oft zu dieser Schreibweise aber sie ist nicht empfehlenswert wenn es um die lesbarkeit des Codes geht. Ich meine die endlosen MEthoden in Methoden aufrufe.

Bsp: Console.WriteLine(Convert.ToDouble32(blabal.balabla))

Ich mein das mit Convert is noch ok. aber über 2 -3 solche konstruktionen sollte es nicht hinausgehen. Das problem besteht dahin, wenn du eine Software schreibst die jemand andere wartet bzw. wenn du sie nach Jahren den Code erst wieder zu gesicht bekommst, ist es äußerst mühsam sich da wieder hineindenken zu müssen.
im "geschäftlichen" Einsatz solltest du daher soetwas auch eher meiden.

mfg TiTime 😉

Hi,
Ich würde anstatt in Numbers ein int[] mit den Werten lieber zwei getrennte Klassen machen:

1. Eine generische Liste mit den Werten

class NumberList : List<Number>

2. Eine Klasse für den Wert. Im Konstruktor wird gleich eine Zufallszahl erzeugt
   Das sieht dann etwa so aus:

NumberList nList = new NumberList();
...
nList.Add(new Number());
...
int input = <<KonsolenEingabe>>;
bool found = nList.Find(input);

Die Methode Find geht durch die eigene Liste (this) durch und sucht sich den Wert:

public bool Find(int test)
{
    foreach (Number number in this)
    {
        if (number.Value == test)
           return true;
    }
    return false;
}

Ich find, das Spiel sollte so tolerant sein und nicht auf die Reihenfolge beharren 🙂
Aus der List Klasse kannst Du dann problemlos das sog. Model machen. D.h. Du packst das ganze Spiel da rein aber ohne ReadLine/WriteLine etc. Nur Zahlen rein, Zahlen raus und Methoden dafür.
Als nächstes kannst Du dann Dein Spiel auf Windows Forms portieren. Das geht dann ganz leicht, weil das ganze Spiel in der Model Klasse (und Number Klasse) ist.
Ich hoffe, Du bist jetzt nicht allzu sehr verwirrt.
Pat

Hallo allerseits,

jetzt hätte ich auch mal ein paar Fragen dazu...

Original von svenson
Namen von Objektvariablen sollten niemals identisch sein mit den verwendeten Typen (Numbers). Siehe "Camel Casing".

Warum nicht, bzw. was hat der CamelCase (es gibt übrigens lowerCamelCase und UpperCamelCase) mit den Objekt- bzw. Variablennamen zu tun?

und

Public Variablen finde ich schlecht.

Variable private machen und dafür ein Property über welches Du Zugriff auf die Variable erhälst.

Wieso das denn? Braucht man wirklich ein Property um eine Variable öffentlich zur Verfügung zu stellen? Wo ist denn da der Vorteil? Man hat doch dadurch nur mehr Programmieraufwand, unübersichtlicheren Code und performanter wird das ganze dadurch auch nicht...

Besten Dank schonmal! 🙂

Viele Grüße,
Christian

zu deiner camelcase frage:
Empfehlungen zur Schreibweise (C#)
(hab ihn nicht durchgelesen, vielleicht werden dir da einige antworten gegeben)

gefunden über forensuche nach: camelcase

zu deiner property frage:
Get / Set
(und vermutlich die darin angegebenen links. ich hab sie nicht weiterverfolgt)

gefunden über forensuche nach: property

nils

Hi,

danke für die Links. Also zusammenfassend kann man sagen, daß Member in camelCase und Properties in PascalCase geschrieben werden sollten. Soweit ich mich erinnere, macht das Microsoft auch so (und ich ganz intuitiv bisher auch).

Was aber die Properties betrifft, so halte ich folgendes für völlig überflüssig:

private int irgendeinWert = 3;

public int IrgendeinWert
{
  get { return irgendeinWert; }
  set { irgendeinWert = value; }
}

Wenn der Wert nur gesetzt und ausgelesen wird, kann ich einfach keinen Grund finden, das Property explizit zu deklarieren. Ich fange meist so an:

public int IrgendeinWert = 3;

Erst wenn sich die Notwendigkeit ergibt, bei der Zuweisung irgendetwas zu verarbeiten, erweitere ich das ganze folgendermaßen, ohne irgendeine weitere Zeile Code verändern zu müssen:

private int irgendeinWert = 3;

public int IrgendeinWert
{
  get { return irgendeinWert; }
  set { irgendeinWert = Math.Abs(value); } // Zum Bleistift.
}

Irgendwelche Einsprüche? 😉

Schönen Abend allerseits,
Christian