Polepszenie architektury/rozwiazania

Chce napisac takie "cos" co mi wyeksportuje plansze do jsona (to jest najwazniejsze zadanie). Zrobilem to, ale nie podoba mi sie w tym pare rzeczy i nie za bardzo wiem jak to zmienic.

http://pastebin.com/U66tK1A3

tutaj jest do tego kod. Pisany dosc szybko, takze formatowanie w niektorych miejscach moze byc popsute.
GenerateBoard jest tylko po to zeby sprawdzic czy dziala json.

taki maly offtop po srodku Ktory code styleguide polecacie do c#?

Idea jest nastepujaca

1. Aplikacja bedzie napisana w WPF
2. Uzytkownik na poczatku podaje rozmiar mapy ktora nie moze sie zmienic. Dlatego na poczatku napisalem mSize_x, mSize_y, ale widze ze nie bedzie to potrzebne, bo jak sie zrobi data-grida to tam sie ustali na sztywno. Tego jeszcze nie obmyslilem jak do konca zrobie
3. Uzytkownik moze wybrac sposrod trzech kategori (tlo, obiekt, przeciwnik) co chce umiescic na mapie
4. Uzytkownik przeciaga np tlo (ktory zajmuje jedno miejsce w tablicy) na mape, na to moze dac obiekt a pozniej na to jeszcze moze dac przeciwnika (wiec background bedzie mialo najmniejszy wspolczynnik layer a enemy najwiekszy)
5. Jak kliknie w opcje eksport, to wyekportuje to do pliku

Czego tutaj brakuje / mi sie nie podoba

1. mam takie wrazenie ze mBackBoard mFrontBoard i mEnemyBoard jest troche lamaniem zasady DRY. Mimo wszystko inne, ale jednak korzysta sie z nich tak samo. Ja nie potrafie tego inaczej zrobic niz trzymanie trzech osobnych tablic skoro chce trzy inne kategorie. Co za tym idzie jest troche "duplikatow". Funkcje wygladaja praktycznie identycznie tylko nazwa zmiennej sie rozni.
2. Czy nie za bardzo rozdrobnilem tych funkcji? Bo przez to zamiast robic raz for-loopa robie go trzy razy (dane sa male, takze pewnie jakbym zrobil jeszcze z 10 razy to odczuwalnej roznicy by nie bylo... ale jednak)

Za kazde uwagi bede wdzieczny :)

InitializeXXXBoard możesz zastąpić jedną metodą, przekazując do niej konkretną listę.

A z innej strony zamiast mieć x list to możesz mieć jedną listę obiektów, które będą miały x pól

na prawdę potrzebne Ci są metody AssignToXXX? - przecież plansze masz publiczne

No a tym samym tropem idąc może mieć też jedną metodę AssignToBoard. W sumie, można by pójść dalej - przerób kod tak, byś mógł mieć dowolną liczbę tablic, nie, @Shalom? :)

macie racje, assignToBoard nie jest potrzebne.

Zmodyfikuje kod tak by byl uniwersalny i posiadal X list, ale problem jest taki ze... bedzie trzeba uzywac property dla kazdego obiektu z listy (by sie do niego dobrac). Wiec mimo ze moge miec X list, to i tak bede mogl uzywac tylko tych ktore zaimplementuje property... rozwiazanie nie jest idealne ;)

(property dla foreBoard, dla backBoard etc. Jak w przyszlosci bede chcial zaimplementowac kolejny np skyBoard to bede musial zrobic kolejne property)

http://pastebin.com/BsRNDWfM

wyglada duzo lepiej :) wieczorem sobie to ladnie dopracuje :)

pare rzeczy ktore bym zmienila (nic super powaznego, bardziej na zasadzie czepialstwa):

• wywalic prefixy 'm' z nazw pol
• wywalic '_' z nazw zmiennych
• nazwy xIndex, yIndex wydaja sie troche zbyt dlugie, x i y w zupelnosci by wystarczyly
• skoro rozmiar kolekcji jest staly to nie powinno byc mozliwosci mutowania jej z zewnatrz
• klasy powinny byc sealed
• wszystkie pola powinny byc readonly
• List<List<BoardObjects>> powinno byc nowa klasa (lub ew. zamienione na BoardObjects[,])

co do code style to polecam guideline na msdn

katelx napisał(a):

• wywalic prefixy 'm' z nazw pol
• wywalic '_' z nazw zmiennych

Jeżeli chcesz stosować rozróżnienie prywatnych i publicznych składników to możesz stosować prefiksowanie znakiem _, np. private int _xSize. To chyba jedyny element, gdzie stosuje się znak podkreślenia zgodnie z coding style.

Krzywy Młot napisał(a):

Jeżeli chcesz stosować rozróżnienie prywatnych i publicznych składników to możesz stosować prefiksowanie znakiem _, np. private int _xSize. To chyba jedyny element, gdzie stosuje się znak podkreślenia zgodnie z coding style.

szczerze to nie wiem czy obecny coding style guidelines jest za stosowaniem podkreslenia, kiedys uzywalam '_' do prywatnych pol ale dalam sobie spokoj i obecnie nie uzywam zadnych magicznych prefixow poza 'I' przy interfejsach (co swoja droga nie jest ok, ale niestety framework stosuje taka konwencje i byloby to po prostu brzydkie gdybym to 'I' omijala; duzo bardziej podoba mi sie podejscie zastosowane np w jdk).
imho jesli klasy maja dobrze dopracowana strukture, odpowiedzialnosc i nazewnictwo skladowych to dodatkowe prefixy bardziej przeszkadzaja niz ulatwiaja.

katelx napisał(a):

szczerze to nie wiem czy obecny coding style guidelines jest za stosowaniem podkreslenia, kiedys uzywalam '_' do prywatnych pol ale dalam sobie spokoj

W zasadach kodowania dla CoreFx (https://github.com/dotnet/corefx/wiki/Coding-style) jest stwierdzenie "We use _camelCase for internal and private members", więc chyba obecny jest nadal za podkreśleniem. Ja osobiście zazwyczaj wolę po prostu nazywać z małej litery prywatne, z wielkiej publiczne i sprawa z głowy.

A może to przyzwyczajenie, ale jakoś mnie bardziej odpowiadają interfejsy z prefiksem I od konwencji z Javy.

Tego Randoma bym dał jako pole statyczne, no chyba że klika wywołań pod rząd ma dać ten sam wynik.

@katelx
prefixy m sa od member. Pomagaja mi w szybszej implementacji ;) (ale zamienie je na _)
wywale _ w nazwach zmiennych, ale dodam na poczatku, podoba mi sie code styleguide od Ktos'ia :)
Uzywalem przez dlugi czas x,y do 2d tablic. Jednak gdy przychodza duzo bardziej skomplikowane operacje, przydaje sie xIndex yIndex a raczej coIndex (bo w pewnym momencie musialem operowac na prawie dziesieciu indeksach...)
Jak zrobic by udostepnic ja (zeby mozna bylo zmieniac wartosci) ale zeby nie mozna bylo zmieniac wielkosci? (w sensie zeby nie mozna bylo uzyc funkcji Add) (wiem ze tablica rozwiazuje sprawe, ale nie o to chodzi ;))
sealed dodane :)
czyli uzywanie zmiennych w ramach tej samej klasy jest zle? Trzeba uzywac proprerty zamiast tego? Ma to po czesci sens (jezeli za kazdym razem zmienna musi miec jakas procedure przypisania), ale co jesli wewnatrz klasy trzeba miec inny sposob przypisywania? To wtedy usuwamy readonly? Bo w takim razie czesc bedzie readonly a czesc nie... troszke dziwnie
Co do ostatniego to calkowita racja, zapewnie zmienie na BoardObjects[,], pozniej jak projekt bedzie ewoluowal to zrobie z tego klase
sprawdze tego msdna ;)

@Ktos
Nie chce nazywac wszystko co publiczne z duzych (a raczej property) bo od duzych liter sa funkcje a nie zmienne ;) A ze kompilatory sa case sensitivity to pomaga pisac zmienne/property z malej

@fasadin
List<List<BoardObjects>> odradzam takie konstrukcje ;) Bo szybko ewoluują w dziwne potworki typu Dictionary<String, Dictionary<String,List<Set<String>>>> w których to następnego dnia już sam nie wiesz co siedzi i co jest czym ;] A jedyne dostepne metody to metody standardowych klas.
Dużo wygodniej jest operować jednak na naszych własnych klasach ;) Bo przecież to List<BoardObjects> to jest coś co ma sens "biznesowy" w tej aplikacji. Może jednak warto objąć to w jakąś klasę która jasno określa co to jest? ;) Co więcej pozwala to też na ładne ukrycie implementacji i metod których użytkownik nie powinien widzieć i używać ;)

fasadin napisał(a):

czyli uzywanie zmiennych w ramach tej samej klasy jest zle? Trzeba uzywac proprerty zamiast tego? Ma to po czesci sens (jezeli za kazdym razem zmienna musi miec jakas procedure przypisania), ale co jesli wewnatrz klasy trzeba miec inny sposob przypisywania? To wtedy usuwamy readonly? Bo w takim razie czesc bedzie readonly a czesc nie... troszke dziwnie

chyba nie do konca cie zrozumialam. w kazdym razie - nic nie trzeba, jednak jesli nie przewidujesz zmieniania pola/property to powinno byc readonly.
jesli pole jest prywatnie modyfikowane przez klase to oczywiscie nie moze byc readonly.
jesli pole jest modyfikowane przez klase a na zewnatrz jest do odczytu to mozesz zrobic property:

 public Typ NazwaPola { get; private set; }