Sprawdzenie konstruktora - java.

Witam, czasami przeglądam to forum, i zainspirowany tym, że ktoś wrzucił do sprawdzenia relacje w bazie danych odnośnie lekarzy, czy przychodni (mniejsza o to), próbuje sobie stworzyć program symulujący, rejestrowanie w przychodni.

Konstruktor klasy Wizyta wygląda tak, i nie jestem zadowolony, w sumie wydaję mi się, że to taki WTF :P

    public Wizyta(int rok, int miesiac, int dzien, int godzina, int minuta) {

        boolean czyPoprawne = sprawdzDane(rok, miesiac, dzien, godzina, minuta);
        if (czyPoprawne) {
            GregorianCalendar GCdata = new GregorianCalendar(rok, miesiac - 1, dzien, godzina, minuta);

            this.data = DateFormat.getDateInstance(DateFormat.FULL).format(GCdata.getTime());

            ///////////////////////////////////////////////////////
            int g = GCdata.get(Calendar.HOUR_OF_DAY);
            this.godzina = Integer.toString(g);
            if (this.godzina.equals("8")) {
                this.godzina = "08";
            } else if (this.godzina.equals("9")) {
                this.godzina = "09";
            }

            /////////////////////////////////////////////////////
            int m = GCdata.get(Calendar.MINUTE);
            this.minuta = Integer.toString(m);
            if (this.minuta.equals("0")) {
                this.minuta = "00";
            }
        } 
        
        else {
            System.out.println("Spróbuj wprowadzić dane ponownie");
        }

    }

Te if-y wyglądaja tragicznie, ale zostały zrobione na potrzeby Collections.sort()

Czy taki kod jest akceptowalny, czy to już WTF ?

Krytyka mile widziana, kto chce wyśmiewać niech się nie udziela.
Pozdro

A nie można by np. tak?

    int g = GCdata.get(Calendar.HOUR_OF_DAY);                    
    this.godzina = String.format("%02d", g);

Nie piszę w Javie ale myślę język nie ma znaczenia.
*

 if (this.godzina.equals("8")) {
    this.godzina = "08";
 }
else if (this.godzina.equals("9")) {
     this.godzina = "09";

Na moje oko to jest źle. Po co zamieniasz '8' na '08". Jeżeli bardzo chcesz to przyjmując że przychodnia pracuje od 6:00 to będziesz musiał zrobić 4 takie if'ów; zastanów się nad funkcją która to zamienia.</del> Nie widziałem tekstu na dole o if'ach.

• Mało komentarzy - przy większych metodach warto pisać komentarze (przypomnienie).
• Niepoprawna nazwa zmiennej 'g', na początku nie wiedziałem od czego ona jest - po chwili się domyśliłem.

boolean czyPoprawne = sprawdzDane(rok, miesiac, dzien, godzina, minuta);
    if (czyPoprawne)

Można zamienić na:

 if (sprawdzDane(rok, miesiac, dzien, godzina, minuta))

chociaż to IMO kwestia gustu.

Przyjdą wyjadacze (w pozytywnym znaczeniu) to powiedzą Ci co jest źle/dobrze dokładniej.

Ku gwoli sprostowania :
Te nieszczęsne ify, są konieczne, gdy ktoś jako godzina wizyty wprowadzi 8 lub 9 (bo zakładam, że od takich godzin pracują przychodnie), to zostanie to przekonwertowane na 08 lub 09.

Dlaczego mi na tym zależy ?
Gdy wywołuje Collection.sort() to string z liczba 8 sortuje za 16, a liczbę 08 już prawidłowo, tzn. nie potrafiłem tego inaczej rozwiązać. Działa, ale nie podoba mnie się to ani trochę (rozwiązanie).

Btw. dzięki wszystkim za odpowiedzi.

Imho, mnóstwo niepotrzebnych pól i mnóstwo niepotrzebnego kodu. Powinno zostać jedno pole

    public Wizyta(int rok, int miesiac, int dzien, int godzina, int minuta) {
 
        if(sprawdzDane(rok, miesiac, dzien, godzina, minuta));{
            this.data = new GregorianCalendar(rok, miesiac - 1, dzien, godzina, minuta);
       } 
    }

@some_ONE sam nie wiem, chyba dlatego, że łatwiej później w compareTo() porównywać. W sumie myślałem, że jeśli pola są prywatne to reprezentacja ich wartości nie ma jakiegoś wielkiego znaczenia. A może po prostu ten kod to crap :P , ale wynika on z tego, że nigdy nie wystawiałem swojego kodu na ocenę (wstydziłem się :D ).

Ty żadnej metody compareTo nie musisz pisać. Pamiętasz datę wizyty jako obiekt typu GregorianCalendar, który już posiada właściwą metodę compareTo.

A może po prostu ten kod to crap

Jestem skłonny się zgodzić.

Tzn. nawet nie wiedziałem, że GregorianCalendar implementuje Compareable, nie mniej imo czasami warto sobie pokodzić coś nawet jeśli jest to nie potrzebne.

Dzięki wszystkim za odpowiedzi, i opinie. Pozdro.

Btw jeśli Java 8, to użyj nowego api dla dat i czasu.

Ja mam pytanie -

 public Wizyta(int rok, int miesiac, int dzien, int godzina, int minuta)

• czemu jeśli chcesz do konstruktora przekazać datę, to zamiast niej przekazujesz jakieś "inty"?