Elektroda.pl
Elektroda.pl
X
Proszę, dodaj wyjątek dla www.elektroda.pl do Adblock.
Dzięki temu, że oglądasz reklamy, wspierasz portal i użytkowników.

[C#] Gra w kości - Wasze opinie i porady

03 Lut 2014 16:59 2094 8
  • Poziom 6  
    Witam!

    Chciałbym Wam przedstawić moją kolejną aplikację napisaną w C#. Jest to bardzo dobrze wszystkim znana - gra w kości. Do jej napisania użyłem Windows Forms.

    Bardzo bym prosił, abyście pisali wszelkie swoje sugestie co do samej aplikacji jak i do kodu, bo wiem, że daleko mu od ideału. Niektóre rzeczy mogłem inaczej rozwiązać - tym bardziej sprawdzanie poszczególnych układów. Od razu chcę powiedzieć, że jest to projekt na studia (na zaliczenie przedmiotu, 3 rok). Używałem w niej tylko i wyłącznie polskich nazw (tak wiem, angielski tylko i wyłącznie, jednak z racji tego że na uczelnie postanowiłem robić po polsku). Także czekam na Wasze wszelką krytykę, porady i sugestie.

    W załączeniu przesyłam projekt.


    PS. ...JAK GRAĆ...
    Po maksymalnie trzech rzutach klikamy na punkty które chcemy przy dowolnym układzie.
    Pozdrawiam!
  • Pomocny post
    Poziom 42  
    Hm, po wykonaniu trzech rzutów przycisk mi się zablokował... i co dalej?

    Okienko drugie powinno raczej zastępować okienko 1... Tamto nie jest do niczego potrzebne i powinno być ukryte wg mnie. Ponadto dwa przyciski powstające na pasku zadań (pewnie chodzi o uchwyt do aplikacji). Przycisk maksymalizacji jest pozbawiony sensu w oknie o stałym rozmiarze. Jeśli już dawać wybór kolorów okna, to niech to będzie pełna paleta (korzystając z systemowego okienka wyboru kolorów).
    Brak obsługi przycisku "Anuluj" podczas wpisywania imion.

    To tak... Na kod nie patrzę, bo słabo znam C#. Nikt nie krytykuje też za pisanie kodu z użyciem polskich pojęć :) (byle nie stosować znaków diakrytycznych w identyfikatorach).
  • Poziom 6  
    Dżyszla napisał:
    Hm, po wykonaniu trzech rzutów przycisk mi się zablokował... i co dalej?


    Klikasz na punkty, które byś chciał za dany układ ;)

    Dżyszla napisał:
    Okienko drugie powinno raczej zastępować okienko 1... Tamto nie jest do niczego potrzebne i powinno być ukryte wg mnie. Ponadto dwa przyciski powstające na pasku zadań (pewnie chodzi o uchwyt do aplikacji). Przycisk maksymalizacji jest pozbawiony sensu w oknie o stałym rozmiarze.


    Poprawione. Wrzuciłem poprawione w pierwszym poście ;p Dzięki za porady :)
  • Pomocny post
    Poziom 40  
    Hmm, ja podobnie nie wiedziałem, co zrobić po zakończenie rzutów - może jakiś komunikat ustawić i zmienić ikonkę kursora nad elementami do kliknięcia?

    Jeśli chodzi o kod, to w kilku(nastu) miejscach bym się przyczepił... Jak widzę kod gdzie jest kilka if'ów jeden pod drugim od razu wiem, że coś tu jest nie tak...

    No to po kolei, co znalazłem - a kolejność przypadkowa i z pewnością nie wszytko:
    1. Form1: deklarujesz zmienne
    Kod: csharp
    Zaloguj się, aby zobaczyć kod
    których potem używasz tylko jeden raz:
    Kod: csharp
    Zaloguj się, aby zobaczyć kod

    W takim wypadku nie ma sensu ich deklarowanie na górze klasy, wystarczy lokalnie, zaraz przy wywołaniu.

    2. Stosujesz typ zmiennych String pisany z dużej litery, gdy generalnie powinno być pisane z małej (co prawda to aliasy, znaczą to samo, ale tak się jakoś przyjęło - pozostałe typy stosujesz z małej...).

    3. Gra ma niczego sobie interfejs graficzny, a do wprowadzania imion używasz - na siłę bo importując z VB - InputBoxów. Jak już się tyle narobiłeś to wysmaruj jeszcze jedną formę z dwoma textboxami i przyciskiem.

    4. Właściwości Imie1 i Imie2 są tak naprawę niepotrzebne - nie są jakoś logicznie powiązane z Form1. Skoro używasz ich tylko raz, do stworzenia klasy Gra, przekaż te zmienne w konstruktorze klasy Gra (plus pousuwaj zbędne wtedy zmienne - pkt 1).

    Gra.cs
    5. .NET bodaj od wersji 3.0 oferuje automatyczne właściwości: zamiast pisać
    Kod: csharp
    Zaloguj się, aby zobaczyć kod

    możesz napisać po prostu:
    Kod: csharp
    Zaloguj się, aby zobaczyć kod


    6. Te wszystkie właściwości typu Label, PictureBox - tego naprawdę nie dało się uniknąć? Jeśli już chcesz manipulować całymi kontrolkami, po prostu je upublicznij. Metody get; set; jest sens robić wtedy, gdy ustawiasz pojedynczą właściwość, np. Label.Text

    7. Do dyspozycji masz całą masę kolekcji - tablice, listy itp., dlaczego z nich nie skorzystasz, tylko deklarujesz po 5 zmiennych tego samego typu (np. obrazki, kości etc.)?

    8. Stwórz sobie klasę Kość, która określa wszystkie parametry pojedynczej kostki - obrazek, zaznaczenie etc. Wtedy takie metody jak te:
    Kod: csharp
    Zaloguj się, aby zobaczyć kod
    będziesz mógł znacznie uprościć.

    Figury.cs - ojj, tutaj można poszaleć...
    9.
    Kod: csharp
    Zaloguj się, aby zobaczyć kod

    Po co tak powielać?? Nie widzisz, że można z tego zrobić jedną, wspólną metodę z parametrem? Wyniki losowania można umieścić w tablicy, wtedy sprawdzanie zrealizujesz jedną pętlą for...
    Podobnie pozostałe figury - nie chce mi się dokładnie sprawdzać ale gołym okiem widać powtarzające się fragmenty, które można wydzielić do pętli for... i ew. tablicy (zmienne pomocnicze są niepotrzebne).

    Zasady gry.cs
    10. Po co powielasz kilkanaście różnych form, które różnią się tylko obrazkiem i opisem?? Śmiało można zastosować jedną, ogólną formę, na którą ładujesz odpowiedni obrazek i opis z przygotowanej wcześniej tablicy. Jak zamiast jednego obrazka potrzebujesz 5, to albo 4 ukryj, albo zrób dwa panele - jeden z jednym obrazkiem, drugi z pięcioma.

    Myślę, że na razie wystarczy ;) Także generalnie powinieneś pójść w kierunku uproszczenia kodu - i nie pisz że się nie da czegoś zrobić bo coś tam. Wierz mi, da się. Tylko trzeba trochę się zastanowić jak nieco "sprytniej" temat ugryźć.
    Pozdrawiam
  • Pomocny post
    Poziom 33  
    Moim zdaniem lepiej było by zrobić wszystko w jednym okienku, np. na kontrolkach. Nie zablokowałeś wyświetlania strzałek zmiany rozmiaru okna ani przycisku maksymalizacji. Zablokuj możliwość edycji zasad gry.
    Co do kodu - trzy ukośniki stosuje się do opisu elementu pod nimi (np. metody). Opis ten jest widoczny w podpowiedziach. Do takiego opisu jak stosujesz możesz użyć #region. Wtedy można cały region zwinąć.
    Dlaczego zmienne pomocnicze w klasie Figury są zadeklarowane w klasie, a nie w metodach? Po co klasa "trzyma" obiekt gry skoro go nie używa. Równie dobrze mogłaby to być klasa statyczna. Warunek sprawdzania fula można znacznie uprościć.
    Takie wykorzystanie elementów statycznych jak w klasie Losowanie to nie jest dobra praktyka.
    Rozumiem, że przez klasę Rozgrywka chciałeś oddzielić logikę gry. W takim razie nie powinno tam być niczego z System.Windows.Forms.
  • Poziom 6  
    Na wstępie dziękuję bardzo za tak obszerne odpowiedzi i zainteresowanie. Właśnie o takie uwagi mi chodziło. Wiem, że właśnie przez takie projekty człowiek się uczy na błędach, zwłaszcza, gdy ludzie są skłonni do pomocy :)

    marcinj12, wszystko co mi napisałeś doskonale rozumiem i będę mógł się zabrać do uproszczenia i optymalizacji kodu.

    marcinj12 napisał:
    Zasady gry.cs
    10. Po co powielasz kilkanaście różnych form, które różnią się tylko obrazkiem i opisem?? Śmiało można zastosować jedną, ogólną formę, na którą ładujesz odpowiedni obrazek i opis z przygotowanej wcześniej tablicy. Jak zamiast jednego obrazka potrzebujesz 5, to albo 4 ukryj, albo zrób dwa panele - jeden z jednym obrazkiem, drugi z pięcioma.

    Masz rację. Myślałem w jaki sposób to zrealizować i pierwsze do głowy przyszło mi dziedziczenie i tak też zrobiłem i byłem zadowolony. Ale widzę, że można jeszcze znacznie prościej to wykonać :)
    Jeśli chodzi o if'y... Zdaje sobie sprawę, że jest to wykonane w najgorszy z możliwych sposobów, po prostu czas mnie gonił i musiałem to szybko ogarnąć dlatego tak na sztywno to wszystko wyszło...


    LED5W, fajne, praktyczne porady w pigułce. O #region nawet nie słyszałem, fajnie wiedzieć :) Właśnie zawsze mam problem z oddzielaniem logiki od interfejsu, ale walczę tym na każdym kroku.

    Raz jeszcze dzięki, zabieram się za poprawki!

    Pozdrawiam
  • Pomocny post
    Poziom 42  
    Popracowałbym jeszcze nad intuicyjnością interfejsu. Dla osoby, która nie grała nigdy - raz, że to klikanie w liczby jest mało intuicyjne, dwa... Dopiero po jakimś czasie załapałem, że można kliknąć tylko raz w dany układ (nie gram w kości, opis czytałem pobieżnie). Ergo - przydał by się skrót do opisu gry wprost z okna gry.
    Jak ktoś wspomniał - okna choć już nie posiadają maksymalizacji, to jednak dalej sąw stylu sugerującym możliwość zmiany rozmiarów (a przy ustawieniach nawet jest to możliwe).

    A może by tak jakieś zdjęcia kostek, które losowo by na dodatek układały się na jakimś obszarze, byle by nie nachodziły na siebie? Faja funkcja by tu była ze sprawdzaniem czy się przecinają.
  • Poziom 6  
    Dokonane zmiany:

    - Dodany skrót do tego JAK GRAĆ już podczas grania
    - Wyłączona możliwość maksymalizacji we wszystkich oknach + kursora do rozszerzania
    - Gdy przechodzimy do gry znika nam okno z menu
    - Inne drobne poprawki (typu zablokowanie edycji TextBox'ów)
    - Ogólnie nieco poprawiony sam kod (losowane kości wrzucone w tablice i sprawdzanie czy wystąpił dany układ - wszystko powrzucane w pętle i zrealizowane w jednej przeciążonej metodzie z parametrem)

    Jeszcze do zmiany:

    - Zmiana sposobu wprowadzania imon
    - I tak jak radzi Dżyszla poprawienie klikania w liczby, aby było bardziej intuicyjne
    - Oraz inne drobne poprawki


    Dżyszla napisał:
    A może by tak jakieś zdjęcia kostek, które losowo by na dodatek układały się na jakimś obszarze, byle by nie nachodziły na siebie? Faja funkcja by tu była ze sprawdzaniem czy się przecinają.

    Nie bardzo rozumiem, co masz na myśli? :)


    PS.
    Kod źródłowy podam jak jeszcze go bardziej zoptymalizuję
  • Poziom 42  
    XardasLord napisał:
    Nie bardzo rozumiem, co masz na myśli? :)

    Jak rzucasz kostkami na stole, to raz jedna zatrzyma się przy jego lewej krawędzi, dwie na środku... Innym razem wszystkie będą na środku, a innym razem... spadną ;) Chodzi mi o to, by takie losowe położenie na danym obszarze odzwierciedlić w programie. Czyli nie rządek pionowy z liczbami, ale trochę mniejsze kości i gdzieś na prostokątnym obszarze rozrzucone.
    [C#] Gra w kości - Wasze opinie i porady