Posizionare le precondizioni nel posto giusto

Sto cercando di capire il modo migliore per definire la responsabilità del controllo delle precondizioni.

Consideriamo il caso seguente: abbiamo una classe Event che rappresenta un evento sportivo e una classe Matchup che modella una corrispondenza tra i giocatori di quell'evento. L'evento ha una serie di matchup Set<Matchup> matchups e possiamo aggiungerne uno utilizzando un metodo addMatchup() :

public void addMatchup(Matchup matchup) {
    matchups.add(matchup);
}

Ma abbiamo alcune precondizioni di corrispondenza:

• Non può essere null
• Nessun giocatore in un matchup può essere null
• Tutti i giocatori devono appartenere all'evento ( Event ha un membro List<Player> players )

Quello che sto cercando di decidere è se è meglio eseguire quei controlli di precondizione nella classe Matchup o nel metodo addMatchup . Vediamo questo:

Opzione A: verifica le precondizioni in Matchup :

public Matchup(Event event, Set<Player> players) {
    if (players == null)
        throw new IllegalArgumentException("Player cannot be null");
    if (players.contains(null))
        throw new IllegalArgumentException("A player cannot be null");
    if (!event.getPlayers().containsAll(players))
        throw new IllegalArgumentException("Not all players belong to the event");

    this.event = event;
    this.players = players;
}

E Event#addMatchup() sarebbe:

public void addMatchup(Matchup matchup) {
    if (matchup == null)
        throw new IllegalArgumentException("Matchup cannot be null");
    matchups.add(matchup);
}

Opzione B: verifica le precondizioni nel metodo Event :

public void addMatchup(Matchup matchup) {
    if (matchup == null)
        throw new IllegalArgumentException("Matchup cannot be null");
    if (matchup.getPlayers() == null)
        throw new IllegalArgumentException("Matchup players cannot be null");
    if (matchup.getPlayers().contains(null))
        throw new IllegalArgumentException("A matchup player cannot be null");
    if (!players.containsAll(matchup.getPlayers())
        threw new IllegalArgumentException("Not all matchup players belong to this event");

    matchups.add(matchup);
}

E il costruttore di Matchup sarebbe solo:

public Matchup(Set<Player> players) {
    this.players = players;
}

Nota che qui non avremmo nemmeno bisogno di un membro Event .

Per come la vedo io, ci sono pro e contro in entrambi gli approcci:

Opzione A: Pro

• Se avessimo più metodi di Event#addMatchup() -like, non dovremmo ripetere più volte il controllo delle precondizioni in ogni metodo perché è già controllato nella classe Matchup . (Questo è il motivo principale per cui ho effettivamente adottato questo approccio nel mio progetto attuale)

Opzione A: Contro

• Non sembra che sia responsabilità di Matchup controllare tali presupposti perché queste sono regole definite da un evento, non un matchup. Nel contesto dell'applicazione ha senso al momento, ma cosa accadrebbe se la classe Matchup dovesse essere utilizzata in modo diverso? Quindi le sue possibilità sarebbero ridotte a causa dei limiti che queste precondizioni impongono a tutti gli stati possibili.

• Forza un event da passare al costruttore per eseguire quei controlli. Cosa succede se non desideriamo avere un membro Event nel nostro Matchup ? Beh, potremmo omettere quel membro, ma dovremmo comunque passarlo come argomento e, onestamente, mi sembra strano passarlo a un costruttore se non verrà memorizzato (o praticamente nessun altro metodo, davvero).

Opzione B: Pro

• La responsabilità di controllare le condizioni preliminari ricade sul luogo in cui ritiene che dovrebbe essere. Se un evento afferma che un matchup non può essere null , non può contenere null giocatori e tutti i giocatori devono essere contenuti nel set di giocatori dell'evento, quindi questo dovrebbe essere applicato da Event , non da Matchup .

• Non abbiamo bisogno di un membro Event nella classe Matchup , o anche di dover passare come argomento.

Opzione B: Contro

• Se avessimo più metodi come addMatchup in Event , dovremmo ripetere tutti questi controlli di precondizione in tutti loro. Questo è un compito faticoso, viola DRY e rende il codice più sporco. Potrei spostare gli assegni in un metodo checkMatchupPreconditions() privato, e in realtà l'ho fatto molto spesso per altre parti del progetto, ma per qualche motivo non mi piace, penso che renda il codice più sporco, togli una certa leggibilità e , in primo luogo, si perde traccia della traccia di esecuzione.

Forse è solo una questione di gusti e tale decisione non è un grosso problema, ma mi piacerebbe sentirti comunque su questo argomento.