Ocena programu OOP fitness calculator

Question

Hej proszę o ocenę programu, wysztkich mechanik (dziedziczenia, klas abstakcyjnych itd.) oraz nazewnictwa zmiennych funkcji itd. Program ten chce znacznie rozbudować, lecz najpierw chce się dowiedzieć co jest do poprawy.

Zamieszczam link do githuba ponieważ program składa się z kilku plików.

PiotrCiosmak/fitness_and_health_calculators (github.com).

Z góry dziękuje za porady, proszę wytykać wszystko.

Answer 1

Tak na szybko odnośnie klas Tmen i Twomen:

• w obu klasach konstruktor, create, calculateAvgBMI, calculateAvgCalorie, getFirstName, getLastName, saveToFileBMI i saveToFileCalorie  to w zasadzie to samo. Skoro dziedziczysz po Tperson, to wspólny kod powinien się znaleźć w tej klasie.
• Nazewnictwo. To T na początku to przypadłość z C++ Buildera, który odziedziczył to po Delphi (po tym zresztą poznasz builderowca :P). Kiedyś w C++ dodawano C od class, dziś się już tego raczej nie robi, ale jak już musisz, to trzymaj się konwencji CamelCase, czyli TMan, TWoman itd.

Tyle...

Comments

Dzięki

---

@j23, co do pierwszej kropki to w jaki sposób to zrobić, klasa TPerson jest czysto wirtualna więc tam definicji nie mogę umieścić tych funkcji. Tym bardziej, że te funkcje są jednak troszkę różne więc nie mam pojęcia jak to zrobić.

---

klasa TPerson jest czysto wirtualna więc tam definicji nie mogę umieścić tych funkcji.

A to jakiś wymóg, by TPerson była klasą abstrakcyjną?

Fun fact: metody pure virtual mogą mieć implementację.

Tym bardziej, że te funkcje są jednak troszkę różne

Zawartość create i obu konstruktorów jest identyczna. W calculateBMI różnice są w wartościach w tej drabince ifów - może rozważyć tablicę? calculateAvgBMI różni się jedynie nazwą pliku i komunikatem, cała reszta to to samo. Zrób prywatną metodę w TPerson, która przyjmuje nazwą pliku w parametrze i zwraca dwie wartości, wywołuj ją w metodach klas pochodnych.

Generalnie unikaj na ile to możliwe powtórzeń kodu, bo takie powtórzenia to potencjalne powtórzenie błędów. Już nie mówiąc o tym, że kod się przesadnie rozrasta i jest trudniejszy w analizie.

---

Dodam jeszcze to:

Tmenu::~Tmenu()
{
    if (p != nullptr)
    {
        delete p;
        p = nullptr;
    }
}

dużo zbędnych rzeczy. Wystarczy tak:

class Tmenu
{
    ...
private:
    Tperson *p = nullptr;
};

Tmenu::~Tmenu()
{
    delete p;
}

W Tmenu::startMenuSelection wywołujesz create, nawet jeśli użytkownik nie poda właściwej opcji - ergo program się wyłoży. Jeśli ta funkcja ma być wywoływana wielokrotnie, a na to wskazuje treść komunikatu, to powinieneś usunąć poprzedni obiekt przed przypisaniem do p nowego. BTW nazwa p mało opisowa.

---

Dziękuję za rozpisanie się na dużą ilość wątków, ma to duży sens napewno poprawie wszystko