Witam
Prosił bym o wytknięcie mi błędów w tym kodzie, jest to kod na zaliczenie.
1.Samochod::licznik
nie powinien być dekrementowany przy usuwaniu instancji klasy?
2.stan
powinien być raczej enum
em.
3.Zamiast 4 zmiennych samochod
, lepiej byłoby skorzystać z tablicy.
Reszta jako-tako wygląda dobrze.
Na szybko czepiłbym się kilku rzeczy:
- Zły include, zamiast
cstring
powinno byćstring
. -
using namespace std
na samym początku to nienajlepsza praktyka, lepiej wprost odnosić się do przestrzeni albo dawaćusing
lokalnie w metodach i funkcjach, najlepiej tylko tych elementów, które są wykorzystane (np.using std::cout
). - Metoda
wpisz()
jest dość nieszczęsna. To powinna być osobna funkcja. - Używaj list inicjalizacyjnych konstruktora zamiast jawnego przepisywania pól.
- Mogę się mylić co do założeń, ale w destruktorze powinieneś chyba zmniejszać
licznik
. - Co robi metoda
funkcja()
? I dlaczego nie nazywa się tak, żeby było to wiadome? - Zatrzymanie programu przez
system("pause")
nie jest najlepszą praktyką. W ogóle jak dla mnie zatrzymywanie programów konsolowych mija się z celem. Ale jeśli już musisz to zrobić, to użyj np.cin.get()
. To pozwoli też wywalić#include <cstdlib>
, który do niczego więcej nie jest tu potrzebny. - Lepiej byłoby podzielić to na dwie jednostki kompilacji, czyli stworzyć plik
samochod.h
z deklaracją klasy,samochod.cpp
z definicjami metod orazmain.cpp
jako program główny.
Edit Jak napisał @Patryk27 – stan
pasowałby bardziej jako enum
. Ponadto – czym jest konstruktor konwertujący i jak działa? Jak widzę jest to po prostu wczytanie wartości od usera z konkretną wartością indeksu. Spora niekonsekwencja: albo przez konstruktor podaje się parametry, a klasa dba o wartość indeks
, albo podaje się wartość indeks
, a klasa sama wczytuje dane od użytkownika (fuj). Do tego nie zmieniasz tu wartości licznik
, więc po zrobieniu:
Samochod s1(1);
Samochod s2("Foo", "bar", 123); // s1.indeks == s2.indeks
Postaram się zastosować do waszych uwag, oczywiście jeżeli ktoś ma coś jeszcze do powiedzenia to proszę.
Zacząć należy od tego, że metoda wpisz nie powinna w ogóle wyglądać tak jak wygląda aktualnie. Odnosi się ona jedynie do konsoli, a co jeśli kiedyś przyjdzie czas na przeniesienie do GUI? Będzie trzeba przepisać metodę.
Cała metoda do wyrzucenia.
Powinna ona być poza klasą.
To samo z metodą wypisz. Metoda wypisz powinna zwracać stringa i nazwę powinna mieć "GetInfoCar".
W tej postaci, której jest, klasa ta kompletnie nie nadaje się do niczego. Jest napisana na "tu i teraz", a to bardzo poważny błąd (ja wiem, że na studiach takie cos przejdzie, ale... u mnie by nie przeszło)
Ogólnie bardzo słabo rozumiesz na czym polega obiektowość i klasy. To co ja napisałem ktoś może uznać za "czepianie się", bo "przecież działa", ale prosisz o ocenę kodu, więc go oceniam. Klasa do wyrzucenia, a konkretnie do przepisania.
Co do logiki samych metod to nie wnikam w nie. Są dość krótkie, więc ok.
Widać, że to poziom podstawowy i dopiero zaczynasz, wręcz raczkujesz, dlatego nie zrażaj się. Tak pisze każdy początkujący i to jest absolutnie normalne. Staraj się jednak za każdym razem pisać lepiej.
nie żebym się czepiał, ale po kiego grzyba pisać:
Samochod samochod2=Samochod ("Fiat","zly",1993);
jak można napisać po prostu:
Samochod samochod2("Fiat","zly",1993);
Te intrukcje są równoważne a mniej pisania jest przy tym.
a i używanie system pause w celu zatrzymania programu nie jest najlepszą praktyką
Te instrukcje nie muszą być równoważne. Dodatkowo zapis 2 powinien być IMHO jedynym dozwolonym, by nie było dwuznaczności przy zapisie:
JakasKlasa f();
Co to jest? Utworzenie obiektu f
przy pomocy konstruktora bezparametrowego czy deklaracja funkcji zwracającej JakasKlasa
i nie przyjmującej argumentów.
winerfresh napisał(a):
Te instrukcje nie muszą być równoważne. Dodatkowo zapis 2 powinien być IMHO jedynym dozwolonym, by nie było dwuznaczności przy zapisie:
JakasKlasa f();
Co to jest? Utworzenie obiektu
f
przy pomocy konstruktora bezparametrowego czy deklaracja funkcji zwracającejJakasKlasa
i nie przyjmującej argumentów.
To jest deklaracja funkcji, bo tak mówi standard C++. W skrócie: wszystko co da się zinterpretować jako deklarację funkcji zostanie tak zinterpretowane.
Dwuznaczność widzą tylko ludzie, dla kompilatora to oczywiste. ;-)