• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Code Review. Kalkulator Model-View-Controller

0 głosów
88 wizyt
pytanie zadane 12 września w Java przez invokeLater Początkujący (270 p.)

Siemanko,

Stworzyłem kalkulator stosując wzorzec architektoniczny Model-View-Controller oraz bibliotekę Swing. Chciałbym żebyście zrobili code review, z chęcią i pokorą przyjmę wszystkie rady. Proszę tylko bez wyzwisk. Wszelkie zastrzeżenia odnośnie zastosowania wzorca oraz innych aspektów. Kalkulator miał działać zgodnie z kolejnością wykonywania działań oczywiście, dbając o to by nie wprowadzono np. podwójnego znaku mnożenia. Docelowo ma rozwiązywać równania, ale póki co equation solver jeszcze nieokodowany. Z góry dzięki i nie krzyczcie hah ;)

Tutaj Github: https://github.com/TheRebMon/KalkulatorMVC .

 

1 odpowiedź

+1 głos
odpowiedź 13 września przez pawi125 Pasjonat (17,440 p.)
wybrane 17 września przez invokeLater
 
Najlepsza

Dobra, kod jest całkiem fajny. Bardzo dobrze, że piszesz po angielsku i ze masz odpowiednią strukturę plików. Ja generalnie nie znam się na swingu więc nie wypowiem się na temat użytych komponentów ze swinga ale skupię się na czytelności kodu. 

W całym kodzie brakuje jednolitego formatowania:

- formatowanie kodu (gdzie niegdzie za duzo nowych lini, jakiego IDE uzywasz? - kazdy ma jakis auto formater wbudowany)

klasa ButtonsPanel

- wywal komentarze (po to jest historia na gicie zeby sprawdzac poprzednie wersje)- twoje menu (linia 56+) wyrzuc do innej funkcji

- showUI() i to wszystko co sie dzieje w srodku mozna zrobic za pomoca petli i to bedzie lepeij skalowalne. 

klasa MainPanel

- linia 93 -> if (for(if - nie idzie sie w tym polapac i na pewno da sie to zrobic bez takiego zagniezdzenia. PAMIETAJ if w ifie to nie jest dobra praktyka, a ty tam jeszcze petle dajesz :D

klasa Model

- to samo co wyzej odnosnie for(if(if))) w az 4 miejscach

klasa Wrapper

- to samo co wyzej for(if(if(switch)))) - koniecznie to popraw :)

 

komentarz 13 września przez invokeLater Początkujący (270 p.)

Bardzo dziękuję za komentarz. Daje mi to świeże spojrzenie na kod :) Szczególne jeśli chodzi o te ify, które dla mnie wydawały się dość naturalne, okazały się być nienajlepsze. Poprawię i za jakiś czas zapraszam zerknąć znowu laugh 

komentarz 13 września przez pawi125 Pasjonat (17,440 p.)
Spoko a jakiego IDE uzywasz?
komentarz 13 września przez invokeLater Początkujący (270 p.)

Eclipse Java

Używam auto-formatowania, ale ja stawiam klamry raczej w taki sposób: 

public void someMethod()
{
     //body
}

Eclipse robi to w ten "drugi" sposób, dlatego są takie nieścisłości, bo po prostu zapominam czasami sprzątnąć po sobie autoformatem. Dodatkowe linie natomiast pomagają mi jak chcę sobie coś poprawić w napisanym już kodzie, A uzywanie skrótu jeszcze nie weszło mi w krew smiley

komentarz 13 września przez pawi125 Pasjonat (17,440 p.)

Spoko, polecam jednak uzywanie klamer typu 

funkcja(){
   ...
}

To jest jednak niepisany standard Javy i nigdy nie spotkalem sie z projektem, w ktorym ktos by inaczej pisal. Gdy bedziesz robil projekt komercyjny tez bedziesz musial tak pisac. Z drugiej strony gdy mialem za zadanie napisac cos w jezyku w ktorym klamry byly stawiane inaczej po prostu to akceptowalem i pisalem wedlug zasad danego jezyka, wiec wiem ze przestawienie sie to nie jest problem. 

komentarz 13 września przez invokeLater Początkujący (270 p.)

Mus to mus, rzeczywiście, dlatego, że póki co sam jestem sobie żaglem, sterem i okrętem to nie pomyslalem, że w sumie kiedyś i tak będzie trzeba przestawić się pod grupę :p Dzięki bardzo! laugh

Podobne pytania

+1 głos
3 odpowiedzi 340 wizyt
0 głosów
2 odpowiedzi 160 wizyt
+60 głosów
0 odpowiedzi 99,660 wizyt
Porady nie od parady
Możesz ukryć, zamknąć lub zmodyfikować swoje pytanie, za pomocą przycisków znajdujących się pod nim. Nie krępuj się poprawić pochopnie opublikowanego pytania czy zamknąć go po uzyskaniu satysfakcjonującej odpowiedzi. Umożliwi to zachowanie porządku na forum.Przyciski pytania

67,210 zapytań

114,169 odpowiedzi

241,978 komentarzy

45,754 pasjonatów

Przeglądających: 332
Pasjonatów: 16 Gości: 316

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...