FURA gra w kości

wymyśliłem i zrobiłem taką grę w kości, czekam na opinie i hejty (idealna okazja)
w załączniku instalka

https://github.com/chrzastek/FURA

• nie comituje sie binarek. takze bin/debug bin/release i publish nie powinny byc w repo
• magic numbery
• MainWindow jest za duzo, nie ma klasy ktora obsluguje logike gry. Wszystko sie dzieje w sumie w MainWindow
• z tego co wiem (moge sie mylic) regionow juz sie nie uzywa. Nie sa po prostu uzyteczne
• funkcje powinny wykonywac jedna akcje. Tak wiec funkcja CreateAndFillHoFFile jest po prostu zla (bo robi dwie rzeczy). Lepiej ja rozbic na SetupHoFFile() a w tym zrobic Create a pozniej Fill
• GetRandoms lepiej uzyc tutaj Dictionary i zmapowac dwie kolekcje (wtedy nie bedzie Ci potrzebne to i)
• GetRoundSum spokojnie mogloby zwracac string (uzywasz tylko w jednym miejscu)
• a co jezeli gra kobieta? Wtedy komunikaty nie sa poprawne
• jezeli to wpf to czemu nie bindujesz tylko ciagle robisz toString i dajesz do text? Przeciez to latwo idzie zbindowac.
• pomieszane polskie z angielskimi nazwami

ogolnie projekt jest maly, ale jest czytelny. Dalbym 6/10 bo czytalo sie to ok.

-też to gdzieś wyczytałem już później. Usunąłem te foldery
-starałem się unikać magic numberów ale masz rację, wkradł się jeden
-regiony były dla mnie użyteczne do przemieszczania się po kodzie ale pewnie dlatego bo jak sam napisałeś cała logika jest w klasie MainWindow. Co z tym zrobić? Wyodrębnić do osobnej dllki?
-co do funkcji GetRandoms() to po prostu wiedziałem że da się to zrobić inaczej i kuło mnie w oczy to i, ale nic nie wymyśliłem innego... Spróbuje z tym dictionary
-grafika jest w Windows Formsie
-nazwy zmiennych, funkcji, klas i obiektów nie wypadają z przyjętej konwencji C# ale interfejs gry jest po polsku więc to się gryzie. Następny projekt będzie miał angielski interfejs

Dzięki za opinię

Poza tym co kolega wyżej napisał, to bym zmienił tego ifa https://github.com/chrzastek/FURA/blob/master/MainWindow.cs#L216
Odwróciłbym warunek do np takiej postaci:

 if (hofMembers.Count >= 5 && points < hofMembers.Max.Points) {
  return;
}

i tracimy wg mnie zbędne zagnieżdżenie.

Dodatkowo podzieliłbym na foldery cały projekt. Np okna w oddzielnym folderze, klasy związane z logiką biznesową w oddzielnym itp

1. można zmienić, jeżeli dla Ciebie to bardziej czytelne

public int Compare(HoFMember member1, HoFMember member2)
{
    if (member1.Points > member2.Points)
        return -1;
    if (member1.Points < member2.Points)
        return 1;
    else
        return 0;
}

na

public int Compare(HoFMember member1, HoFMember member2)
{
    return member2.Points - member1.Points;
}

(Jeżeli koniecznie potrzeba liczby 1 oraz -1 to użyj funkcji Math.Sign)

2. Może moje spaczenie z javy ale ja lubię mieć

private Random r = new Random();

zapisany tak private static readonly Random random = new Random();

3. Kurcze szkoda że nie mam visuala pod ręką bo bym Ci to fajnie zrefactorował (a bez IDE trochę strach że coś zepsuje :D) ale ogólnie jedyne co bym zrobił to miejsca w MainWindow takie jak:
linijki 54-62 do nowej metody
linijki 64-72 do nowej metody
linijki 41-74 do nowej metody

4. `buttonPass_Click()` i `odNowaToolStripMenuItem_Click` ma pomieszany widok z logiką. (W jednym miejscu decyduje się co robić oraz np ustawia treść labelkom -> nie ładne)

5. Mógłbyś ogólnie całą logikę dać do innej klasy a widok ewentualnie zostawić w MainWindow. To MainWindow teraz za dużo robi niestety.

6. `ResetDices`, `GetRandoms`, `GetAmountOfUncheckedBoxes`, `CreateAndFillHoFFile`, `SerializeSet`  i `GetRoundSum` ładnie wyglądają! Brawo :D