Ocena pierwszego, małego projektu

pytanie zadane 17 października 2017 w C i C++ przez Czarus0 Obywatel (1,040 p.)

Cześć,

Chciałbym zaprezentować mój pierwszy projekt. Jest to mały program symulujący piłkarską ligę angielską. Prosiłbym o ocenę kodu czy jest on czytelny i o jakieś praktyczne uwagi czego unikać, co zmienić itp.

https://github.com/Czarus0/TableOfMatches

2 odpowiedzi

odpowiedź 17 października 2017 przez criss Mędrzec (172,590 p.)
wybrane 17 października 2017 przez Czarus0

Najlepsza

Ogromna liczba deklaracji friend. To znak, że nie dbasz o hermetyzacje (albo raczej udajesz, że dbasz)
tabl_Bergera.h - w taki sposób każdy #include tego pliku będzie oznaczał po prostu przeklejenie deklaracji tablicy - utworzenie nowej zmiennej. Aby stworzyć zmienną globalną, w headerze zadeklaruj taką zmienną jako extern, a w pliku źródłówym (*.cpp) ją zdefniuj. Słowo extern oznacza deklaracje, że taka zmienna gdzieś w kodzie istnieje, ale zdefiniowana jest gdzie indziej. To daje gwaracje, że tylko jedna taka zmienna będzie istnieć, bo każdy plik źródłowy jest kompilowany tylko raz. Headery w ogóle nie są kompilowane same w sobie - ich zawartość jest zwyczajnie przeklejana dyrektywą #include i jeśli zawarty w nich kod trafi w końcu do pliku źródłowego, to zostaje skompilowany (tak, to oznacza, że kod w headerze najczęściej jest kompilowany więcej niż raz)
quick_sort_points.h i podobne pliki nagłówkowe - nie definiuj funkcji w headerach, spowalniasz proces kompilacji - patrz: ostatnie zdanie powyższego punktu
Club - member `nastepny`. Z góry zakładasz, że Club jest częścią implementacji listy. Nieładnie. Staraj się, żeby klasy były jak najbardziej od siebie niezależne.
Wygoogluj sobie jakąś implementacje listy. U ciebie, jak już wspominałem, część implementacji listy jest w klasie Club. Już nie mówiąc o jakiejkolwiek generyczności. A co jeśli chciałbyś przechowywać co innego? Pisałbyś nowy kontener? Staraj się pisać kod z myślą, że być może się przyda jeszcze gdzie indziej. Myśl do przodu - zaoszczędzisz czas (prędzej czy później - jasne, że sam kontener będziesz pisał nieco dłużej), sporo się nauczysz, a kod będzie bardziej pro. Mógłbyś napisać szablon listy. IMO lista to nawet bardzo dobry sposób na zaczęcie z szablonami (nie wymaga za dużo zachodu, ale nakreśli temat).
List::choose(const int) - zamiast pisać kilku ifów staraj się ogarnąć tak, żeby jedna konstrukcja zamykała wszystkie możliwości. Np. tutaj zamiast aktualnego kodu (wyrzuciłem const z arg.):
```
Club* List::choose(int n) const
{
   Club* szukaj = pierwszy;
   while (n-- > 0)
      szukaj = szukaj->nastepny;
        
   return szukaj;
}
```
List::show_table() - nie mieszaj jakiegokolwiek UI ze "światem wewnętrzym" twojej apki.
List::change_places(int, int) - skorzystaj z tego, że to lista i swap między jej elementami możesz wykonać odpowiednio zamieniając wskaźniki poprzedni/nastepny. Niepotrzebnie kopiujesz cały klub. Przy okazji: tutaj też odbija się to mieszanie listy i klubu - konieczność pisania takiej dziwnej metody copy_parameters.
Przydałby się przenoszący konstruktor i operator przypisania dla listy (możliwe, że jeszcze nie wiesz czym jest to całe przenoszenie - w takim razie możesz ten punkt na razie zignorować, albo doczytać jeśli masz ochotę - raczej pozostanie to dla ciebie ciekawostką na razie)
Wszędzie niepotrzebnie tworzysz puste destruktory. Nawet więcej niż niepotrzebnie - to błąd i to czasem dość poważny. Zadeklarowany w kodzie destruktor (nawet jako =defualt) powstrzymuje kompilator przed zapewnieniem twojej klasie przenoszenia (przenoszący konstruktor i operator przypisania). Czyli w przypadku gdy uruchomienie np. przenoszącego konstruktora byłoby bardzo pożądne, kompilator uruchomi konstruktor kopiujący, a ty nawet tego nie zauważysz. Możesz strasznie stracić na wydajności o tym nie wiedząc.
W ogóle klasy u ciebie mają strasznie dużo memberów. To znak, że dałoby się to podzielić na jeszcze więcej klas. Ale to przyjdzie z czasem - koduj dalej i czytaj kody innych.
main.cpp: nie używaj exit w c++

komentarz 17 października 2017 przez Czarus0 Obywatel (1,040 p.)

Dziękuję za tak rozbudowany feedback :) na pewno dostosuję się do uwag i będę miał je na uwadze w następnych projektach ;)

komentarz 17 października 2017 przez criss Mędrzec (172,590 p.)

Prosze bardzo :) Starałem się znaleźć jak najwięcej rzeczy do przyczepienia się :)

komentarz 17 października 2017 przez Czarus0 Obywatel (1,040 p.)

Właśnie o to mi chodziło ;) dzięki wielkie ;)

odpowiedź 17 października 2017 przez Eryk Andrzejewski Mędrzec (164,260 p.)

Tak na szybko:

Mieszasz język angielski z polskim
Niepotrzebnie tworzysz własne rozwiązania, zamiast używać gotowych. Przykładowo, funkcja do sortowania (bąbelkowego - a więc powolna). Mógłbyś użyć w zamian std::sort(). Do tego na przykład klasa List - czemu nie użyjesz jakiegoś kontenera z STL, przykładowo std::vector?

komentarz 17 października 2017 przez Czarus0 Obywatel (1,040 p.)

1. Masz rację, powinno się trzymać jednego języka :)

2. Rozumiem, czyli nie ma co tworzyć własnych rozwiązań skoro są gotowe? :) po prostu przerabiam krok po kroku książkę Stephana Praty i nie doszedłem jeszcze do takiej wprawy, aby korzystać z dodatkowych bibliotek :)

Dziękuję bardzo za te podpowiedzi :)

komentarz 17 października 2017 przez Eryk Andrzejewski Mędrzec (164,260 p.)

Polecam z całego serca cppreference.com. Co prawda pamiętam, jak dopiero zaczynałem to ciężko było z tego korzystać, ale jak się nabrało trochę wprawy to jest niezastąpione.

Masz tam spis różnych klas/funkcji w określonych bibliotekach, z przykładami ich użycia. Ciężko bez tego funkcjonować.

komentarz 17 października 2017 przez Czarus0 Obywatel (1,040 p.)

Dzięki jeszcze raz za pomoc ;) na pewno będę korzystał :)

komentarz 17 października 2017 przez Eryk Andrzejewski Mędrzec (164,260 p.)

A tak przy okazji, to jak na pierwszy projekt ładnie, ładnie.

Ja bym natomiast osobiście preferował pisanie std::* zamiast stosowania dyrektyw (brr) czy deklaracji using (np. using std::cout).

komentarz 17 października 2017 przez Czarus0 Obywatel (1,040 p.)

Rozumiem, właśnie nie wiem jakie praktyki stosuję się w firmach. Jestem na etapie zmiany branży, od marca chciałbym załapać się na jakiś staż :) dlatego też zależy mi na tym, aby nie przyzwyczaić się do złych praktyk :)