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

Poprawione. Nowa wersja na dysku.

@Polkom jest już na prawdę dobrze !
W tym arkuszu widać, że zostało Ci do poprawienia kilka poprawek na krzyż (w tym jedna duża poprawka od Foresa dot. zastąpienia zmiennych publicznych klas metodami o nazwach Get.. oraz Set... w parze ze zmiennymi prywatnymi z nazwami poprzedzonymi podkreślnikami).
https://docs.google.com/spreadsheets/d/10fbwzh6hFnInbfJSB7gkx9kIgZlzdZBFFZVG_F-CFHQ/edit?usp=sharing

Przejrzyj powyższy arkusz i dokop się pozycji ze statusem "zaakceptowane" (to oznacza , że nie są jeszcze wdrożone przez Ciebie).

Jak to Ci się uda, to zrobimy kolejną rundę uwag, a cały ten etap zakończymy dopiero wtedy jak przez jakiś czas nikt nie zgłosi uwag ...  zbliżasz się to tego stanu , bo kod powoli robi się taki, że nie ma się do czego przyczepić.

To czekam polkom na nową wersję Twojej aplikacji 

Poprawiłem co zostało do poprawienia. Jak by coś jeszcze tam zostało to dawać znać.

P.S. Nowa wersja na dysku google.

@polkom z wersji na wersję coraz bliżej ideału, ale niestety jedziemy z koksem dalej :p

Moje uwagi:

1.
klasa Plane - metoda Flight() - 0.75 - to jest magic number - Twój komentarz w kodzie tym tym bardziej to uwydatnia (Twoją konieczność wyjaśniania w kodzie co ta wartość znaczy)


2.
public bool Flight()
        {
            if ((double)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;
            }
        }
else wg mnie można usunąć


3.
if (passengers > 0)
            {
                return Math.Round(sumOfAge / passengers, 2);
            }
            else
            {
                return 0;
            }
else wg mnie można usunąć


+ to co uznałem za nie wdrożone (w arkuszu kontrolnym) pozostawiłem w status "zaakceptowane". Może się zdarzyć że ktoś z kontrolerów uzna że jego uwaga nie została wdrożona to wtedy również usunę tej uwadze status "wdrożone".

Jeszcze jedna uwaga.

Skoro już nie możemy stosować właściwości, a mamy jednocześnie używać metod Set_ Get_ (jako tymczasowy zastępnik właściwości) dla wybranych zmiennych prywatnych klasy, to musimy zachować konsekwencję i zrobić to dla ostatnich publicznych zmiennych w kodzie, tych tablicowych bodajże public Planes[] w klasie Airport oraz public Passengers[] w klasie Plane (piszę teraz z pamięci bo kod oglądałem wczoraj).

Rozumiem , że problemem dla polkoma jest niemożność użycia konstruktora (ograniczenie programu szkolenia dla tego warsztatu 17).

Proponuję więc:

• uczynić te zmienne tablicowe prywatnymi (nazwa z podkreślnikiem i z małej litery)
  
• założyć metodę np. CreatePlanes() która zainicjuje tablicę samolotów dla tej zmiennej prywatnej
  
• oraz metody Get_ Set_ Plane która w parametrze będzie przyjmować dodatkowo indeks tablicy
  
• analogicznie postąpić dla zmiennej tablicowej dla pasażerów
  

co myślicie ?

Kilka moich uwag idac od poczatku bedac bardzo 'upierdliwym':

Form1.cs
• 
  

  Kod:

          
Airport airport;
int activePlane = 0;


  Wypadaloby dopisac jawnie modyfikator private, poprawia to czytelnosc kodu. Poza tym usunalbym zbedne puste linie (na przyklad miedzy zmiennymi prywatnymi) - kwestia estetyki.
  
  
• 
  Airport airport; - pole moze byc typu readonly
  
  
• 
  Konstruktor klasy FormAirportControl
  
  

  Kod:

  
airport = new Airport();
airport.Name = "Test airport";
airport.Planes = new Plane[5];

  
  Tutaj mozna uzyc object initializera, wyglada czytelniej.
  
  Dodatkowo sama zawartosc konstrutora mozna rozdzielic na 2 osobne metody (pierwsza od inicjalizacji samolotow, druga od wypelnienia kolumn listview pasazerow).
  
  
• 
  Nie rozumiem koncepcji przypisywania numerow id do pasazerow. Pole nigdy nie jest uzywane o czym swiadczy brak wywolan metody GetId w klasie Passenger.
  
  Airport.cs
  
  
• 
  

  Kod:

  return (int)sum / passengers;

  Zbedne rzutowanie na int
  
  
• 
  

  Kod:

  public int DismissedPassengers()

  Zmienilbym na GetDismissedPassengers()
  
  Passenger.cs
  
  
• 
  

  Kod:

  return (double)Math.Round(_passengerWeight + _luggageWeight, 2);


  Zbedne rzutowanie na double
  
  
• 
  Flight() - nic nie mowiaca nazwa
  
  
• 
  GetDismissed - bardziej adekwatna nazwa bedzie IsDismissed()
  
  
  Interfejs - po wykonaniu CheckIn lista automatycznie mi sie odswieza tracac zaznaczenie, a powinna zostac na zaznaczonym rekordzie.
  
  

koszmarek napisał:
Proponuję więc:

• uczynić te zmienne tablicowe prywatnymi (nazwa z podkreślnikiem i z małej litery)
  
• założyć metodę np. CreatePlanes() która zainicjuje tablicę samolotów dla tej zmiennej prywatnej
  
• oraz metody Get_ Set_ Plane która w parametrze będzie przyjmować dodatkowo indeks tablicy
  
• analogicznie postąpić dla zmiennej tablicowej dla pasażerów
  

co myślicie ?

Pomysl jak najbardziej ok. Ale chcialbym zauwazyc, ze kolekcja Plane nie jest uzywana poza klasa i moze byc nawet prywatna. Podobnie pole Name nigdzie nie jest uzyte.

Fores napisał:

koszmarek napisał:
Proponuję więc:

• uczynić te zmienne tablicowe prywatnymi (nazwa z podkreślnikiem i z małej litery)
  
• założyć metodę np. CreatePlanes() która zainicjuje tablicę samolotów dla tej zmiennej prywatnej
  
• oraz metody Get_ Set_ Plane która w parametrze będzie przyjmować dodatkowo indeks tablicy
  
• analogicznie postąpić dla zmiennej tablicowej dla pasażerów
  

co myślicie ?

Pomysl jak najbardziej ok. Ale chcialbym zauwazyc, ze kolekcja Plane nie jest uzywana poza klasa i moze byc nawet prywatna. Podobnie pole Name nigdzie nie jest uzyte.

słusznie !

Fores napisał:

koszmarek napisał:
Proponuję więc:

• uczynić te zmienne tablicowe prywatnymi (nazwa z podkreślnikiem i z małej litery)
  
• założyć metodę np. CreatePlanes() która zainicjuje tablicę samolotów dla tej zmiennej prywatnej
  
• oraz metody Get_ Set_ Plane która w parametrze będzie przyjmować dodatkowo indeks tablicy
  
• analogicznie postąpić dla zmiennej tablicowej dla pasażerów
  

co myślicie ?

Pomysl jak najbardziej ok. Ale chcialbym zauwazyc, ze kolekcja Plane nie jest uzywana poza klasa i moze byc nawet prywatna. Podobnie pole Name nigdzie nie jest uzyte.

A jednak - zarówno kolekcje Planes oras Passengers używane są w kodzie formatki "Form1.cs".

Fores napisał:

• 
  Nie rozumiem koncepcji przypisywania numerow id do pasazerow. Pole nigdy nie jest uzywane o czym swiadczy brak wywolan metody GetId w klasie Passenger.
  
  

Tu podobnie , identyfikator pasażera jest używany (prezentowany) na formatce Form1.cs. , taki jakby zastępnik wyświetlania jego imienia i nazwiska

Moja uwaga - zmienić nazwę Form1.cs na bardziej oddającą przeznaczenie formatki czyli zamiast tajemniecze Form1.cs zmienić nazwę na np. WinFormsAirportControl.cs czy coś w tym stylu

Form1.cs - Thread.Sleep(20);  - usunąć .

Poprawki uwzględnione w kodzie. Nowa wersja na dysku google.

Kończymy ten mozolny etap

Oczywiście będę nanosił jeszcze drobne poprawki w tym kodzie (w trakcie realizacji kolejnego etapu  - tworzenia finalnego opracowania dla laborki) - ale moje poprawki w kodzie będę na bieżąco konsultował na forum.

Wielkie podziękowania dla autorów aplikacji oraz naszych "code reviewerów" !!
Blokuję ten wątek do dalszej dyskusji.