• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Java ocena kodu

VPS Starter Arubacloud
0 głosów
1,091 wizyt
pytanie zadane 3 grudnia 2018 w Java przez mn130496 Gaduła (3,530 p.)

Witam 

Jestem studentem informatyki i ostatnio zacząłem naukę programowania w języku Java.Ostatnio na zajęciach otrzymałem zadanie o następującej treści: Stwórz klasę o nazwie Samochod reprezentującą dane pojazdu zawierającą następujące pola:
 marka,
 model,
 pojemnosc,
 typ,
silnik,
 rok produkcji,
 data 1 rejestracji,
 cena.
Klasa powinna zawierać osobne metody służące do ustawiania i zwracania wartości poszczególnych pól obiektów.
 Dodatkowo metodę, która sprawdzi, czy samochód posiada jeszcze gwarancję (wiek pojazdu do 2 lat licząc od daty rejestracji) i zwróci informację w formie komunikatu tekstowego.. Dodaj klasę AutoKomis, w której stworzone będą nowe obiekty klasy Samochod, przechowujące dane o pojazdach. (10 sztuk). Napisałem taki kod programu :

package zadania_lab5;
import java.util.Date;
import java.time.Duration;
import java.time.LocalDate;
import java.time.format.DateTimeFormatter;
public class Samochód {
	//deklaracja zmiennych.
	private String theCarBrand;
	String model;
	double capacity;
	String type;
	int engine;
	double yerOfProduction;
	String dateOfFirstRegistration;
	double price;
	//Stworzenie konstruktora.
	public Samochód(String theCarBrand, String model,  double capacity, String type, int engine, double  yerOfProduction, String dateOfFirstRegistration, 	double price) {
		this.theCarBrand = theCarBrand;
		this.model = model;
		this.capacity = capacity;
		this.type = type;
		this.engine = engine;
		this.yerOfProduction = yerOfProduction;
		this.dateOfFirstRegistration =dateOfFirstRegistration;
		this.price = price;
	}
//	metody zwracające i ustawiające wartości.
	public void settheCarBrand(String t) {
		theCarBrand = t;
	}
	public String gettheCarBrand() {
		return theCarBrand;
	}
	public void setmodel(String m) {
		model = m;
	}
	public String getmodel() {
		return model;
	}
	public void setcapacity(double c) {
		capacity = c;
	}
	public double getcapacity() {
		return capacity;
	}
	public void settype( String ty) {
		type = ty;
	}
	public String gettype() {
		return type;
	}
	public void setengine(int e) {
		engine = e;
	}
	public int getengine() {
		return engine;
	}
	public void setyerOfProduction(double y) {
		yerOfProduction = y;
	}
	public double getyerOfProduction() {
		return yerOfProduction;
	}
	public void setdateOfFirstRegistration(String d) {
		dateOfFirstRegistration = d;
	}
	public String getdateOfFirstRegistration() {
		return dateOfFirstRegistration;
	}
	public void price(double p) {
		price =p;
	}
	public double getprice() {
		return price;
	}
//	Metoda sprawdzająca czy Auto ma gwarancje;
	public void czy_ma_gwarancje(String d)
	{
		LocalDate now = LocalDate.now();
		LocalDate reg = LocalDate.parse(d, DateTimeFormatter.ISO_LOCAL_DATE);

	    Duration diff = Duration.between(reg.atStartOfDay(), now.atStartOfDay());
	 long minione_dni = diff.toDays();
		if(minione_dni<=730)
		{
			System.out.println("Posiada gwarancję.");
		}
		else System.out.println("Nie posiada gwarancji.");
	}

}
package zadania_lab5;
import java.util.ArrayList;
public class AutoKomis {
	public static void main(String[] args) {
//		tworzę tablicę obiektów klasy Samochód.
		ArrayList<Samochód> listCars = new ArrayList<Samochód>();
//		Dodaję obiekty do listy.
		listCars.add(new Samochód (" Toyota " , " jaris " , 1000, "sedan", 65, 2000, " 2017.11.20 ", 3000 ));
		listCars.add(new Samochód ("Mazda"," 3 ", 1560," kombi ", 105,2004," 2006.11.20 ", 4600));
				listCars.add(new Samochód ("Volkswagen","Beetle", 2000, "Hatchback", 60, 2002, "2002.11.20",5499));
				listCars.add(new Samochód ("Nissan", "Almera", 2000, "sedan",  75, 1999, "2001.11.12", 1900 ));
				listCars.add(new Samochód ("Nissan", "Qashqai", 2500, "sedan", 170, 2008, "2017.11.20", 27900 ));
				listCars.add(new Samochód ("Nissan", "Micra", 1240, "sedan", 65, 2004, "2005.11.20", 7000));
				listCars.add(new Samochód ("Mazda", "6", 2000, "sedan",150 ,2009 , "20011.11.11",20000));
				listCars.add(new Samochód ("BMW", " E39",1500 , " kombi",104 , 2005 , " 2005.11.11", 10000));
				listCars.add(new Samochód ("Nissan", "primera", 2000 , " sedan", 150,2003 , "2004.11.11", 8000));
				listCars.add(new Samochód ("volkswagen", " golf", 1500 , " sedan ", 108 , 1999 , "2000.12.12", 1000));
//				Wyświetlenie tablicy obiektów i wywołanie metody sprawdzającej czy auto ma gwarancje.
				for(Samochód x: listCars) {
				System.out.println("Marka samochodu to: " + x.gettheCarBrand());
				System.out.println("Model samochodu to: " + x.getmodel());
				System.out.println("Pojemność samochodu to:" + x.getcapacity() + "cm sześciennych");
				System.out.println("Typ samochodu to: " + x.gettype());
				System.out.println("Moc silnika  samochodu to: " + x.getengine() + " Km");
				System.out.println("Rok produkcji samochodu to: " + x.getyerOfProduction());
				System.out.println("Data pierwszej rejestracji to:" + x.getdateOfFirstRegistration());
				System.out.println("Cena samochodu to: " + x.getprice() + " zł");
				x.czy_ma_gwarancje("2017-11-20");
	}
}
}

Czy ktoś może mi poprawić metodę , która sprawdza czy auto ma gwarancję bo chyba nie działa dokonca dobrze 

oraz proszę o ogólną opinie na temat tego kodu  i wskazanie co ewentualnie zmienić czy poprawić.Z góry dziękuję

komentarz 3 grudnia 2018 przez X3h Dyskutant (9,540 p.)
nie mieszaj angielskiego i polskiego, nie używaj polskich znaków

3 odpowiedzi

+2 głosów
odpowiedź 3 grudnia 2018 przez niezalogowany
Staraj się nie tworzyć konstruktorow z dużą ilością parametrów.
komentarz 3 grudnia 2018 przez Ehlert Ekspert (212,630 p.)
Ogólnie się zgadzam, ale jest to czasem nieuniknione.
komentarz 4 grudnia 2018 przez Aisekai Nałogowiec (42,190 p.)
Nie mogę się z tym zgodzić (przynajmniej nie w całości). O ile w sytuacji, gdy da się wyodrębnić jakieś dodatkowe klasy (i zmniejszyć ilość parametrów konstruktora) np można byłoby zrobić klasę Engine w której będą pola name, capacity, o tyle jeżeli założenia nie pozwalają na wydzielenie innych klas (np w tym zadaniu) to w konstruktorze według mnie powinno sie umieścić parametry odpowiedzialne za ustawienie wszystkich pól, bez których dany obiekt "nie powinien " istnieć.
komentarz 4 grudnia 2018 przez Ehlert Ekspert (212,630 p.)
Tak, oczywiście. W konstruktorze tylko must have.

Ale takie obiekty jak kontrolery aplikacji http często mają zależności od 5, 10 klas serwisów. Bez sensu wyodrębniać z nich klasę bazową bo powstanie Ci potwór którego nie sposób nazwać.
komentarz 4 grudnia 2018 przez niezalogowany
komentarz 4 grudnia 2018 przez Aisekai Nałogowiec (42,190 p.)

@Ehlert, Tak, zdaje sobie z tego sprawę i to też miałem na uwadze. Wyodrębniając klasy mówiłem bardziej o zasadzie SRP lub o klasach jako strukturach danych.

 

W sumie można byłoby użyć wzorca Budowniczy (czy nawet na upartego Fabryki) - o tym nie pomyślałem, ale to chyba z powodu późnej godziny

0 głosów
odpowiedź 4 grudnia 2018 przez izonik Stary wyjadacz (12,560 p.)
    String model;
    double capacity;
    String type;
    int engine;
    double yerOfProduction;
    String dateOfFirstRegistration;
    double price;

Te zmienne powinieneś uczynić prywatnymi.

    public void setdateOfFirstRegistration(String d) {
        dateOfFirstRegistration = d;
    }

Tutaj i w pozostałych metodach `get` i `set` lepiej zastosować nazwę `setDateOfFirstRegistration`. Zapis od razu stanie się czytelniejszy.

Drugą rzeczą do której można by było się przyczepić to użycie `d` zamiast `dateOfFirstRegistration`, ale ja jak najbardziej rozumiem to podejście ponieważ tym wypadku nazwa metody jednoznacznie wskazuje na to czym ma być `d`, więc tego nie musisz zmieniać.

    public void price(double p) {
        price =p;
    }

Tutaj po prostu zapomniałeś o `set` przed nazwą `price`, ale nie przejmuj się zdarza się i najlepszym.

    public void czy_ma_gwarancje(String d)
    {
        LocalDate now = LocalDate.now();
        LocalDate reg = LocalDate.parse(d, DateTimeFormatter.ISO_LOCAL_DATE);
 
        Duration diff = Duration.between(reg.atStartOfDay(), now.atStartOfDay());
     long minione_dni = diff.toDays();
        if(minione_dni<=730)
        {
            System.out.println("Posiada gwarancję.");
        }
        else System.out.println("Nie posiada gwarancji.");
    }
 

Nazwa metody, nawet jeśli polecenie mówi że ma  być po polsku to warto przestrzegać jednego sposobu nazywania w projekcie, czyli lepiej jeśli będzie `czyMaGwarancje`. Druga sprawa to dlaczego ta metoda przyjmuje jakiś parametr ? Ma przecież powiedzieć, czy samochód jest na gwarancji, a wszystkie dane zwarte są w ciele klasy. Zamiast tego `d` po prostu lepiej użyć `this.dateOfFirstRegistration` . Jeśli po zastosowaniu się do powyższego coś ci nie zadziała to możesz zdefiniować własny sposób formatowania tekstu `DateTimeFormatter.ofPattern("yyyy.MM.d")` .  Tutaj trochę poprawiona wersja tej metody: (Zadziała jeśli dateOfFirstRegistration będzie zawierał tylko datę bez spacji, tabulatorów itp )

public void czyMaGwarancje()
{
    DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy.MM.d");

    LocalDate now = LocalDate.now();

    LocalDate dateOfFirstRegistration = LocalDate.parse(this.dateOfFirstRegistration, formatter);

    Duration diff = Duration.between(dateOfFirstRegistration.atStartOfDay(), now.atStartOfDay());
    if(diff.toDays()<=730)
    {
        System.out.println("Posiada gwarancję.");
    }
    else
    {
        System.out.println("Nie posiada gwarancji.");
    }
}

ps. Może łatwiej by było przechowywać datę jako obiekt daty, a nie String ?

0 głosów
odpowiedź 4 grudnia 2018 przez Dev Ox29A Dyskutant (8,400 p.)
Ogólnie za ten kod 3 na zachętę.

Tak jak pisali poprzednicy: polski , angielski w jednej klasie ? nazwy metod są nie odpowiednie, nazwy zmiennych , modyfikatory zmiennych itd zła metoda do obliczania gwarancji:)

Podobne pytania

0 głosów
1 odpowiedź 1,981 wizyt
pytanie zadane 4 grudnia 2018 w Java przez mn130496 Gaduła (3,530 p.)
0 głosów
2 odpowiedzi 953 wizyt
pytanie zadane 12 lutego 2019 w Java przez mn130496 Gaduła (3,530 p.)
+1 głos
0 odpowiedzi 1,698 wizyt
pytanie zadane 16 stycznia 2016 w Java przez Fusion789 Nowicjusz (130 p.)

92,452 zapytań

141,262 odpowiedzi

319,085 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...