Czy chcesz zareagować na tę wiadomość? Zarejestruj się na forum za pomocą kilku kliknięć lub zaloguj się, aby kontynuować.

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

+4
Fores
Stravi
Cygnus
koszmarek
8 posters

Strona 1 z 2 1, 2  Next

Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Sob Lut 14, 2015 10:07 pm

Otwieram etap 06 produkcji warsztatu nr 17.

Proszę sobie otworzyć ten folder: 
https://drive.google.com/folderview?id=0ByEDbuEU88x4fnpYckxmNVlkMFMwNk5rZ3JROGNIWkhFQnZ0V2JtRkpXdmdkcjRiUVFhemM&usp=sharing

W powyższym katalogu proszę zlokalizować najnowszą wersję głównej aplikacji warsztatu ("1.Odprawa pasażerów na lotnisku") , załadować ją sobie do Visual studio, uruchomić itd.
Proszę pisać w tym wątku wszystkie Wasze uwagi jakościowe, czyli wszystko co Wam wpadnie do głowy:

  • niefortunne praktyki w kodzie w tej aplikacji
  • propozycje na bardziej intuicyjne rozwiązania w UI tej aplikacji
  • itd.

Proszę mieć na uwadze, że aplikacja nie może wykraczać materiałem poza zagadnienia wypunktowane na samym dole tego dokumentu (nie ma mowy aby w tym warsztacie obcować np. z "klasami abstrakcyjnymi" , "interfejsami" czy np. Dao itd.): 
https://docs.google.com/document/d/1u-AvfcWESsbD04oKTQK043xIJLarh3L8hHelFYKq0X0/edit?usp=sharing

Co do drugiej aplikacji we wcześniej wspomnianym folderze  ("2.Inteligentny dom") , to nie zdziwcie się , że jest tam tylko plik .exe tej aplikacji. Tu celowo nie udostępniamy kodu, bo ma to być aplikacja egzaminacyjna (po co Wam psuć zabawę i udostępniać kod przed egzaminem). To oznacza, że:

  1. "1.Odprawa pasażerów na lotnisku" - zgłaszajcie tu proszę uwagi jakościowe dot. kodu oraz UI
  2. "2.Inteligentny dom" - zgłaszajcie tu proszę uwagi jakościowe dot. tylko UI


Wszystkie zgłoszone tu uwagi, w miarę na bieżąco będę wstawiał do tego arkusza:
https://docs.google.com/spreadsheets/d/1AoK5eRK2bzekaqxa-ez8jM4finseVSi70jyK6TNuTrc/edit?usp=sharing
Proszę zgłaszać uwagi ze zrozumieniem i szacunkiem (a przyznaję, że zawsze szanowaliśmy się na forum!). Czyli szacun dla polkom21 i Cygnus głównie za to, że podjęli wysiłek napisania tych aplikacji, oraz że mieli odwagę stawić czoła Waszej krytyce.

To czekam na Wasze uwagi jakościowe w tym wątku ! (powodzenia)


Ostatnio zmieniony przez koszmarek dnia Pią Mar 06, 2015 12:50 pm, w całości zmieniany 1 raz
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Cygnus Nie Lut 15, 2015 9:35 am


  1. Nie wiem jakie założenia są do obliczania wieku średniego pasażerów w samolocie, ale jeżeli ma liczyć wiek średni tylko osób odprawionych to na liście samolotów jest źle liczony. Domyślnie dzieli wiek przez 50 pasażerów, a nie przez ilość odprawionych osób.
  2. Z tego co wiem nie można było używać konstruktorów, a tutaj zostały użyte
  3. W niektórych przypadkach użyłbym pętli foreach zamiast pętli for np:

    Metoda Fight
    Zamiast:
    Kod:
          

                for (int i = 0; i < passengers.Length; i++)
                {
                passengers[i].Flight();
                }

    Użyłbym:
    Kod:

               foreach (Passenger passenger in passengers)
                {
                passenger.Flight();
                }
    To samo się tyczy innych przypadków odczytania elementów z tablicy obiektów
  4. Jest kilka linijek kodu, który nie jest używany a jest zakomentowany to należałoby go usunąć
Cygnus
Cygnus

Liczba postów : 27
Join date : 05/01/2014

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Stravi Nie Lut 15, 2015 10:37 am

Używanie this.zmienna nie jest stosowane w przy każdym użyciu zmiennych klasy. Np. dismissedPassengers, passengers.
Zmienne publiczne powinny być pisane z dużej litery.

Tak jak kolega powyżej wspomniał bardzo często można skorzystać z foreach dzięki czemu kod jest bardziej przejrzysty i łatwiejszy w analizie.

Stravi
Lider Rozwoju Oprogramowania "Madagaskaru"
Lider Rozwoju Oprogramowania

Liczba postów : 92
Join date : 01/03/2014
Age : 34
Skąd : Gdańsk

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Cygnus Nie Lut 15, 2015 10:55 am

Stravi napisał:Używanie this.zmienna nie jest stosowane w przy każdym użyciu zmiennych klasy. Np. dismissedPassengers, passengers.
Zmienne publiczne powinny być pisane z dużej litery.

Tak jak kolega powyżej wspomniał bardzo często można skorzystać z foreach dzięki czemu kod jest bardziej przejrzysty i łatwiejszy w analizie.

Co do zmiennych publicznych to spotkałem się z różnymi praktykami, ale nigdy żeby były pisane z dużej litery. Spotkałem się że zmienna globalna ma przedrostek g (gZmienna, g_Zmienna) albo z podkreśleniem (_Zmienna). Sądzę, że co do standardu nazewnictwa to jest kwestia umowna.
Cygnus
Cygnus

Liczba postów : 27
Join date : 05/01/2014

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Fores Nie Lut 15, 2015 3:29 pm

Nie powinno się używać czegoś takiego jak zmienna publiczna. Jeśli tutaj nie możemy używać właściwości to powinien pozostać zapis z dużej litery. Wszelkie przedrostki oczywiście są akceptowalne, jeśli tylko cały zespół używa takich samych.

Ja się spotykam oraz używam takiej konwencji:
- zmienne prywatne zaczynające się od "_", na przykład: _customer (pascal case)
- właściwości zaczynające się od dużej litery (camel case)
- metody zaczynające się od dużej litery (camel case)

Co do błędów jakie ja zauważyłem:

1. Nie akceptowalny sposób nazewnictwa metod. Tutaj bezwzględnie powinny być według camel case. Nie spotkałem się w C#, aby ktokolwiek pisał je z małej litery. Samą konwencje nazewnictwa pozostałych elementów zmieniłbym na taką jaką opisałem wyżej, jest ona przyjęta prze większość prze co każdemu dobrze się czyta taki kod.
2. Brak modyfikatorów dostępu przy zmiennych i klasach. Kod nie jest przejrzysty.
3. Coraz częściej zauważam, że programiści rezygnują z użycia słowa kluczowego 'this', ponieważ jest ono zbędne. Nazywając zmienne prywatne od "_" potrafimy je rozróżnić od zmiennych lokalnych. Sam osobiście używam tego słowa w bardzo skrajnych przypadkach.
4. Nazewnictwo kontrolek na formularzu - przyjętą zasadą jest, że nazwa powinna zawierać informację również o typie kontrolki. Na przykład:
Zamiast airportAverageage, powinno być lblAirportAverageAge lub coś podobnego.

A teraz dokładniej:

Klasa Passenger:
Kod:

            Random r = new Random((int)DateTime.Now.Ticks & 0x0000FFFF);
            this.age = r.Next(6, 80);
            this.passengerWeight = Math.Round(r.NextDouble() * (180 - 30) + 30, 2);
            this.luggageWeight = Math.Round(r.NextDouble() * 30, 2);

Masa 'magic numbers', numery te zapisałbym do stałych, które będą mówić co one oznaczają. Ja czytając aktualnie ten kod tego nie wiem i nie chcę zgadywać.

Klasa Plane:

Kod:

public void Flight()
        {
            if ((float)dismissedPassengers / (float)passengers.Length >= 0.75)

Kolejny magiczny numerek oraz niepotrzebne rzutowanie obu zmiennych na float (wystarczy rzutować jedną i wynik będzie jako float).

Klasa Airport:

Kod:
public float CalculateAverageAge()
...
return sum / passengers;

Możliwa utrata danych, brakuje rzutowania na float.
Fores
Fores

Liczba postów : 73
Join date : 30/05/2013
Age : 33
Skąd : Katowice

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by polkom21 Nie Lut 15, 2015 7:53 pm

Jak na razie wydaje mi się, że wszystko poprawiłem. Czekam na dalsze uwagi. Nowa wersja na dysku google.

polkom21
Egzaminator Warsztatów "Madagaskaru"
Egzaminator Warsztatów

Liczba postów : 15
Join date : 14/10/2013

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Nie Lut 15, 2015 8:00 pm

Zgadzam się z Cygnusem i Foresem, że nie ma (również wg mnie) reguły nazywania zmiennych publicznych z dużej litery.  Również uważam, że powinny być nazywane z małej litery (jak pozostałe zmienne) i pisane notacją wielbłądzią.

Zgadzam się również z tym, że nie powinno się w ogóle używać zmiennych publicznych w kodzie (powinny być zastępowane właściwościami) ale plan warsztatu (dogadany na etapie 02) narzuca ograniczenie - nie stosować w kodzie warsztatu 17 właściwości (Fores przyznał w swoim wpisie że mniej lub bardziej zdaje sobie również z tego sprawę) ... co w naszym przypadku wymusza użycie zmiennych publicznych (będzie to wyjaśnione w materiałach warsztatu)

Potwierdzam to co napisał Cygnus - kolejne ograniczenie w w17: nie stosujemy konstruktorów. Wszystkie znaki na niebie i ziemi wskazują że idealnym materiałem na kolejny warsztat (nr 18) będzie wprowadzenie do kodu tych samych aplikacji zarówno właściwości jak i konstruktorów. Nie chcę tego jednak robić w pierwszym warsztacie wprowadzającym do OOP (czyli w aktualnie produkowanym przez nas w17).

Generalnie z 95% Waszych uwag się zgadzam (w przypadku spornych kwestii opowiedziałem się po jednej ze stron powyżej :p ) ... polkoma21 czeka teraz trochę roboty przy wdrażaniu poprawek (ale to normalne , taki urok tego etapu !) , oczywiście polkom możesz bronić na forum swoich racji (jak się nie zgadzasz z którąś z uwag).

Wprowadziłem wstępnie wszystkie uwagi do arkusza:
https://docs.google.com/spreadsheets/d/10fbwzh6hFnInbfJSB7gkx9kIgZlzdZBFFZVG_F-CFHQ/edit?usp=sharing

czekam na kolejne !


Ostatnio zmieniony przez koszmarek dnia Pon Lut 16, 2015 7:40 am, w całości zmieniany 3 razy
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Nie Lut 15, 2015 8:02 pm

Sorry za mój wpis trochę nieaktualny względem wpisu polkoma (był chłopak szybszy ode mnie o kilka min Very Happy )
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Stravi Pon Lut 16, 2015 8:38 am

Wersja5:

  • Używanie this. nie jest zbędne przy większych projektach może odbić się to sporą czkawką, jeżeli ktoś stworzy zmienna o takiej samej nazwie w swojej klasie. Wystarczy poszperać w sieci o powodach dlaczego warto używać this. Decyzja czy powinno być zastosowane należy do wszystkich w projekcie, ale już na pewno trzeba się stosować do tego konsekwentnie, a nie raz z this. a raz bez.
  • W klasie Plane nadal jest magiczna liczba 0.75
  • Jeżeli nie stosujemy konstruktorów to ten jest do wyrzucenia public Plane(int id, int passengers)
  • Klasa Passenger składa się z wielu magicznych liczb
  • Thread.Sleep(20); - Zło które nie ma komentarza dlaczego taki sleep jest wykonany
  • foreach(Passenger passenger in plane.Passengers)
    {
    if (passenger.Age > 0) passengers += 1;
    }
    Proszę o dwie dodatkowe klamry. Dwie dodatkowe linie nikogo nie zbawią, a czytającym kod ułatwi analizę.

Stravi
Lider Rozwoju Oprogramowania "Madagaskaru"
Lider Rozwoju Oprogramowania

Liczba postów : 92
Join date : 01/03/2014
Age : 34
Skąd : Gdańsk

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Pon Lut 16, 2015 9:30 am

Dobra, to teraz ja wkraczam z moimi uwagami (do  najnowszej wersji aplikacji):

  • nazewnictwo modułów aplikacji: ponieważ (jak wszyscy już wiemy) wszystkie elementy aplikacji powinny nazywać się zgodnie z ich przeznaczeniem to uważam, że nazwa rozwiązania (Solution) nie powinna nazywać się "Warsztat 17" a powinna się nazywać np. AirportControl
  • analogicznie nazwa projektu nie powinna nazywać się "Warsztat 17" a powinna się nazywać np. WinFormsAirportControl (aby informowała czym jest aplikacja oraz że jest aplikację Windows Forms)
  • na etapie tworzenia opracowania (pdf) warsztatu zacznę od omawiania tworzenia klasy Passenger oraz jej obiektów na konsoli (bo konsola idealnie nadaje się do prezentowania uwydatniania podstawowych cech OOP). Potem chcę się przenieść tą samą (w pełni oprogramowaną) klasą Passenger (może nawet też klasą Plane) do aplikacji WinForms. Będzie bez sensu robić copy and paste kodu tych klas z aplikacji konsolowej do formatkowej (przerabialiśmy to już w warsztacie 15 i 16). Dlatego uważam, że klasy obsługujące (nazwijmy to górnolotnie) model obsługi lotniska nie powinny być wstawione bezpośrednio (osobno) do aplikacji Winforms (aplikacja WinForms na ile się da ma zajmować się tylko prezentacją tego modelu). Na razie bez zaawansowanych technik / wzorców projektowych (wspieranych użyciem np. interfejsów czy klas abstrakcyjnych) musimy (na ile potrafimy na tym etapie szkolenia) oddzielać model od prezentacji., czyli uważam , że klasy Passenger, Plane oraz Airport powinny być wydzielone do dedykowanej biblioteki (o nazwie np. "LibAirportConstrol") po to aby można było się po prostu podpinać do tej biblioteki (przy generowaniu obiektów tych klas) zarówno z aplikacji konsolowej, okienkowej, webowej, unity 3d whatever. ... będziemy tym samy kontynuować podejście z warsztatów 15 oraz 16
  • klasa Passenger: wiem że sugerowałem w specyfikacji założyć zmienną PlaneId, ale teraz z niej jednak bym zrezygnował bo w aktualnej (okrojonej) aplikacji nie jest nigdzie używana (o ile się nie mylę)
  • wszystkie klasy: zrezygnować z konstruktorów ! - temat poza dyskusją (ograniczenie wynikające z planu szkolenia) - ja wiem polkom że Ci to Ci trochę teraz zbrzydzi :p kod, ale nie mam wyboru. Niemal każde opracowanie wprowadzające w OOP nie omawia od razu konstruktorów (bo wiąże się z z tym np. omawianie np. przeciążania konstruktorów i materiał takiej laborki się mocno rozrasta). Jak nie wiesz jak to ugryźć (uwstecznienie technologiczne Twojej aplikacji) do pogadaj z Cygnusem jak to zrobił u siebie. Ja bym zrobił (na poziomie być może na razie aplikacji WinForms) metodę (poza klasami) która najpierw założy obiekt klasy Airport. A potem (linijkę niżej) w zagnieżdżonej pętli pozakłada 5 obiektów klasy Plane a dla każdego samolotu (w pętli wewnętrznej) pozakłada po 50 obiektów klasy Passenger
  • Passenger.CheckIn() - po co ten Thread.Sleep(20); ??? Zrezygnowałbym z tego .
  • klasa Passenger: mnie jestem pewien tej uwagi ! - zastanawiam się czy metoda Flight akurat w tej klasie ma odzwierciedlenie biznesowe (zgodne z realem). Staramy się, żeby OOP odzwierciedlało (na ile to możliwe) rzeczywistość. Stwierdzenie, że pasażer odlatuje (jakby samodzielnie) trochę mi nie pasuje do realu. Ja bym jednak metodę Flight() pozostawił tylko na poziomie klasy Plane i zrezygnowałbym z metody Passenger.Flight() bo  właśnie z poziomu samolotu mają być wg mnie zmieniane (na razie) zmienne (a w kolejnej laborce właściwości) klasy Passenger: zerowana ma być waga każdego pasażeta, oraz dismissed = true itd.) ...ale może tu nie mam racji, przemyśl to !
  • klasa Passenger: brakuje metody Passenger.CalculateWeight . W przeciwieństwie do metody Passenger.Flight() to właśnie metoda o nazwie np. Passenger.CalculateSumWeight wg mnie ma sens ... bo jest wg mnie zgodne z logiką biznesową wyliczenie całego obciążenia w kilogramach jakie generuje pojedynczy pasażer
  • wszystkie klasy: notacja: przypominam, że zmienne publiczne wg mnie powinny nazywać się z małej a nie z dużej litery.  (ustalenie moje, Foresa oraz bodajże Cygnusa vs ustalenie Straviego) 3:1 :p
  • nadal są "magic numbers" o których wspomina Fores
  • klasa Plane.CalculateWeight() - tu bym w pętli foreach uruchomił po prostu metodę Passenger.CalculateSumWeight, ktorej na razie nie ma a której wspominałem powyżej
  • Plane.CalculateAverageAge niekonsekwencja w typach danych. Ta metoda na poziomie klasy Airport zwraca float a ta sama metoda w Plane zwraca double.
  • Plane.DismissedPassengers - wystarczy wg mnie tu w pętli (dla każdego samolotu) zsumować wartości zmiennych Plane.dismissedPassengers
  • klasa Airport - dorobiłbym metodę sumarycznej wagi wszystkich pasażerów i ich bagażu dla całego lotniska (wywołującej tą samą metodą dla każdego samolotu w pętli) po to aby można było ją bardzo łatwo zaprezentować na formatce (w statystykach całego lotniska) ... tym bardziej bym to zrobił, żeby zachować analogię względem istniejącej już metody Airport.CalculateAverageAge(). Poza tym ułoży nam się fajna piramidka :p (po kolei wywoływanych 3 metod) CalculateWeight dla wszystkich 3 klas : Passenger, Plane oraz Airport ... na szkolenie fajne.
  • formatka: zmieniliśmy koncept (zrezygnowaliśmy z drzewa) - dlatego nazwa metody createTree() jest niefortunna
  • formatka: również jej notacja metod używasz javowej nie c#, + metody mają się nazywać z dużej litery
  • do statystyk lotniska dodałbym info o łącznej wadze pasażerów i bagażu
  • ciągle uważam że odpalanie konkretnych funkcjonalności z menu górnego jest nieintuicyjne (menu górne służy do nawigowania po aplikacji, odpalania takich opcji jak "settings", "help" , "about" itd.) Tu użyłbym uniwersalnego klawisza (kontrolki Button) (który zmienia swoją treść (a może nawet ikonkę - będzie bardziej cool ;p) na "Flight" , "CheckIn" w zależności od zaznaczonego elementu). Ale dobra nie upieram się (zawsze jest argument, że nauczymy uczestnika kontrolki tego menu) ... przemyśl to jeszcze !


Jeśli gdzieś piszę bzdury, to dyskutujcie ze mną !

Polkom21, traktuj proszę arkusz z uwagami jako listę kontrolną (masz tam jeszcze sporo rzeczy nie wdrożonych z poprzednich uwag uczestników):
https://docs.google.com/spreadsheets/d/10fbwzh6hFnInbfJSB7gkx9kIgZlzdZBFFZVG_F-CFHQ/edit?usp=sharing
.... wtedy jak najmniej Ci umknie i czeka nas jak najmniej rund dodatkowych poprawek !
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Stravi Pon Lut 16, 2015 9:44 am

koszmarek napisał:
wszystkie klasy: notacja: przypominam, że zmienne publiczne wg mnie powinny nazywać się z małej a nie z dużej litery. (ustalenie moje, Foresa oraz bodajże Cygnusa vs ustalenie Straviego) 3:1 :p
Fores napisał tak jak powinno być, czyli właściwości. Jeżeli nie chcemy wdrażać na tym etapie właściwości to zapisał, że powinno być z dużej litery - czyli 2:2 Razz


Ewentualnie, a może i ładniej (jeżeli nie możemy używać właściwości) powinniśmy napisać publiczne metody GetSize, SetSize, itp.

Stravi
Lider Rozwoju Oprogramowania "Madagaskaru"
Lider Rozwoju Oprogramowania

Liczba postów : 92
Join date : 01/03/2014
Age : 34
Skąd : Gdańsk

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Pon Lut 16, 2015 10:15 am

Stravi napisał:
koszmarek napisał:
wszystkie klasy: notacja: przypominam, że zmienne publiczne wg mnie powinny nazywać się z małej a nie z dużej litery.  (ustalenie moje, Foresa oraz bodajże Cygnusa vs ustalenie Straviego) 3:1 :p
Fores napisał tak jak powinno być, czyli właściwości. Jeżeli nie chcemy wdrażać na tym etapie właściwości to zapisał, że powinno być z dużej litery - czyli 2:2 Razz


Ewentualnie, a może i ładniej (jeżeli nie możemy używać właściwości) powinniśmy napisać publiczne metody GetSize, SetSize, itp.
I tak odejdziemy w w18 od publicznych zmiennych. Będziemy używać od w18 właściwości (i te już poza wszelkimi wątpliwościami będziemy pisać z dużej)
Uważam więc, że jeżeli w tym 17-tym warsztacie będziemy kazali ludziom pisać nazwy tych zmiennych z dużej litery, to się zagubią, bo do tej pory zawsze (i słusznie) kazano uczestnikom nazwy zmiennych (jakichkolwiek publicznych i niepublicznych) zaczynać z małej. Kolejnym argumentem przemawiającym za nakazem pisania zmiennych publicznych z małej litery jest to, że zasada ta obowiązywała w warsztatach 15 i 16. (musimy być konsekwentni).

Jak już się uprzecie i będziecie zaczynali te zmienne publiczne z dużej litery to też tragedii nie będzie (niekonsekwencja względe w15 i w16 to nie będzie żaden dramat/katastrofa). Najwyżej opatrzę to dodatkowym komentarzem w warsztacie 17 (w którym wyjaśnię dylematy).
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Fores Pon Lut 16, 2015 11:03 am

Mialem napisana dluga wiadomosc na temat nazewnictwa, ale niefortunnie forum nie cachuje tresci wiadomosci i przez zly click wszystko stracilem, wiec napisze jeszcze raz to samo, ale solidniej Wink

Chcialem rozwinac swoja wypowiedz na temat nazewnictwa zmiennych.

Zaczne od tego, ze programowanie jest latwe, latwo mozna sie nauczyc samego jezyka, ale pisanie dobrze tak, aby nasz kod mogl byc latwy w utrzymaniu, podatny na zmiany, a co najwazniejsze rozumiany przez innych programistow przychodzi dopiero z czasem i doswiadczeniem. Nazewnictwo tutaj odgrywa ogromna role, dobre nazwy zmniejszaja ilosc bledow oraz umozliwiaja przyjemniejszy rozwoj oprogramowania.
Aktualnie mamy sytuacje w ktorej nie mozemy uzyc pewnej funkcji jezyka i myslimy jak rozwiazac ten problem. Chcialbym zaproponowac teraz swoje mozliwe rozwiazania, jakie jako programista bylbym w stanie napisac i zaakceptowac:

1. Uzycie zmiennych publicznych piszac je z duzej litery. To rozwiazanie jest bardzo podobne do standardowej najprostszej wlasciwosci:
Kod:

public Plane Plane {get; set;}
Dziala dokladnie tak samo i w nastepnych warsztatach wystarczy przedstawic temat wlasciwosci dodajac modyfikatory dostepu. I tak jak w przypadku wlasciwosci konwencje nazewnicza mamy dokladnie taka sama i tracimy problem rozroznienia obiektu prywatnego od publicznego.

2. Wszystkie pola, ktore powinny byc przedstawione jako wlasciwosci zapisac jako pola prywatne z metodami operujacymi dostepem do nich. Dla zrozumienia przyklad:
Kod:

private Plane _plane;

public Plane GetPlane()
{
    return _plane;
}

public void SetPlane(Plane plane)
{
    _plane = plane;
}

W tym wyppadku mamy jak najbardziej poprawnie napisany kod zgodny ze standardami. Kod podobny do powyzszego jest i tak automatycznie generowany gdy uzywamy wlasciwosci. Minusem takiego rozwiazania jest utrudnione bindowanie danych, lecz w tym przypadku i tak nie uzywamy tego mechanizmu, wiec to nie przeszkadza.


Nazywanie zmiennych publicznych z malej litery jest zla praktyka, bardzo zla. Wszystko co publiczne powinno byc pisane z duzej litery, wszystko co prywatne powinno byc z malej litery lub od "_". Tak jak wczesniej wspomnialem, ze kwestia nazewnictwa jest umowna i tez nie chce tego negowac, lecz istnieja pewne ogolno przyjete nie pisane zasady do ktorych powinno sie stosowac. Piszmy poprawnie, rynek IT rozrasta sie naprawde szybko i niestety wraz z tym wzrostem pojawia sie coraz wiecej smieciowych aplikacji i beznadziejnych programistow, ktorzy nie potrafia solidnie napisac oprogramowania. Wyrozniajmy sie jakoscia na tle hindusow i chinczykow Wink
I to na tyle z mojej strony. Mam nadzieje, ze znajdzie sie tutaj rowniez miejsce na inzynierie oprogramowania i przedstawienie choc takich podstaw jak SOLID.
Fores
Fores

Liczba postów : 73
Join date : 30/05/2013
Age : 33
Skąd : Katowice

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Pon Lut 16, 2015 11:52 am

@Fores W takim razie w wiadomym arkuszu uwag dodałem Twoją uwage 2 z powyższego w wpisu o konieczności zrezygnowania ze zmiennych publicznych klas , oraz zastąpienia ich zmiennymi prywatnymi z zestawem metod pełniących rolę  "settera" oraz "gettera" wartości tych zmiennych.
Dzięki !

P.S. Dzisiaj (jak widzicie) nawaliłem sporo moich uwag. Dałem w tej chwili w arkuszu te uwagi w status "w analizie" bo prostu czekają na Wasz ewentualny sprzeciw (przeanalizujcie je proszę). Dajcie znać czy trzymają się kupy, długa ciszę będę musiał potraktować jako zaakceptowane.

+ oczywiście zgłaszajcie kolejne Wasze uwagi Very Happy
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Jaro Pon Lut 16, 2015 1:43 pm

Cześć,

1.Odprawa pasażerów na lotnisku.

Moje spojrzenie na aplikację z punktu widzenia osoby uczącej się a nie programisty.

Odnośnie działania aplikacji: pasażera można ponownie odprawić. (po odprawie pasażera, można ponownie kliknąć na niego i odprawić go ponownie, co skutkuje zamianą odprawionego pasażera na nowego a właściwie danych pasażera, bo ID się nie zmienia). Z punktu widzenia realnego świata to nie wygląda dobrze.

A teraz o kodzie:

Form1.cs
private void createTree() – Nazwa nieadekwatna do wykonywanego działania. Proponuję displayPlanes

private void createPassengers() – raczej displayPassengers

private void checkinPassengerToolStripMenuItem_Click(object sender, EventArgs e) – nazwa zagmatwana. Może lepiej menuItemCheckIn_Click

private void flight_Click(object sender, EventArgs e) – nazwa powinna być spójna z powyższą. Proponuję menuItemFlight_Click

private void checkinPassengerToolStripMenuItem_Click(object sender, EventArgs e)
-   airport.Planes[activePlane].DismissedPassengers++; - to chyba nie jest eleganckie rozwiązanie. Metoda CheckIn() dla pasażera powoduje jego odprawę. Ilość odprawionych pasażerów per samolot można znaleźć zliczając odprawionych pasażerów. A jeżeli już potrzebujemy mieć nadmiarową informację, to powinno się to załatwić w metodzie CheckIn(). Unikniemy niebezpieczeństwa, że ktoś w innym miejscu kodu odprawi pasażera a nie zmieni właściwości DismissedPassengers w samolocie.

GUI
Należało by nadać odpowiednie nazwy zamiast domyślnych:
groupBox1
label1
label2
Form1
menuStrip1

Airport.cs
public int DismissedPassengers()
– ta funkcja liczy ilość pasażerów starszych niż 0 lat. Liczenie powinno się odbywać pod warunkiem Dismissed = true
-  int passengers = 0 raczej dismissedPassengers (lepiej opisuje to, co liczymy)

Plane.cs
public double CalculateAverageAge()
- float sum = 0; - proponuję zmiany nazwy na sumOfAge
- int numPassenger = 0; proponuję zmiany nazwy na numPassengers (liczba mnoga)

public double CalculateWeight()
- return Math.Round(weight, 2); - może wagę należy zaokrąglić w górę aby uniknąć przeciążenia samolotu 

CalculateAverageAge() – funkcja mogła by mieć zbliżony wygląd jak dla klasy Airport (nazwy, formatowania)

Passenger.cs
Funkcja Flight – nazwa nie oddaje, tego, co robi ta funkcja

public int PlaneId; - właściwość nie do końca związana z pasażerem (raczej samolot powinien zawierać odnośnik, jakich pasażerów przewozi). Czy właściwość ta jest gdzieś wykorzystywana?

Jaro

Liczba postów : 13
Join date : 24/05/2013

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Stravi Pon Lut 16, 2015 1:50 pm

Jaro pracujesz w walidacji? Smile

Stravi
Lider Rozwoju Oprogramowania "Madagaskaru"
Lider Rozwoju Oprogramowania

Liczba postów : 92
Join date : 01/03/2014
Age : 34
Skąd : Gdańsk

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Jaro Pon Lut 16, 2015 1:59 pm

Aplikacja do sterowania domem.

Nazwy pokojów powinny być pisane spójnym stylem (małe i duże litery, spacje) i najlepiej w języku ludzi a nie kodu  Very Happy

Problem z odświeżaniem danych szczegółowych o urządzeniach przy zmianie pomieszczenia.
Zmiana pomieszczenia skutkuje przeliczeniem pól podsumowań dla pokoju ale nie powoduje zmiany w informacjach o szczegółach pomieszczenia. Po zmianie pokoju powinno być ponownie wybierane urządzenie wg jednego z poniższych schematów:
- pierwsze z listy
- to samo co było wybrane dla poprzedniego pokoju (jeżeli nie ma takiego urządzenia to pierwsze z listy)
- żadne, ale wtedy lista szczegółów urządzenia powinna być czyszczona

Mając aktywną listę urządzeń i wybrane urządzenie nie widać jaki pokój jest aktualnie wybrany (odwrotnie również, ale to mniej przeszkadza).

Pozdrawiam,
Jaro

Jaro

Liczba postów : 13
Join date : 24/05/2013

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Jaro Pon Lut 16, 2015 2:00 pm

Stravi napisał:Jaro pracujesz w walidacji? Smile

Nie, ale mam tą cechę, że dostrzegam różne szczegóły i niekonsekwencje. Czasami to pomaga a czasami jest koszmarem. Smile

Jaro

Liczba postów : 13
Join date : 24/05/2013

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Pon Lut 16, 2015 3:46 pm

Dzięki za dotychczasowo b. intensywne Code Review z Waszej strony Very Happy
(widzę że nasze laboratoria aż się dymią ;p ;p )

Wszystkie świeże uwagi wstawiam do wiadomego arkusza:
https://docs.google.com/spreadsheets/d/10fbwzh6hFnInbfJSB7gkx9kIgZlzdZBFFZVG_F-CFHQ/edit?usp=sharing

... i ustawiam w status "w analizie". Jeżeli po jakimś czasie nikt nie zaneguje tych uwag (etap "kiszenia się" :p), to ustawiam je w status "zaakceptowane" i jest to sygnał dla polkom21 i Cygnusa (autorów swoich aplikacji) , że mają je wdrażać.

Czekam na kolejne uwagi (ew. dyskusje dot. dotychczas zgłoszonych uwag) !
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by zefirek Wto Lut 17, 2015 4:22 pm

STEROWANIEDOMEM.EXE
------------------------
Pierwsze moje spostrzeżenie - coś źle liczy pole Power(W) przy włączaniu i wyłączaniu "devices"
a dokładniej - nie wiadomo w jakim jestem pokoju jak przejdę do tabeli urządzeń i nie wiadomo co to za pole o którym pisze wyżej (może jakiś opis pól)

zefirek
Egzaminator Warsztatów "Madagaskaru"
Egzaminator Warsztatów

Liczba postów : 41
Join date : 27/05/2013
Skąd : Gryfów Śląski

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Cygnus Sro Lut 18, 2015 9:57 pm

1. Power(W) jest to pole które pokazuje pobór danego urządzenia niezależnie od tego czy jest włączone czy wyłączone (pole informacyjne)
2. W Details of device jest pole Name z nazwą urządzenia, którego dotyczą szczegóły, czyli Power i State.

Po za tym program poprawiony i wrzucony w nowej wersji na dysk.
Cygnus
Cygnus

Liczba postów : 27
Join date : 05/01/2014

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by koszmarek Czw Lut 19, 2015 1:42 pm

Super, czekamy na polkoma i odpalam wtedy jeszcze jedną (po wdorżeniu wielu uwag) , wierzę że ostatnią , rundę kontroli.
koszmarek
koszmarek
Lider grupy "Madagaskar"
Lider grupy

Liczba postów : 596
Join date : 25/10/2012

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by polkom21 Pią Lut 20, 2015 5:13 pm

Nowa wersja na dysku google.

polkom21
Egzaminator Warsztatów "Madagaskaru"
Egzaminator Warsztatów

Liczba postów : 15
Join date : 14/10/2013

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Stravi Pią Lut 20, 2015 5:29 pm

Nadal mnóstwo liczb, które nic mi nie mówią analizując ten kod.
Kod:
Age = r.Next(6, 80);
PassengerWeight = Math.Round(r.NextDouble() * (180 - 30) + 30, 2);
LuggageWeight = Math.Round(r.NextDouble() * 30, 2);

Zbędne rzutowanie na double(Math.Round zwraca typ double):
Kod:
return (double)Math.Round(PassengerWeight + LuggageWeight, 2);

Z pierdółek, tego typu if'y można skrócić
Kod:
if (passengers > 0)
            {
                return (double)sum / passengers;
            }
            else
            {
                return 0;
            }
do
Kod:
if (passengers > 0)
            {
                return (double)sum / passengers;
            }

            return 0;

Stravi
Lider Rozwoju Oprogramowania "Madagaskaru"
Lider Rozwoju Oprogramowania

Liczba postów : 92
Join date : 01/03/2014
Age : 34
Skąd : Gdańsk

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Cygnus Sob Lut 21, 2015 12:21 am


  1. W programie w bloku Airport stats jest pole Average age, w którym można ograniczyć ilość cyfr wyświetlanych po przecinku do dwóch tak jak to jest w innych miejscach.
  2. Wszędzie w programie jest rzutowanie na double lub jest zwracany double, a w jednym miejscu jest rzutowanie na float
    Kod:
    public bool Flight()
            {
                if ((float)DismissedPassengers() / Passengers.Length >= 0.75) // Sprawdzenie czy odprawione zostało minimum 75% pasażerów
                {
                    foreach(Passenger passenger in Passengers)
                    {
                        passenger.Flight();
                    }
                    return true;
                }
                else
                {
                    return false;
                }
            }

Cygnus
Cygnus

Liczba postów : 27
Join date : 05/01/2014

Powrót do góry Go down

(zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej Empty Re: (zakończony)Warsztat 17: etap 06: KONTROLA JAKOŚCI kodów aplikacji treningowej oraz egzaminacyjnej

Pisanie by Sponsored content


Sponsored content


Powrót do góry Go down

Strona 1 z 2 1, 2  Next

Powrót do góry

- Similar topics

 
Permissions in this forum:
Nie możesz odpowiadać w tematach