Wątek przeniesiony 2014-12-16 23:46 z Newbie przez furious programming.

Ocena kodu miniprojektu

0

Cześć,
chciałbym prosić o ocenę poprawnośći kodu - szczególnie chodzi mi o klasy, dziedziczenie, widoczność zmiennych etc. czyli tak właściwie wszystko co jest związane programowaniem obiektowym. Dopiero zacząłem się uczyć programowania obiektowego i nie jestem do końca pewien czy wszystko poprawnie napisałem.

Link do źródeł: http://github.com/shinigami7/CompanyManager/tree/master/src

Nie chodzi mi o błędy typu - brak funkcji zwolnij pracownika, czy brak weryfikacji wprowadzanych danych.

Opis projektu:
Proste zarządzania pracownikami z uwzględnieniem sposobu wyliczania pensji.

2

Kod jest dość ok.
Kilka rzeczy do których można się przyczepić:

  1. Nadmiar dołączanych plików w nagłówkach
  2. Switche. Na miłość boską, pozbądź się tych switchy
  3. Nie mieszaj języków (polgielski wypada słabo)
  4. (malutkie) Nie musisz martwić się o czyszczenie rzeczy, które nie są dynamiczne - czyszczenie kontenerów w destruktorze jest zbędne.
0

Dzięki za pomoc, poprawione ;)
Ewentualnie jeszcze chętnie się dowiem odnośnie zastrzeżen co do kodu - co zrobić żeby był bardzo okej

0

Pytanie, jeśli nie switch to co? If też źle wygląda? :D

0

Spacje w nazwach plików to proszenie się o kłopoty!
Z tego co widzę nie rozwiązałeś konfliktu merge i takie coś popchnąłeś do repozytorium!
Zrób ten merge: https://github.com/shinigami7/CompanyManager/commit/a5960db5273eeacab7607e7c2097dd57fc3d136d
jeszcze raz od początku i porozwiązuj konflikty.

1

Nie bardzo rozumiem jakie masz intencje w kodzie. Z jednej strony tworzysz klasę bazową z wirtualnymi metodami, tworzysz pochodne klasy i implementujesz te metody, a z drugiej strony w klasie Company definiujesz składową vector<Employee> employees i tworzysz, poniższe kwiatki:

Employee *hourlyEmployee = new HourlyEmployee(i, n, per_h, hours, d, m, y);
employees.push_back(*hourlyEmployee);
delete hourlyEmployee;

Postępując w ten sposób nie skorzystasz z polimorfizmu :-(

2
  1. merge jest zle wykonany
  2. powinienes podzielic kod na mniejsze funkcje
  3. korzystaj z polimorfizmu, jezeli masz switch case i tworzysz tam roznego employee w zaleznosci od typu uzyj fabryki. A mozesz zrobic sobie takiego "stwora/twora/potwora" i miec jedna klase ktora masz switch-case (przesylasz tam typ) i Ci zwraca odpowiednia fabryke i tworzysz employee.
  4. RAII !
0
MarekR22 napisał(a):

Spacje w nazwach plików to proszenie się o kłopoty!
Z tego co widzę nie rozwiązałeś konfliktu merge i takie coś popchnąłeś do repozytorium!
Zrób ten merge: https://github.com/shinigami7/CompanyManager/commit/a5960db5273eeacab7607e7c2097dd57fc3d136d
jeszcze raz od początku i porozwiązuj konflikty.

Spacje, tak widzę - do tej pory używałem jako IDE geany'ego, wczoraj przesiadłem się na Eclipse i nie zwróciłem uwagi, że utworzył taki plik.
Z gitem jestem troche w plecy - improwizowałem, bo do tej pory jak już to miałem do czynienia z aplikacją okienkową na windowsa, a teraz mam terminal pod linuxem i improwizowałem.

fasadin napisał(a):

(...)
2) powinienes podzielic kod na mniejsze funkcje
3) korzystaj z polimorfizmu, jezeli masz switch case i tworzysz tam roznego employee w zaleznosci od typu uzyj fabryki. A mozesz zrobic sobie takiego "stwora/twora/potwora" i miec jedna klase ktora masz switch-case (przesylasz tam typ) i Ci zwraca odpowiednia fabryke i tworzysz employee.
4) RAII !

  1. Ok, dwójkę rozumiem.
    3,4) z wzorcami jestem w plecy, zaczynam powoli nadrabiać z użyciem "Design Patterns" E. Gramma i linków znalezionych na forum. Tak na boku, zna ktoś może jakiś kurs typu ocw.mit.edu, coursera.org z tego zakresu?
0

Gdy uporasz się z usuwaniem błędnego merge'a zajmij się tym:

Executive(string, string, double, unsigned int, double, unsigned short int, unsigned short int, unsigned int);

Pomocny może być wzorzec "budowniczy" lub po prostu nazwy argumentów.

http://www.spuriousthoughts.com/2012/11/06/chaining-setters-in-c-builder-pattern/

1 użytkowników online, w tym zalogowanych: 0, gości: 1