Suche Menschen die meinen C#-Code kritisieren!

[Kokujou]

ich hab extra die unity ignore eingefügt... und die visual studio ignore...
alles was jetzt noch drin ist soll offenbar drin sein.
Ermstla. Die Skripts sind im Ordner Assets/Resources/Scripts
Ja ich weiß ich sollte sie umplatzieren, das mach ich auch vielleicht noch... mal sehn.
Ist halt Unity... aber sehen wir uns das mal an:
Ich schätze mal die ganzen Asset Files im ProjectSettings Ordner sind notwenig... glaube ich
Okay den Build den hatte ich wohl übersehen... ursprünglich hatte ich dafür nen extra ordner aber davor hab ich ihn mal blind da reingehauen das werde ich beim nächsten Mal bereinigen, danke
Aber alles im Asset-Ordner ist notwendig, das hab ich ja selbst da reingelegt
DLLs finde ich so gar nicht. kA was ihr da habt

PS: den _Data Ordner der fliegt beim nächsten Push raus, sorry der war noch ein überbleibsel eines alten Builds den die ignore übersehen hat.

 

[Enurian]

Und soll man sich da was Spezielles ansehen? Ich meine das kostet Zeit sich da durchzuarbeiten...
Gerade mal zwei Klassen aufgemacht und aus denen folgende Hinweise:
- Dokumentation mehr als dürftig. Größtenteils überhaupt nicht vorhanden.
- Was sollen diese öffentlichen Felder? Keine Ahnung von Unity, aber in C# nutzt man Properties. Die haben einige entscheidende Vorteile (z.B. databind, reflection, debugging vom get + set). Die Kurzform ist immer noch ein Einzeiler und erlaubt spätere Erweiterung: public string MyStringProperty { get; set; }
- Dann werden mal Zugriffsmodifizierer verwendet und mal nicht - Hintergrund? Wenn du keinen angibst, entspricht das "internal", falls dir das nicht bewusst ist.
- Sowas geht gar nicht:
/// <summary>
/// 1: Normal, 2: Kartenzug
/// </summary>
internal int PlayMode;
Dafür gibt es enums.
- Erzeugung von Feldern mitten zwischen irgendwelchen Methoden ist auch kein guter Stil.
- Den Aufbau finde ich nicht sehr übersichtlich/logisch. Offenbar hast du dieses "Global", um dir Dinge wegzuspeichern, viel mehr macht es aber nicht.
Wäre es nicht praktischer, ein "Spiel" (o.ä.) Objekt zu haben, das zum Einen alle benötigten Daten beinhaltet und außerdem Dinge wie die Möglichkeit einen Spielzug zu machen? Sodass die (wie auch immer ausgelösten) Events dieses "Spiel" immer als Anlaufstation haben und lediglich ein paar Parameter (welcher Spieler, welche Karte) mitgeben?
Das aber nur als erste Vermutung, ich habe wie gesagt keinen Plan von Unity und mangels Kommentaren auch keine Lust mich durch die ganze Logik zu arbeiten.

 

[Kokujou]

ach du guter Gott
Also was die Dokumentation angeht bitte ich das zu entschuldigen aber da ich gerade dabei bin riesige Umstürze was die Gesamtstruktur meines Projekt angeht zu machen ist die Dokumentation eher zweitrangig. Ich hoffe zumindest dass du damit nur die Kommentare der einzelnen Funktionen meinst.

Zugriffsmodifizierer verwende ich immer dann wenn ich sie brauche. Internal... uff... Keine Ahnung. Ich kenne aktuell nur public und nicht public XD wenn irgendeine Variable bestimmte Events braucht wenn sie beschrieben wird füge ich get/set hinzu, ich weiß nicht warum ich das immer machen sollte. Ich versuche mir ja gerade ein besseres Bewusstsein dafür einzuprägen wann ich private und public nehmen sollte aber ... joa >.<

Okay das seh ich ein und da hast du Recht das war für mich zwecks Übersichtlichkeit, zu welcher Funktion sie gehören, aber da ich die alle warscheinlich eh nochmal irgendwohin abdocke ^^

Ja Global verwende ich als ne zentrale Klasse für alle Variablen und Einstellungen mehrerer Instanzen. Man hat mir hier geraten auf sowas zu verzichten also habe ich erstmal viele statische Funktionen in Erweiterungsmethoden ausgegliedert a la Linq.

Ehrlich gesagt ist das ganze Chaos für mich durch die vielen Klassen und neuen Skripte fast nur noch größer geworden XD Aber es ist nunmal ein größeres Projekt wo man nicht erwarten kann sofort durchzublicken. Eine zentrale Klasse für die Hintergrundarbeit habe ich tatsächlich. Die Klasse Spielfeld in der aktuellen Spielfeld.cs beinhaltet die Spieler, wer gerade am Zug ist (dass ich dafür ein bool verwende überdenke ich auch nochmal, die ganze Multiplayerkomponente wird nochmal Tage dauern bis ich die Fehler raus habe >.<), welche Karten im Deck und im Spielfeld liegen und die Player-Klasse beinhaltet dann Infos wie die Karten der beiden Spieler. Beide verfügen Funktionen über die Hintergrundarbeit.

Ich sollte vielleicht ne Ordnerstruktur in die doch inzwishcen sehr zahlreichen Skripte bringen... obwohl das auch nichts nützen würde denn die meisten gehören ja leider zum Hauptspiel. Mal gucken wie ich das besser hinkriege.

Grundsätzlich benutze ich die Main.cs aktuell zur Koordinierung, ich bin gerade dabei es so hinzukriegen dass sie wie ne Art Ablaufplan aussieht. Wenn ihr mal in den YakuManager schaut den ich gestern erstelllt habe, seht ihr, dass ich die ganze Regelung mit den Yaku in Komponenten abgedockt habe, was ich ehrlich gesagt sehr gelungen finde. Aber dass doch noch so viele Grundlagen-Fehler drin sind ist nicht ermutigend.

 

[Enurian]

Man kann auch in (deutlich) größeren Projekten schnell durchblicken.
Ich arbeite klassischerweise an Business-Applikationen mit GUI - Logik - Datenbank. Und die drei Teile sind auch im Code voneinander getrennt. Wenn da jemand neues kommt, kann er sofort sehen, dass ein ganzes Teilprojekt nur Datenmodell und DB-Zugriffe enthält. Hat er nicht vor, etwas daran zu ändern, braucht er sich das alles nicht ansehen. Er ruft nur auf "new XyzService().SpeicherMirDasWeg(Objekt objektMitDaten)".
Wenn ich irgendwelche neue Logik schreibe, wird das soweit separiert, dass es klar definierten Input und Output gibt. Da wird nicht zwischendurch z.B. irgendwas in die Oberfläche geschrieben (kann ich auch gar nicht, weil das meist Web Zeug ist ). Das Stichwort lautet "Separation of concerns".

 

[Kokujou]

Ja das sollte/wollte ich eigentlich auch so machen >.<
Nur irgendwie ist es dann schnell vermischt worden...
Die Hintergrundprozesse ziehen nunmal graphische Prozesse nach sich.

Aber so schlimm ist es auch nicht... lass mal sehen ich habe ein extra Skript für:
Das Hovern von Karten auf der Hand
Das Anzeigen von Yaku
das Zeichnen der Mobilen/PC GUI
Natürlich für die ganzen Stages davor, Startbildschirm, Oya-Aushandlung, Spielauswertung
Die Züge werden so separiert dass ich sie sowohl vom Multiplayer- als auch vom KI Modus aufrufen kann

Aber ich hab wohl noch viel zutun bis das ganze deutlich wird hm? >.<

Du kannst dir ja inzwischen mal die klasse YakuManager und YakuHandler ansehen. Die stehen inzwischen größtenteils allein und werden nur mit der Init aufgerufen.
Wie Yaku definiert sind siehst du in den Assets unter Assets/Resources/Yaku
Vielleicht sollte ich mal ein Readme verfassen dass zumindest die Spielregeln erklärt^^

So die Readme ist Online. Da stehen jetzt erstmal Grundregeln zum Projekt selbst damit jeder versteht was eigentlich abgeht^^

 

[Crast]

Nur irgendwie ist es dann schnell vermischt worden...

Dann solltest du anfangen alles wider auseinander zu ziehen. Wikipedia hat dazu einen schönen Artikel (hier klicken).

Die Hintergrundprozesse ziehen nunmal graphische Prozesse nach sich.

Wo ist das Problem? Angenommen Prozess A ist deine Business Logik und Prozess B kümmert sich um die View. Wenn Prozess A eine Änderung feststellt wird Prozess B über die Änderung informiert. Somit kann Prozess B unabhängig von Prozess A z. B. die Ansicht ändern. Genauso kann Prozess B eine Information losschicken, wenn der Nutzer eine Eingabe gemacht hat.
Der Vorteil ist dann, dass alles schön getrennt ist und leicht ausgetauscht werden kann - Stichwort Modular.

Um das zu erreichen, hast du zwei Module die voneinander vollkommen Unbabhängig sind (Okay.. sie teilen sich z. B. Typen).
Eine Mögliche Umsetzung wäre:
Beide Module kommunizieren über eine Fassade.
Beide Fassaden erlauben es Events zu triggern, indem eine Funktion direkt aufgerufen wird.
Beide Fassaden erlauben es sich für das Reagieren auf Events an- und abzumelden (Observable-Pattern)

Unterteilst du dein Programm in diese Teile (oft werden hier auch die Begriffe Frontend und Backenend verwendet), dann hast du auch die möglichkeit beides von einander unabhängig zu entwickeln.

Z. B. möchtest du ein bestimmtes Verhalten ändern, z. B. das berechnen von einem Wert, dann kannst du das machen, ohne die Gefahr zu haben das dein Frontend nicht mehr wie erwartet funktioniert, vorausgesetzt die Ein und Ausgabewerte ändern sich nicht.
Ebenso kannst du neue Funktionen einfügen, ohne direkt die GUI dafür an zu passen und vice versa.
Es können auch zwei Personen daran arbeiten. der eine kümmert sich um die GUI, der andere um die Logik. keiner kommt sich in die Quere

 

[Kokujou]

Ja ich bin dabei! es ist ein großes Projekt da kann ich nicht alles gleichzeitig machen aber es ist meine Priorität #1! Besonders weil ihr mir alle so freundlich helft, da kann ich doch nicht rumlahmen

Ja und genau da ist das Problem "Wenn prozess A eine Änderung feststellt". Soll ich die ganze Zeit schleifen laufen lassen ob sich das virtuelle Spielfeld verändert und demnach das echte verändern? Das klingt doch sehr unschön.

Aktuell ist es ja schon recht getrennt man betrachte die Erweiterungsmethode "StandardAnimation" die quasi ein einziges großes Wrapper für jede Animation ist. Der Aufbau hingegen erfolgt noch parallel. Die FieldSetup ist dafür ein Beispiel. Hier werden für jede Karte erstmal ein GameObject erzeugt, dem Deck zugewiesen und dann StandardAnimation für alle Karten getriggert damit die Aufbau-Animation stattfindet. Ist mir übrigens sehr hübsch gelungen Bei den Karten ist es dann noch intensiver, denn die dispersieren mit ner Abweichung von -5 bis 5 vom Ursprungsort sowie einem Winkel von -60 bis 60, wenden sich beim Spiele, falten sich zusammen zu einem Stapel und fächern sich dann auf wie man es auch real kennt.
Auch hierbei: Alles graphische regelt quasi die Funktion StandardAnimation. Aber aufgerufen wird sie halt in der Funktion die auch die visuelle Spielumgebung erstellt und die GameObjects werden darin auch erstellt...

Ein bischen abdocken könnte ich vielleicht besonders in dem Part dafür müsstet ihr mir aber eins sagen:
Kann man die List.Add Methode irgendwie ein Event auslösen lassen? Oder sie überschreiben oder irgendwie was anhängen?

DANN könnte ich nämlich jedes Mal wenn ich einem meiner Kartendecks eine Karte hinzufüge oder entferne UI-Aktionen triggern lassen... zumindest in bestimmten Fällen. Vielleicht sogar automatisch Remove-Aktionen Triggern weil die Karte ja nur in einem Mitglied ist.

 

[Crast]

Soll ich die ganze Zeit schleifen laufen lassen ob sich das virtuelle Spielfeld verändert und demnach das echte verändern? Das klingt doch sehr unschön.

Ihhh! NEIN!
Schau dir das Observable Pattern an. Im Grunde hast du zwei Klassen.

using System;

class Observable
{
    public event EventHandler SomethingHappened;

    public void DoSomething()
    {
        EventHandler handler = SomethingHappened;
        if (handler != null)
        {
            handler(this, EventArgs.Empty);
        }
    }
}

class Observer
{
    public void HandleEvent(object sender, EventArgs args)
    {
        Console.WriteLine("Something happened to " + sender);
    }
}

class Test
{
    static void Main()
    {
        Observable observable = new Observable();
        Observer observer = new Observer();
        observable.SomethingHappened += observer.HandleEvent;

        observable.DoSomething();
    }
}

Die Klasse Observable loest zu beliebigen Zeitpunkten ein oder mehrere Events aus.
Die Klasse Observer uebergibt an Observable einen Handler, der aufgerufen wird, wenn ein Event ausgeloest wird.

Zwei Hilfreiche Artikel dazu
http://csharpindepth.com/Articles/Chapter2/Events.aspx
https://www.philipphauer.de/study/se/design-pattern/observer.php

In High Level Languages wie C# (und besonders bei OOP) sind komische Schleifen, unterschiedliche Verhaltensweisen ueber If bzw Switch zu etwa 90% vermeidbar, wenn man sich nur bemueht.
Verwendet man dann auch noch ein Framework wie Unity, dann wuerde ich behaupten sind solche (haesslichen) Konstrukte zu 99% vermeidbar.

 

[Kokujou]

Das ist das was ich immer in C# benutzt habe... Dieses != null kann ja inzwischen auch durch den Fragezeichen Operator gelöst werden bei Funktionen war das dann glaube ich function?.Invoke();

Aber so hundertprozentig versteh ichs noch nicht. Wo sieht man denn WANN das event ausgelöst wird? Wo definiert man was?
Sagen wir ich will immer ein Event auslösen wenn in einer Liste etwas hinzugefügt wurde. Das ist ja das Problem.

So wie ich das sehe bringst du die Klasse dazu etwas zutun und löst dann quasi dieses "Event" aus, was ja dann quasi nur noch ne Funktion mit bestimmten Parametern ist. so ähnlich mache ich es in vielen Fällen nur dass ich hier und da mal die getter und setter bemühe.

Aber wenn ich jedes Mal wenn ich ner Liste was hinzufüge diese Funktion explizit aufrufen muss dann ist das nicht so toll...

Das beste was mir dazu einfiele wäre sowas:

public class CardList : List<Card>
{
    public void AddCard(Card card)
    {
        this.Add(card);
        OnCardAdded();
    }
}

Aber irgendwie hab ich noch nicht das Gefühl dass das zufriedenstellend ist. Selbst wenn ich es über ne Erweiterungsmethode so mache, aber wenn ihr meint dass das völlig valide und guter Stil ist dann mach ich das vielleicht einfach so.

 

[Crast]

ber so hundertprozentig versteh ichs noch nicht.

Les dir bitte den Link hier durch: http://csharpindepth.com/Articles/Chapter2/Events.aspx
Da wird es sehr schoen erklaert. Ich weiss du liest nicht gerne, aber daran musst du dich gewoehnen.

Wo sieht man denn WANN das event ausgelöst wird?

Dem Observer interesiert es nicht, wann ein Event ausgeloest wird. Er sagt dem Observable nur, was gemacht werden soll, wenn eins ausgeloest wird.
Der Observable entscheidet wann ein Event ausgeloest wird und fuehrt nur blind das aus, was die Observers ihm gesagt haben.

Sagen wir ich will immer ein Event auslösen wenn in einer Liste etwas hinzugefügt wurde. Das ist ja das Problem.

Hier kurz mit Pseudo Code

class ClassWithList {
    private List<any> list = [];
    
    private List<Handler> handlers;
    
    public void subscribe(Handler handler) {
        handlers.push(handler);
    }
    
    public void add(any item) {
        list.push(item);
        notifyObservers;
    }
    
    private void notifyObservers() {
        handlers.forEach((handler) => {
            handler.call(list);
        });
    }
}

class Main {
    public static main(String[] args) {
        ClassWithList classWithList = new ClassWithList();
        
        classWithList.subscribe((list) => console.write("List Update. New length: ", list.length));
        
        classWithList.add("V1") // -> console output: List Update. New length: 1
        classWithList.add("V2") // -> console output: List Update. New length: 2
        classWithList.add("V3") // -> console output: List Update. New length: 3
        classWithList.add("V4") // -> console output: List Update. New length: 4
    }
}

Aber das duerfte das sein, was du auch machst, richtig?

 

[Kokujou]

Dem Observer interesiert es nicht, wann ein Event ausgeloest wird. Er sagt dem Observable nur, was gemacht werden soll, wenn eins ausgeloest wird.
Der Observable entscheidet wann ein Event ausgeloest wird und fuehrt nur blind das aus, was die Observers ihm gesagt haben.

Ja aber woher weiß es der Observable!

Okay also im Prinzip ist es etwa so wie ich es mir gedacht habe. Ein bischen anders da du hier ne Liste mit Handlern erstellt hast, aber ich schätze dann mach ich es wohl demnächst auf diese Weise... Ich frag mich warum generische Listen nicht gleich Events für sowas zur Verfügung stellen... Ist ja wohl nicht das unüblichste dass man Events auslösen will wenn von den unzähligen Standorten mal was hinzugefügt wird^^ Aber andererseits hab ich dieses Eventbasierte Vorgehen seit ich mich mit C# beschäftige gar nicht mehr gesehen.

Das letzte mal existierte das eigentlich nur noch in Windows Forms und bei Xamarin in sehr sehr kleinen moderaten Dosen.

Ich überlege auch ob ich das System der Karten nochmal überarbeite und sie den GameObjects in Unity als Komponente hinzufüge. Logisch gesehen ist das zumindest der sinnvollste Schritt weil man sie ja so oder so miteinander verknüpft.

Ergänzung (15. Dezember 2018)

Eine Sache möchte ich an dieser Stelle noch für alle Beteiligten Hinzufügen: Danke!
Ehrlich ich hab immer Probleme mich zu motivieren weiterzuarbeiten aber seit ihr mir helft hab ich richtig Bock weiterzuarbeiten^^ Endlich lerne ich mal was, auch wenn's ne ziemliche Fummelei ist den Spagghettisierten Code auseinanderzufriemeln macht es Spaß endlich zu wissen wie n richtiger Programmierer an sowas rangeht.
Bald kann ich meine Arbeitgeber mit wunderschönem künstlerisch wertvollem Code beeindrucken Naja Spaß muss sein^^

 

[Darlis]

Ich frag mich warum generische Listen nicht gleich Events für sowas zur Verfügung stellen

ObservableCollection<T> Class

 

[Kokujou]

das geht?! wow. hätte ich das gewusst. das hätte bei 3 projekten geholfen

 

[Crast]

Ja aber woher weiß es der Observable!

Der Obervable weiss es, weil er der jenige ist, der die Operation durchführt und anschließend die benachrichtigung abschickt. Er entscheidet auch wann bzw ob und was informiert wird.

Ein bischen anders da du hier ne Liste mit Handlern erstellt hast, aber ich schätze dann mach ich es wohl demnächst auf diese Weise...

Das war Pseudo Code. Die meisten Highlevel Sprachen bieten dafür interfaces, Klassen zum vererben oder direkte Implementierungen. Bitte informier dich für C# selbst etwas genauer darüber.
@Darlis hat dir ja auch bereits eine Implementierung gegeben.

 

[Enurian]

das geht?! wow. hätte ich das gewusst.

Sobald du das Gefühl hast, dass etwas viel zu umständlich ist / "doch eigentlich gehen müsste", einfach danach googlen.
Das macht jeder Softwareentwickler und ohne dem kommst du nicht weit. In diesem Fall hättest du nach Sekunden die Lösung gehabt.
Suchbegriff in etwa "c# event when list changes"

 

[Nero1]

Kann ich @Enurian nur beipflichten. Google, Stackoverflow, meine Hauptquellen wenn ich Code. Ich brauch sie halt doch immer wieder

 

[Kokujou]

Mach ich ja eigentlich und hatte ich auch mal... kA ob das jetzt irgendwie neu ist oder ob mich Google einfach nur nicht leiden kann aber damals hatte ich das nicht gefunden vor ein paar Jahren als ich es gebraucht habe XD
Seitdem hatte ich gar nicht mehr dran gedacht dass sich mal einer die Mühe gemacht hat dafür.

Das ist wie damals als ich mir HTML angeeignet habe und dann plötzlich sagte man mir "das ist alles total veraltet" und ich steh da wien Schlumpf und hatte keine Ahnung.

Das gefährliche ist wenn man wie selbstverständlich davon ausgeht dass es das nicht gibt.

 

[Crast]

vor ein paar Jahren

Naja... die IT ist etwas schnelllebiger als manch andere Bereiche.
Es komen täglich neue Frameworks hinzu, bestehende werden aktualisiert.
Neue Standards werden schneller Entwickelt als sie veröffentlicht werden können.
Neue "LTS" Versionen laufen shneller aus, als man migrieren könnte.

Wenn es ums Programmieren geht, kannst du dich nicht auf die Techniken von vor ein oder zwei Monten verlassen. Es ist ein ständiges neulernen. Stillstand bedeutet Rückschritt.

 

[Kokujou]

Ja stimmt ^^ darum sag ich ja es ist schon fatal wenn man etwas als selbstverständlich annimmt^^
Aber man kann auch nicht jedes bischen nachgoogeln, dann würde man ja nie fertig.
Das ist ja einer der Gründe für diesen Thread.
Wo ist meine Methodik veraltet, wo benutze ich Klassen die nur ein Teil von dem Können was ich eigentlich hätte verwenden können, was für Funktionen machen viel zu viel Aufwand für das was andere viel effizienter gelöst kriegen.

Sicher gibt es Beispiele für die String-Verknüpfung auch in größerem Stil, bestimmt gibts haufenweise Funktionen die durch viel effizientere Algorithmen gelöst worden.

 

[andy_m4]

Naja... die IT ist etwas schnelllebiger als manch andere Bereiche.

Quantitiv sicher ja. Qualitativ eher nein.
Wenn ich da regelmäßig sehe das da so Sachen hoch kommen, die man schon mal vor 20 30 Jahren gesehen hat, dann habe ich da so manchmal meine Zweifel.
Aber vielleicht ist es ja das, was die Leute mit Retro-Computing meinen. :-)

Ich weiß auch nicht, ob es Sinn macht jedem Hype irgendwie hinterher zu jagen und jedes Framework or whatever zu lernen, wo gesagt wird "das ist die Zukunft" und nach 2 Jahren hört man nix mehr von.
Und es nützt einem auch in der Praxis nicht viel, wenn man zwar viel kann aber nix richtig, weil man nur schafft sich oberflächlich damit zu beschäftigen.

Dann lieber einen fundierten Grundstock an Wissen erarbeiten und von dem ausgehend kann man ja dann auch mal ab und an neue Sachen hinzu nehmen, wenn sie sich als sinnvoll erweisen.