Perché Optional.get () non chiama isPresent () male, ma non iterator.next ()?

23

Quando si utilizza la nuova API di Java8 stream per trovare un elemento specifico in una raccolta, scrivo un codice come questo:

    String theFirstString = myCollection.stream()
            .findFirst()
            .get();

Qui IntelliJ avverte che get () viene chiamato senza prima controllare isPresent ().

Tuttavia, questo codice:

    String theFirstString = myCollection.iterator().next();

... non fornisce alcun avviso.

C'è una profonda differenza tra le due tecniche, che rende l'approccio allo streaming più "pericoloso" in qualche modo, che è fondamentale non chiamare mai get () senza chiamare isPresent () prima? Cercando su Google, trovo articoli che parlano di come i programmatori sono negligenti con Optional < > e supponiamo di poter chiamare get () ogni volta.

Il fatto è che mi piace ottenere un'eccezione il più vicino possibile al luogo in cui il mio codice è bacato, che in questo caso sarebbe dove sto chiamando get () quando non c'è alcun elemento da trovare . Non voglio scrivere saggi inutili come:

    Optional<String> firstStream = myCollection.stream()
            .findFirst();
    if (!firstStream.isPresent()) {
        throw new IllegalStateException("There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>");
    }
    String theFirstString = firstStream.get();

... a meno che non vi sia qualche pericolo nel provocare eccezioni da get () di cui non sono a conoscenza?

Dopo aver letto la risposta di Karl Bielefeldt e un commento di Hulk, mi sono reso conto che il mio codice di eccezione sopra era un po 'goffo, ecco qualcosa di meglio:

    String errorString = "There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>";
    String firstString = myCollection.stream()
            .findFirst()
            .orElseThrow(() -> new IllegalStateException(errorString));

Sembra più utile e probabilmente verrà naturale in molti posti. Mi sento ancora come se volessi essere in grado di scegliere un elemento da una lista senza dover affrontare tutto questo, ma potrebbe essere solo che ho bisogno di abituarmi a questi tipi di costrutti.

    
posta TV's Frank 19.05.2016 - 10:44
fonte

3 risposte

10

Prima di tutto, considera che il controllo è complesso e probabilmente richiede molto tempo. Richiede alcune analisi statiche e potrebbe esserci un bel po 'di codice tra le due chiamate.

In secondo luogo, l'uso idiomatico dei due costrutti è molto diverso. Programma a tempo pieno in Scala, che usa Options molto più frequentemente di quanto faccia Java. Non riesco a ricordare una singola istanza in cui ho dovuto utilizzare un .get() su un Option . Dovresti quasi sempre restituire Optional dalla tua funzione oppure utilizzare orElse() . Questi sono modi molto superiori per gestire i valori mancanti rispetto al lancio di eccezioni.

Dato che raramente .get() dovrebbe essere chiamato nell'uso normale, è logico che un IDE avvii un controllo complesso quando vede un .get() per vedere se è stato usato correttamente. Al contrario, iterator.next() è l'uso corretto e ubiquo di un iteratore.

In altre parole, non si tratta di uno che è cattivo e l'altro no, si tratta di un caso comune che è fondamentale per il suo funzionamento ed è altamente probabile che generi un'eccezione durante il test degli sviluppatori, mentre l'altro è un caso raramente usato che non può essere esercitato abbastanza da generare un'eccezione durante il test degli sviluppatori. Questi sono i tipi di casi in cui desideri che gli IDE ti avvisino.

    
risposta data 19.05.2016 - 15:39
fonte
14

La classe opzionale è pensata per essere utilizzata quando non è noto se l'elemento in esso contenuto sia presente o meno. L'avviso esiste per informare i programmatori che esiste un ulteriore percorso di codice che potrebbero essere in grado di gestire con garbo. L'idea è di evitare che vengano lanciate eccezioni. Il caso d'uso che hai presentato non è quello per cui è stato progettato, e quindi l'avviso è irrilevante per te - se vuoi effettivamente lanciare un'eccezione, fallo e disabilitala (anche se solo per questa linea, non a livello globale - dovresti essere prendere la decisione di gestire o meno il caso non presente su un'istanza per istanza e l'avviso ti ricorderà di farlo.

Probabilmente, dovrebbe essere generato un avvertimento simile per gli iteratori. Tuttavia, semplicemente non è mai stato fatto, e aggiungerne uno ora probabilmente causerebbe troppi avvisi nei progetti esistenti per essere una buona idea.

Nota inoltre che lanciare la tua eccezione può essere più utile di una semplice eccezione, includendo una descrizione di quale elemento ci si aspettava che esistesse, ma non può essere molto utile nel debug in un secondo momento.

    
risposta data 19.05.2016 - 11:02
fonte
6

Sono d'accordo sul fatto che non vi è alcuna differenza intrinseca in questi, tuttavia, vorrei sottolineare un difetto più grande.

In entrambi i casi, si ha un tipo, che fornisce un metodo che non è garantito per funzionare e si suppone che si debba chiamare prima un altro metodo. Se il tuo IDE / compilatore / etc ti avvisa di un caso o dell'altro dipende solo se supporta questo caso e / o se lo hai configurato per farlo.

Detto questo, entrambi i pezzi di codice sono odori di codice. Queste espressioni accennano a difetti di progettazione sottostanti e producono codice fragile.

Hai ragione a voler fallire velocemente, naturalmente, ma la soluzione almeno per me, non può essere semplicemente scrollarla di dosso e gettare le mani in aria (o un'eccezione in pila).

Per ora la tua raccolta potrebbe non essere vuota, ma tutto ciò su cui puoi fare affidamento è il suo tipo: Collection<T> - e questo non garantisce la non-vacuità. Anche se ora sai che non è vuoto, un requisito modificato può comportare la modifica anticipata del codice e all'improvviso il tuo codice viene colpito da una raccolta vuota.

Ora sei libero di respingere e dire agli altri che dovresti ottenere una lista non vuota. Puoi essere felice di trovare questo 'errore' velocemente. Tuttavia, hai un pezzo di software rotto e hai bisogno di cambiare il tuo codice.

Confrontalo con un approccio più pragmatico: dal momento che ottieni una collezione e può essere vuota, ti costringi ad affrontare tale caso usando la # mappa opzionale, #orElse o qualsiasi altro di questi metodi, eccetto #get.

Il compilatore fa il maggior lavoro possibile per te in modo tale da poter contare il più possibile sul contratto di tipo statico di ottenere Collection<T> . Quando il tuo codice non viola mai questo contratto (ad esempio tramite una di queste due catene telefoniche maleodoranti), il tuo codice è molto meno fragile per cambiare le condizioni ambientali.

Nello scenario sopra riportato, una modifica che produce una raccolta di input vuota non avrebbe alcuna conseguenza per il tuo codice. È solo un altro input valido e quindi ancora gestito correttamente.

Il costo di questo è che devi investire tempo in progetti migliori, a differenza di a) rischiare codice fragile o b) complicare inutilmente le cose generando eccezioni.

Nota finale: poiché non tutti gli IDE / compilatori mettono in guardia sulle chiamate iterator (). next () o facoltative.get () senza i controlli precedenti, tale codice è generalmente contrassegnato nelle recensioni. Per quello che vale, non permetterei che un tale codice permetta di passare una recensione e richiedere invece una soluzione pulita.

    
risposta data 19.05.2016 - 11:13
fonte

Leggi altre domande sui tag