Setter wyrzucający wyjątek i prywatny konstruktor w klasie.

Natknąłem się na publikację dot. Javy i jest tam rozdział opisujący sposoby przechwytywania wyjątków.
Przykład dobrany przez autora mnie zaintrygował. Osobiście nie programuję w Javie i nie wiem na ile te przykłady są zgodne z dobrymi praktykami.

Co mnie uderza:

1. Wyrzucanie wyjątku w setterze.
2. Tworzenie prywatnych i publicznych konstruktorów w klasie. Jakie są typowe zastosowanie tego podejścia?

public class User {
    
    private String username;
    private String password;

    public User(){
            this("username", "password", false);
    }

    private User(String username, String password, boolean b) {
        // only call this because we don't want to throw the exception
        this.username = username;
        try{
            setPassword(password);
        }catch(InvalidPassword e){
            throw new IllegalArgumentException(
                            "Default password incorrect ", e);
        }
    }

    public User(String username, String password) 
                                        throws InvalidPassword{
        this.username = username;
        setPassword(password);
    }

    public void setPassword(String password) throws InvalidPassword {
      if(password.length()<7){
         throw new InvalidPassword("Password must be > 6 chars");
      }

      this.password = password;
    }

    public String getUsername() {
        return username;
    }

    public String getPassword() {
        return password;
    }

}

Setter wyrzucający wyjątek 1 raz widzę, ale na przykład prywatny konstruktor to nic niezwykłego, chroni przed możliwością utworzenia instancji z użyciem konstuktora

Ten kod to (mała) katastrofa designowa.
Chyba najlepsze jest "boolean b" w drugim konstruktorze.
Ale konstruktor domyślny też niczego sobie (WTF ?).

Generalnie w praktyce w kodzie jak wyżej nie powinno się używac checked exceptions. ( Właściwie w ogóle nie powinno się używac checked exceptions//www.artima.com/intv/handcuffs.html).

setPassword mogłby rzucać IllegalArgumentException ewentualnie i mieć jakąś asercje.

Dlaczego:
W praktyce i tak obsługa wyjątku będzie na jakiejś walidacji pola formularza (trzeba przecież Userowi dokładnie powiedzieć co zrobił nie tak)... więc do tego setera trafi już zwalidowana wartość. Co najwyżej można zrobić dodatkowe zabezpieczenie - na wypadek kiedy coś dziwnego dzieje się z systemem i do setera trafia niezwalidowana wartość.
(Ponieważ jest to coś dziwnego z systemem - więc rzucamy RuntimeException (IllegalArgument) - bo i tak normalnie nie powinno się coś takiego stać.)

No konstruktor domyślny istnieje po to, aby każdy kto chce stworzyć sobie egzemplarz obiektu User z zahardkodowanym hasłem nie musiał obsługiwać wyjątku. Teraz uproszczony przykład. Jak teraz?

public class User {
    
    private String username;
    private String password;

    public User() {
        this("username", "password");
    }

    public User(String username, String password) {
        this.username = username;
        setPassword(password);
    }

    public void setPassword(String password) {

        if (password.length() < 7) {
            throw new IllegalArgumentException("Password must be > 6 chars");
        }

        this.password = password;
    }

    public String getUsername() {
        return username;
    }

    public String getPassword() {
        return password;
    }

}

A teraz pomysł. Jakby w ogóle zrezygnować z setterów i stworzyć taki immutable object. A wyjątek o niepoprawnym haśle dać do konstruktora? Co o tym myślicie?

Wychodzę z komentarzy @barslo

Nie spotkałem się z twierdzeniem, aby konstruktor wyrzucający wyjątek był jakimś anti-pattern. Co więcej pierwsze lepsze stackoverflowy[0] potwierdzają to spostrzeżenie.

Proszę Cię abyś wzniósł się na wyżyny edukacyjnych możliwości i wytłumaczył mi - podpierając się przykładem - swoje racje. Jakbym Cię o to zapytał na rozmowie kwalifikacyjnej to również byś mi odpowiedział "wystarczy się zastanowić"?

[0] http://stackoverflow.com/questions/6086334/is-it-good-practice-to-make-the-constructor-throw-an-exception

no dobrze a czy konstruktor rzucający wyjątkiem nie oznacza że jest on źle napisany? gdyż tworzy niepoprawnie obiekt?

Np. Jakieś pole będzie niezainizjalizowane, a jakaś metoda z niego korzysta. Bardziej mam na myśli to że ktoś kto nie pisał tego kodu może się po prostu nie spodziewać, że coś z jego obiektem jest nie tak i działać na nim tak jak by się nic nie stało.
Napisałem, że nie powinien, a nie że nie może.

@rafal20-1988: Taka sytuacja - mamy kod z jakiejś tam klasy, który potrzebuje np. imienia w formie Stringa, do konkretnej operacji, która się wysypie, jeśli go nie będzie (czyli to co powiedział @barslo )
Z takim kodem niby jest wszystko ok.

public class ExampleClass {
	
	private String firstName;

	public ExampleClass(String firstName) {
		this.firstName = firstName;
	}

Ale co jak firstName będzie nullem?
Wtedy wszystko szlag trafił, a jak jeszcze nie masz czasu/chęci/szczęścia przy debugowaniu to szukaj wiatru w polu.
Natomiast taki kod poniżej, powoduje, że od razu wiemy co w trawie piszczy.

public class ExampleClass {
	
	private String firstName;

	public ExampleClass(String firstName) {
		if (firstName != null)
			this.firstName = firstName;
		else
			throw new NullPointerException("A gdzie imię? Nie zapomniałeś czegoś wpisać?");
	}

Dodatkowo IDE samo wychwyci przy pisaniu kodu, że tutaj kolego jest coś nie halo.

Jak kogoś razi wyjątek w konstruktorze to robi taki myk i już

public class ExampleClass {
	
	private String firstName;

	public ExampleClass(String firstName) {
		setFirstName(firstName);
	}

	void setFirstName(String firstName) {
		if (firstName != null)
			this.firstName = firstName;
		else
			throw new NullPointerException("A gdzie imię? Nie zapomniałeś czegoś wpisać?");
	}

Przynajmniej ja tak robię. Jestem początkującym, więc niech mnie ktoś bardziej doświadczony i mądrzejszy poprawi.

@Burdzi0: no mnie jakoś Twój argument nie przekonuje bo:

• pisząc np. GUI, które ma pobierać np. Imię od użytkownika to kontrolę tego co podał tworzysz np. w metodzie obsługującej akcję np. Button-a, a nie dopiero w konstruktorze klasy czyż nie?

@rafal20-1988: To tylko przykład, gdzie napisałem, że: potrzebuje np. imienia w formie Stringa, do konkretnej operacji, która się wysypie, jeśli go nie będzie.
I nie, nie zgadzam się.
Klasa obsługująca Button powinna sprawdzać osobno czy dany input nie jest pusty i czy wszystko jest z nim ok.
[Moja opinia] Kod powinien być na tyle uniwersalny, że jak wywalę GUI i napiszę naprędce jakiś kod bez UI, to będę miał pewność, że jak coś jest nie tak jak być powinno to wywali mi wyjątek (czytaj: zostanę o tym powiadomiony wystarczająco wcześnie, żeby nie grzebać debuggerem przez najbliższy tydzień) [Moja opinia]

Na takie rzeczy jak wyżej są asercje:

  void setFirstName(String firstName) {
     assert firstName !=null: "gdzie z tym nullem;
            this.firstName = firstName;
     
    }

albo Guava

  void setFirstName(String firstName) {
                this.firstName =  Preconditions.checkNotNull(firstName, "ale  wstyd - null"); //he - dopiero dziś to odkryłem, że działa onliner jak w  java 8 -  pisałem w osobnej linii
    }

albo Java 8

  void setFirstName(String firstName) {

            this.firstName = Objects.requireNonNull(firstName, "no nie... znowu null");;
     
    }

Ten kawałek kodu to ciekawe pomieszanie obiektu domenowego z DTO.

1. Z jednej strony walidowane jest hasło, ale nie jest walidowana nazwa użytkownika.
2. Klasa nie ma żadnych metod oprócz setterów i getterów.

Jeśli o mnie chodzi to można - tutaj do wyboru:

1. Wywalić ten wyjątek.
2. Wywalić ten wyjątek, settery i dorzucić dwie metody: isPasswordValid oraz isUsernameValid.
3. Wywalić settery, dorzucić dwa wyjątki w konstruktorze (usernameNotValidException, passwordNotValidException).

Wszystko zależy od przeznaczenia klasy, tyle tylko że nie mamy opcji sprawdzić, jakie to przeznaczenie jest.

Czy mozna z góry powiedziec ze zawsze lepiej jest wywalic settery i zrobic assercje w konstruktorze? (zakladajac ze settery nie sa potrzebne w innym miejscu kodu)

No jak nie potrzebujesz setterów to na pewno warto je wywalić - chyba, że potrzebne Ci do muzeum starego kodu.
A w konstruktorze asercje jak najbardziej mogą być. Ważne, że to powinny być takie sanity checki - ostatnia deska ratunku.
Walidacja i komunikacja z użytkownikiem powinna być na poziomie wyżej.
Także serwisy lepiej jak zwracają jakieś Eithery, Option(ale) niż ja rzucają exceptionami (ale to już ewentualna dyskusja - nie każdy musi lubić robienie Scali w Javie).

filemonczyk napisał(a):

Czy mozna z góry powiedziec ze zawsze lepiej jest wywalic settery i zrobic assercje w konstruktorze? (zakladajac ze settery nie sa potrzebne w innym miejscu kodu)

Zależy od przeznaczenia obiektów. Takie klasy będące de facto strukturami do przenoszenia danych mogą sobie settery mieć.

Oczywiście, to zależy od przeznaczenia, ale generalnie zauważyłem taką praktykę, że gdzie można to odchodzi się od setterów na rzecz immutable objects.

InterruptedException napisał(a):

Oczywiście, to zależy od przeznaczenia, ale generalnie zauważyłem taką praktykę, że gdzie można to odchodzi się od setterów na rzecz immutable objects.

Wszędzie można, ale po co? Po prostu czasami masz obiekty, które nie mogą być wypełnione przy tworzeniu ale powinny być przekazane dalej. Dodatkowo jeśli nie dzielisz tych obiektów pomiędzy wątkami to przejście z stateful na stateless ma pewnie raczej znikome znaczenie.