Suche Menschen die meinen C#-Code kritisieren!

[Kokujou]

Hallo~ Erstmal: Ich bin mir im Klaren dass meine Frage sehr zeitaufwändig ist, es würde mir ja schon helfen wenn mir jemand sagen könnte an wen ich mich denn wo wenden könnte, um eventuell Glück haben, vielleicht irgendne Discord-Gruppe oder so gerne auch in Englisch.

Ich hab ein Projekt am Laufen, ein 3D-Hanafuda Spiel für mehrere Spieler mit mobilem und PC-Modus und natürlich sollen später auch noch ein paar KIs rein. Der Punkt ist der. Ich hab mir das Programmier vollständig selbst beigebracht ohne jemals irgendeine Art von Buch zu lesen weil ich nunmal aus bestimmten Gründen einfach keine Bücher lesen kann ( Sorry >.< ). Ich bin aber nicht so arrogant zu denken dass mein Code so wie er jetzt ist perfekt ist, auch wenn es ein recht einfaches Projekt ist. Also suche ich mal jemanden der Zeit und Muße hat sich in meine Code-Files reinzulesen und an allem rumzumeckern was da zu finden ist, vorzugsweise mit Relevanz für die Performance

Ich hab schon ein paar Tipps zur Strukturierung bekommen und bereits versucht alles was mit GameObject.Find und Resources.Load zutun hat rauszuschmeißen, was einen merklichen Effekt auf die Laufzeit auf mobilen Geräten hatte, immerhin! Aber ich bin sicher da ist noch viel mehr zutun! Die Skripte sind im Resources-Ordner direkt unter dem Namen Scripts zu finden, der Rest ist nur für die Umgebung damit ihr ein Bild bekommt.

Der Link: https://drive.google.com/open?id=1sM8_ED-j0MjAz87MdKz9DhbZCcGXIjXW

 

[Yuuri]

1,1 GB? Ich hab zwar ne 200 MBit Leitung, aber "zum Drübergucken" lad ich mir das nicht runter.

 

[Kokujou]

... okay das ist jetzt peinlich... warum zur hölle ist das so groß sekunde ich update den link sofort! peinlich >.<

und ich frag mich hier warum das dauernd so lange zum Entpacken dauert... Und zum hochladen, kein Wunder bei 1GB. Jetzt sinds 100MB, dankeschön für den Tipp!

 

[Rage]

Ich spreche kein C#, kann aber eventuell trotzdem bei mathematischen Fragen (was Geschwindigkeitssteigerungen in aller Regel beinhalten) Hilfe anbieten, sofern das gewollt wäre.

 

[Kokujou]

Tja ich weiß nicht so ganz wie das laufen soll ... soll ich dir meinen Algorithmus textuell beschreiben oder... ?
aber wenn du irgendwie helfen kannst wäre ich happy ^^ ich will ja was lernen und in meinem Studium sind alle irgendwie Gegner des Lernens... da gehts nur um Bulimie-Lernen.

 

[Rage]

Wenn Dir ne Frage einfällt, meld Dich einfach, würd ich vorschlagen

 

[haze4real]

Hab einfach mal reingeschaut, musste ab und an etwas schmunzeln.

Klassiker:

                if (((Card)obj).Name == Name)
                    return true;
                else return false;

Was versuchst du hiermit zu erreichen?

            while (true)
            {
                ...
                yield return null
            }

Ist das eine Art Pattern in Unity? Scheinst es relativ oft zu verwenden, sieht komisch aus, habe allerdings keine Ahnung von Unity...

/edit: Ist wohl das Pattern für eine Coroutine in Unity.

Hab nicht wirklich Zeit, aber würde dir empfehlen etwas an deinem Coding Style zu arbeiten.

 

[andy_m4]

Jetzt sinds 100MB, dankeschön für den Tipp!

100 MB ?
Das finde ich immer noch viel fett, wenns ums "drüberschauen" geht.
Der aktuelle Apache Webserver 2.4 sind ca. 25MB Quelltext. Und schon den schaut man sich nicht mal eben an.

 

[haze4real]

Handelt sich scheinbar um ein Unity Projekt, ist also nicht reiner Quelltext, das Verzeichnis mit Quelltext ist nen paar Kilobyte groß...

 

[Kokujou]

Jap^^ Die Skripte sind sehr gering, sorry, aber es kann ja nicht schaden zu wissen mit was für Dateien man es hier zutun hat, nur mit dem Code kann man sich vielleicht ein schlechtes Bild von dem Gesamtprojekt machen^^ Aber ich sagte ja es ist ein 3D-Projekt ihr habt sicher nicht gedacht ich hätte das alles mit OpenGL geschrieben

Ja die Frage mit den Coroutinen hast du dir ja schon selbst beantwortet. Mit yield return null unterbrichst du sie und führst mit der Ausführung des Hauptprogramms weiter, so läuft sie also pseudoparallel^^

Hab einfach mal reingeschaut, musste ab und an etwas schmunzeln.

Klassiker:

                if (((Card)obj).Name == Name)
                    return true;
                else return false;

Hab nicht wirklich Zeit, aber würde dir empfehlen etwas an deinem Coding Style zu arbeiten.

Wenn ich raten müsste würde ich sagen das ist bestimmt die Equals Funktion? Ja damit wollte ich eigentlich nur erreichen dass der Name zurückgegeben wird, ist das schlechter Stil?

Und "an meinem Coding Style arbeiten" ist leicht gesagt ich hab keine Ahnung von irgendwelchen Standards, seid froh dass ich es erst jetzt hochlade, wo ich die Anzahl der Skripte schon verdoppelt habe, vorher war die Main.cs mindestens 3mal so lang^^

Ergänzung (30. November 2018)

An dieser Stelle willich übrigens nochmal ansprechen: Es wrüde mir auch sehr helfen wenn ihr mir eine Community oder direkt Peronen nennen könntet, bei denen man eher Glück hat, ich meine sicher hat niemand von euch Zeit erstmal tausende Codezeilen durchzuforsten und zu sehen ob das alles sinn macht! XD Wenn doch wäre das natürlich ideal

Ich habs schon auf nem Unity Forum versucht aber naja auch da haben die Leute die Zeit nicht unbedingt mit Löffeln gefressen... Meine Profs kannich natürlich auch nicht fragen und meine Mitschüler erst recht nicht XD von denen bin ich der beste Programmierer. und der einzige... Ich würd ungern erst im Beruf feststellen dass ich ein Kurpfuscher bin XD

 

[haze4real]

einzelne Funktionen/Algorithmen kann man bei codereview.stackexchange.com zum "Review" posten. Ansonsten stackoverflow für Fragen aller Art...

bzgl. Coding Style ist ein guter Anfang https://docs.microsoft.com/de-de/dotnet/csharp/programming-guide/inside-a-program/coding-conventions

Magst du keine Leerzeilen?

Schau dir auch mal dieses Artikel an: https://de.wikipedia.org/wiki/Smell_(Programmierung)

Gibt so ein paar Punkte die mir direkt bei dir aufgefallen sind, z.B. zu lange Methoden. Du solltest deinen Code in einer Weise schreiben so dass dieser für jeden Entwickler verständlich und gut lesbar ist, guter Code kommentiert sich selbst...

Wegen der Equals Methode ging es mir eher darum, dass du innerhalb einer If-Abfrage lediglich den Wahrheitswert der Abfrage zurückgibst, zur Verdeutlichung (Pseudocode):

bool value = ((Card)obj).Name == Name;
if(value) { // value == true
    return true
} else { // value == false
    return false
}

=

return ((Card)obj).Name == Name;

 

[FranzvonAssisi]

Wegen der Equals Methode ging es mir eher darum, dass du innerhalb einer If-Abfrage lediglich den Wahrheitswert der Abfrage zurückgibst, zur Verdeutlichung (Pseudocode):

Oh, da würden sich die Seniors jetzt aber drüber streiten.

Beides ist richtig, aber ich wäre mir nicht sicher ob und wenn ja, welche der beiden Varianten einen Performancevorteil bringt.
Würde ich ja ausprobieren, aber ich habe gerade keinen PC

In diesem Fall ist auch der Einzeiler noch gut lesbar, aber das kann sich schnell ändern, weswegen Reviewer, die ich hatte dort schon immer vorsichtig waren und gesagt haben: "Gut, aber Einzeiler nicht übertreiben"...

Lg

 

[Fonce]

@Kokujou
Das Gefühl das du dir nach dem Studium manchmal wie ein Kurpfuscher vorkommst wirst du mit hoher Wahrscheinlichkeit eh erleben. Aber das ist fast schon unabhängig von Studium. Wenn du nach zwei Jahren wieder in deiner Code guckst wirst du oft vieles finden was du deutlich eleganter hättest lösen können.
Einfach Mal nach Best Practices, SOLID und Design Partnern suchen. Irgend wann fängt man an das ganze zu verinnerlichen, aber es braucht halt Erfahrung um dann zu wissen was du wann wie am besten löst.

 

[Kokujou]

Fangen wir mal von hinten an :3

Ja das weiß ich, baer ich würde mich trotzdem gerne vorbereiten^^ Ich bin gerade voll in dem Panik-Modus dass ich keinen guten Job finde, besonders wegen diesem nichtswürdigem Studium in dem ich eigentlich mehr vergessen als gelernt habe... ich glaub vor meinem Studium war ich schlauer als jetzt >.> nicht gut!

Stimmt, oft mache ich das tatsächlich so wie vorgeschlagen mit den Einzelern, oft schreibe ich in Variablen-Definitionen sogar die ?: Operatoren^^ Die Equals war mir halt nicht so wichtig und ne Methode die nur aus nem Einzeler besteht sieht auch irgendwie doof aus Aber ich werds ändern.

Und was meinst du mit Leerzeilen? Also wenn der Code irgendwie komisch in der Datei aussieht lass dir gesagt sein dass Visual Studio die Einschübe ordentlich macht mit Leerzeilen nach dem Komma etc... in die rohe CS-Datei hab ich noch nicht geguckt

Hach die Codekonventionen... ich drücke mich schon lange darum und benenne meine Variablen immer frei Schnauze, allerdings möglichst schon relevant. Gerade versuche ich mir i/j in for-Schleifen abzugewöhnen ^^ Der Artikel über die Smells kommt mir interessant vor da les ich mich auf jeden Fall mal rein... Dass ich Codedopplungen habe hab ich größtenteils z.B. schon rausgekriegt aber an einigen Stellen ist die ähnlichkeit so indirekt dass ich es weiß Gott nicht wegkriege ^^ in der Update-Routine in der Main.cs z.B., ihr seht ja wie lang sie ist aber oft passieren sehr ähnliche Akte.

Gut "Code-Ausschnitte" wenn die n Limit von 1.000 Zeilen haben wirds knapp oder ich lasse jede Datei einzeln reviewn ansonsten versuchichs mit dem anderen Link^^ Danke!

 

[andy_m4]

Beides ist richtig, aber ich wäre mir nicht sicher ob und wenn ja, welche der beiden Varianten einen Performancevorteil bringt.

Ich glaube kaum, dass sich da ein großer Unterschied ergibt.

Ansonsten lässt sich generell sagen, dass die Lesbarkeit des Codes an erster Stelle steht. Da im Vorhinein irgendwelche "Hacks" zu machen, die dann evtl. Performancevorteile bringen macht wenig Sinn. Da kann man dann mal raufgucken, wenn Performance wirklich ein Problem ist.

Zumal wirklich große Performancegewinne vor allem dadurch entstehen, dass man die richtigen Algorithmen/Datenstrukturen wählt.

 

[Kokujou]

Und ob ich die gewählt habe ist ja gerade die Frage~ Ich bin ja verglichen mit den Meisten noch ein Anfänger, darum wollte ich eifnach mal sehen welche Teile euch z.B. besser eingefallen wären aber natürlich verstehe ich dass kaum jemand Zeit hat sich in ein Fremdes Projekt einzulesen. Wobei ich nebenbei eigentlich nicht finde dass mein code SO schlecht lesbar ist... außer von der Update-Routine in Main.cs XD

Und Performance war ein Problem. Bevor ich z.B. die GameObject.Find und Resources.Load Methoden rausgehauen habe und durch was besseres ersetzt habe, hat es ziemlich intensiv gelaggt auf mobilen Geräten.

Ach übrigens: theoretisch ergibt sich ein Unterschied Denn erst wird der Wahrheitswert abgerufen, das passiert immer. Aber dann wird Ja erstmal geprüft ob eine zweite Variable damit übereinstimmt und erst dann ein dritter Wahrheitswert zurückgegeben. mit dem Einzeiler wird nur einmal der Wahrheitswert abgerufen und ohne ne weitere Variable zu bemühen zurückgegeben ^^ das wäre jetzt aber nur wichtig wenn das in ner millionenfachen for-Schleife in der Update-Routine getan werden würde ^_^

 

[Sgt_H4rtman]

Ach übrigens: theoretisch ergibt sich ein Unterschied Denn erst wird der Wahrheitswert abgerufen, das passiert immer. Aber dann wird Ja erstmal geprüft ob eine zweite Variable damit übereinstimmt und erst dann ein dritter Wahrheitswert zurückgegeben. mit dem Einzeiler wird nur einmal der Wahrheitswert abgerufen und ohne ne weitere Variable zu bemühen zurückgegeben ^^ das wäre jetzt aber nur wichtig wenn das in ner millionenfachen for-Schleif

Ich bin zwar kein C# Entwickler, mir aber sehr sicher, dass der Compiler das intern ohnehin zum Einzeiler umwandelt, sprich das Compilat identisch ist. Von daher geht Lesbarkeit im professionellen Umfeld immer vor.

Sowas hier:

if ((bool) expression) {
    return <some value>;
} else {
    return <other value>;
}

kann man lesbarer lieber so schreiben:

if ((bool) expression) {
    return <some value>;
}

return <other value>;

Schau dir mal https://clean-code-developer.de an. Da findest du viele gute Hinweise zu sauberer Strukturierung und Benennung. Gibt auch ein Buch von Robert C. Martin (Clean Code) dazu. Das lohnt sich in jedem Fall auch.

 

[Crast]

Erstmal vor weg finde ich es schön, dass du selbst erkennst das zwischen Studium und Praxis ein "geringfügiger" Unterschied besteht. Genug Studenten erlebt die meinten nach dem Studium ein Top-Entwickler zu sein, simple Probleme konnten sie dann trotzdem nicht ordentlich lösen.

Ach übrigens: theoretisch ergibt sich ein Unterschied Denn erst wird der Wahrheitswert abgerufen, das passiert immer. Aber dann wird Ja erstmal geprüft ob eine zweite Variable damit übereinstimmt und erst dann ein dritter Wahrheitswert zurückgegeben. mit dem Einzeiler wird nur einmal der Wahrheitswert abgerufen und ohne ne weitere Variable zu bemühen zurückgegeben ^^ das wäre jetzt aber nur wichtig wenn das in ner millionenfachen for-Schleife in der Update-Routine getan werden würde ^_^

Du darfst nicht danach gehen was du schreibst, sondern was dein Compiler daraus macht. Es gibt einige Sachen die werden von modernen Compilern bereits sehr schön optimiert. Du solltest in erster Linie darauf achten, dass dein Code sauber und verständlich ist.

Zum Code direkt kann ich nicht besonders viel sagen, liegt daran dass ich C# praktisch nicht nutze und mien Unity-Wissen sich auf wenig allgemeine Sachen beschränkt. (Werde mir morgen mal dein Code laden und anschauen)

Was du dir definitiv anschauen solltest ist eine Versionierung. Ich würde dir Git mit LFS (da 3D-Projekt) empfehlen. Wenn du Hilfe brauchst und du dein Programm an mehrere Leute verteilen willst, kannst du damit auch Platformen wie Github nutzen (Github unterstützt LFS, allerdings bietet es nur öffentliche Repos in der kostenfreien Version).

Für C# gibt es, wie für fast alle Sprachen, Analysetools, die deinen Code überprüfen und u. A. Schönheitsfehler finden können. Für C# ist meiner Wissens nach FxCop der Industriestandard, da es auch in VS verwendet wird. Bei MS heist es allerdings Code Analysis.
Abhängig davon welche IDE du für deinen Code verwendest, kannst du dort auch Style Guides laden und mit der Code-Format-Funktion (Bei manchen heist es Autoformatter, bei anderen Code-Reformatter usw., in der Funktionsweise passen sie aber alle deinen Code optisch an).
Abhängig davon wie weit du dich in die Technologien einarbeiten möchtest, die die Code-Qualität sicherstellen sollen, kannst du dich auch mit CI beschäftigen.

 

[Kokujou]

Ich weiß nicht ob ich es bereits erwähnt habe, aber ich kann aus bestimmten gründen keine Bücher lesen.
Aber die Seite gucke ich mir mal an, ich werd wohl improfessionellen Rahmen nicht drumrum kommen, auch wenn ich nicht so ganz verstehe warum ich auch noch auf Groß- und Kleinschreibung achten muss. Ich versteh ja son bischen warum man sich den Inhalt der Variablen aussuchen soll und nicht mit pasorkpo kommen soll XD Aber Groß- Kleinschreibung? egal^^ ihr seid die experten.

Zur Versionierung: Ich hab heute mal einen Blick auf GIT riskiert... veruscht es zum Laufen zu bringen... erfolglos... das Unity-plulgin für GIT hat erstmal gar nicht funktioniert, erst fehlermeldungen, dann gar nichts mehr, und wie ich das "normale" GIT mit Unity synchronisieren kann ohne einen ganzen ordner manuell synchronisieren zu müssen ist mir auch schleiferhaft. Aktuell ist meine Erfahrung mit diesem ach so angepriesenen Git also eher dahingehend dass ich dieses Programm zum Teufel wünsche XD 2mal!

CI? Wie funktioniert das alles, heftet man die ans laufende Programmm oder lesen die tatsächlich nur den rohen Code und sagen dann "das ist schlecht", oder meinst du vielleicht diese Tipps links bei visual Studio wo immer sowas steht wie verbesserungsvorschläge, das nutze ich immer gerne wenn ich mal zu faul bin das "using ..." hinzuschreiben^^

Aber falls ihr nochmal einen Blick riskieren würdet, mir wären ja Tipps zur Performance-Steigerung oder Code-Kürzung lieber. Denn kürzer ist immer besser^^ Und einige Änderungen die ich bereits vorgenommen habe, wie z.B. das rausschmeißen der GameObject.Find Anweisungen sind mir glaub ich auch nur halbgut gelungen, wodurch uach diese unschöne CardRef klasse entstanden ist.

Für alle die keine Unity-Kenntnisse haben: Es gibt GameObjecte, das ist die Basisklasse, diese können Komponenten haben, das können dann Renderer sein die also graphische Ausgaben erzeugen, oder GUI-Elemente oder Physik-Tools, etc... Jede Karte hat bei mir also einen Parent mit Collider dr die Kollision regelt, sowie zwei Children für Vorder- und Hintergrund, außerdem hab ich in der Szene (Unity ermöglicht das Erstellen von Szenen, die dann nach belieben geladen werden können und in denen man sich quasi seine Landschaft zusammmenbauen kann) wo das Spiel stattfindet erstmal GameObjecte nur als Startpunkt fürs Layout gewählt, von denen ich also nur die geometrischen Eigenschaften Transformation, Rotation und Skalierung nutze..

Meine größte Herausforderung war das erwähnte rausschmeißen von den ganzen Find und Load, Anweisungen mit denen GameObject in der Szene gefunden werden können wohl aber indem man alle existierenden Objekte durchläuft, also ineffizient. Und Resources.Load holt aus der Sammlung meiner Resourcen, die ja sehr groß ist, die geforderte Resource. Abgedockt hab ich das in die PrefabCollection und VariableCollection, in der Prefabcollection sind alle sogenannten Prefabs, also Objekte samt children wie z.B. der Rohbau einer Karte, GUI-lemente und dergleichen, am schlimmsten war es abermit der Vairablen Kollektion, hier konnte ich nämlich nicht einfach in die Szene gehen und die Variablen zuweisenweil die Klasse in der sie benötigt werden noch gar nicht in der Szene existiert und dementsprechend vorher auch keine variablen zugewiesen werden können. Folglich musste ich eine weitere klasse global zugänglich machen und das geht leider nur indem ich statische Methoden verwende, aber auch das geht nicht denn statische Variablen können im Unity-Inspektor nicht zugewiesen werden, also musste ichzusätzlich einen Singleton erstellen, den ich dann endlich zuweisen kann und dann die statische variable mit dem Singleton gleichsetze, damit ich sie von anderen Skripten ansprechen kann. Ihr hört schon das ist nicht sehr elegant, vielleicht fällt euch ja was dazu ein.


Weiterhin habe ich noch einen Weg genutzt die Finds rauszuschmeißen undzwar habe ich in der Card-Klasse das entsprechende GameObject hinzugefügt, sodass ich es jederzeit ansprechen kann. nun musste ich es allerdings auch umgekehrt machen, also quasi von dem GameObject auch das assoziierte Card-Objekt hinzufügen. und das kging leider nur indem ich die CardRef-Klasse erstellt hab, die ich dann als Komponente dem GameObject jeweils hinzufügen konnte, aber auch das ist nicht sehr elegant wie ich finde...

 

[Crast]

Zur Versionierung: Ich hab heute mal einen Blick auf GIT riskiert... veruscht es zum Laufen zu bringen... erfolglos... das Unity-plulgin für GIT hat erstmal gar nicht funktioniert, erst fehlermeldungen, dann gar nichts mehr, und wie ich das "normale" GIT mit Unity synchronisieren kann ohne einen ganzen ordner manuell synchronisieren zu müssen ist mir auch schleiferhaft. Aktuell ist meine Erfahrung mit diesem ach so angepriesenen Git also eher dahingehend dass ich dieses Programm zum Teufel wünsche XD 2mal!

Falls interesse kann ich dir per TeamSpeak oder so helfen.
Ein VCS finde ich sehr wichtig. Es hilft dir deinen Code sauber zu halten, experimente durch zu führen und auch immer sehr schnell Fehler zu finden indem Stände miteinander verglichen werden.

CI? Wie funktioniert das alles, heftet man die ans laufende Programmm oder lesen die tatsächlich nur den rohen Code und sagen dann "das ist schlecht", oder meinst du vielleicht diese Tipps links bei visual Studio wo immer sowas steht wie verbesserungsvorschläge, das nutze ich immer gerne wenn ich mal zu faul bin das "using ..." hinzuschreiben^^

Continous Integration
Das umfasst Automatisierungen, z. B. automatisches bauen, Code Analysen, Tests uvm. Arbeitet aber immer mit einem VCS zusammen. Gibt oft Leute in größeren Betrieben die sich nur damit beschäftigen.