Un metodo deve essere perdonante con gli argomenti che vengono inoltrati? [chiuso]

21

Supponiamo di avere un metodo foo(String bar) che funziona solo su stringhe che soddisfano determinati criteri; ad esempio, deve essere in minuscolo, non deve essere vuoto o avere solo spazi bianchi e deve corrispondere al modello [a-z0-9-_./@]+ . La documentazione per il metodo indica questi criteri.

Il metodo dovrebbe rifiutare qualsiasi e tutte le deviazioni da questo criterio, o se il metodo fosse più indulgente su alcuni criteri? Ad esempio, se il metodo iniziale è

public void foo(String bar) {
    if (bar == null) {
        throw new IllegalArgumentException("bar must not be null");
    }
    if (!bar.matches(BAR_PATTERN_STRING)) {
        throw new IllegalArgumentException("bar must match pattern: " + BAR_PATTERN_STRING);
    }
    this.bar = bar;
}

E il secondo metodo permissivo è

public void foo(String bar) {
    if (bar == null) {
        throw new IllegalArgumentException("bar must not be null");
    }
    if (!bar.matches(BAR_PATTERN_STRING)) {
        bar = bar.toLowerCase().trim().replaceAll(" ", "_");
        if (!bar.matches(BAR_PATTERN_STRING) {
            throw new IllegalArgumentException("bar must match pattern: " + BAR_PATTERN_STRING);
        }
    }
    this.bar = bar;
}

La documentazione deve essere cambiata per dichiarare che sarà trasformata e impostata sul valore trasformato, se possibile, o se il metodo dovrebbe essere il più semplice possibile e rifiutare tutte le deviazioni? In questo caso, bar potrebbe essere impostato dall'utente di un'applicazione.

Il caso d'uso principale per questo sarebbe l'accesso agli oggetti da un repository da parte di uno specifico identificatore di stringa. Ogni oggetto nel repository dovrebbe avere una stringa univoca per identificarlo. Questi repository potevano archiviare gli oggetti in vari modi (sql server, json, xml, binary, ecc.) E così ho cercato di identificare il minimo comune denominatore che corrispondesse alla maggior parte delle convenzioni di denominazione.

    
posta Zymus 03.08.2015 - 00:52
fonte

6 risposte

47

Il tuo metodo dovrebbe fare ciò che dice di fare.

Questo impedisce ai bug, sia dall'uso che dai manutentori, di cambiare comportamento in seguito. Risparmia tempo perché i manutentori non debbano spendere tanto tempo per capire cosa sta succedendo.

Detto questo, se la logica definita non è facile da usare, dovrebbe forse essere migliorata.

    
risposta data 03.08.2015 - 01:03
fonte
20

Ci sono alcuni punti:

  1. La tua implementazione deve fare ciò che afferma il contratto documentato e non dovrebbe fare altro.
  2. La semplicità è importante, sia per il contratto che per l'implementazione, anche se più per il primo.
  3. Il tentativo di correggere l'input errato aggiunge complessità, in modo intuitivo non solo al contratto e all'implementazione, ma anche all'utilizzo.
  4. Gli errori dovrebbero essere rilevati in anticipo solo se ciò migliora la debugabilità e non compromette troppo l'efficienza.
    Ricorda che esistono delle debug-assertion per diagnosticare gli errori logici in modalità debug, che allevia principalmente qualsiasi problema di prestazioni.
  5. L'efficienza, a patto che tempo e denaro disponibili lo consentano senza compromettere troppo la semplicità, è sempre un obiettivo.

Se implementi un'interfaccia utente, i messaggi di errore descrittivi (inclusi suggerimenti e altro aiuto) fanno parte di un buon design.
Ma ricorda che le API sono per i programmatori, non per gli utenti finali.

Un vero esperimento di essere sfocato e permissivo con l'input è HTML.
Il che ha comportato che tutti lo facessero in modo leggermente diverso, e le specifiche, ora è documentato, sono un enorme numero pieno di casi speciali.
Vedi Legge di Postel (" Sii prudente in ciò che fai, sii liberale in ciò che accetti dagli altri. ") e un critico che ci tocca ( O molto meglio MichaelT mi ha fatto conoscere ).

    
risposta data 03.08.2015 - 02:17
fonte
15

Il comportamento di un metodo dovrebbe essere chiaro, intuitivo, prevedibile e semplice. In generale, dovremmo essere molto riluttanti a eseguire un'ulteriore elaborazione sull'input di un chiamante. Tale ipotesi su ciò che il chiamante intendeva invariabilmente ha molti casi limite che producono un comportamento indesiderato. Considera un'operazione semplice come l'unione dei percorsi dei file. Molte (o forse anche la maggior parte) delle funzioni di unione dei percorsi dei file ignoreranno automaticamente tutti i percorsi precedenti se uno dei percorsi che vengono uniti apparirà radicato! Ad esempio, /abc/xyz unita a /evil avrà come risultato solo /evil . Questo non è quasi mai ciò che intendo quando mi unisco ai percorsi dei file, ma poiché non c'è un'interfaccia che non si comporti in questo modo, sono obbligato ad avere bug o scrivere codice extra che copre questi casi.

Detto questo, ci sono rare occasioni in cui è logico che un metodo sia "perdonatore", ma dovrebbe essere sempre essere nel potere del chiamante per decidere quando e se queste fasi di elaborazione si applicano alla loro situazione. Pertanto, quando hai identificato un passaggio di pre-elaborazione comune che desideri applicare agli argomenti in una varietà di situazioni, devi esporre le interfacce per:

  • La funzionalità non elaborata, senza alcuna pre-elaborazione.
  • Il passaggio di pre-elaborazione di per sé .
  • La combinazione della funzionalità non elaborata e la pre-elaborazione.

L'ultimo è facoltativo; dovresti fornirlo solo se un numero elevato di chiamate lo utilizzerà.

L'esposizione della funzionalità non elaborata offre al chiamante la possibilità di utilizzarlo senza la fase di pre-elaborazione quando è necessario. Esporre il passo del preprocessore da solo permette al chiamante di usarlo per situazioni in cui non stanno nemmeno chiamando la funzione o quando vogliono pre-processare qualche input prima chiamando la funzione (come quando vogliono passarlo prima in un'altra funzione). Fornire la combinazione consente ai chiamanti di invocare entrambi senza problemi, il che è utile soprattutto se la maggior parte dei chiamanti lo usa in questo modo.

    
risposta data 03.08.2015 - 09:16
fonte
4

Come altri hanno già detto, rendere la stringa che corrisponde a "perdonare" significa introdurre ulteriore complessità. Ciò significa più lavoro nell'implementazione della corrispondenza. Ad esempio, ora ci sono molti altri casi di test. Devi fare del lavoro aggiuntivo per assicurarti che non ci siano nomi semi-uguali nello spazio dei nomi. Una maggiore complessità significa anche che c'è di più da sbagliare in futuro. Un meccanismo più semplice, come una bicicletta, richiede meno manutenzione di uno più complesso, come una macchina.

Quindi la combinazione di stringhe lievi vale tutto quel costo aggiuntivo? Dipende dal caso d'uso, come altri hanno notato. Se le stringhe sono una sorta di input esterno non hai controllo, e c'è un netto vantaggio per l'abbinamento clemente, è potrebbe valerne la pena. Forse l'input arriva dagli utenti finali che potrebbero non essere molto coscienziosi in merito ai caratteri spaziali e alle lettere maiuscole e hai un strong incentivo a rendere il tuo prodotto più facile da usare.

D'altro canto, se l'input provenisse da, ad esempio, file di proprietà assemblati da tecnici, che dovrebbero capire che "Fred Mertz" != "FredMertz" , sarei più propenso a rendere più rigoroso il matching e a risparmiare il costo di sviluppo.

Penso comunque che valga la pena di tralasciare e trascurare gli spazi iniziali e finali, ho visto troppe ore sprecate per il debugging di questo tipo di problemi.

    
risposta data 03.08.2015 - 04:26
fonte
3

Hai menzionato parte del contesto da cui proviene questa domanda.

Dato questo, vorrei che il metodo facesse solo una cosa, asserisse i requisiti sulla stringa, lasciandola eseguire in base a quello - non proverei a trasformarla qui. Resta semplice e tienilo chiaro; documentalo e cerca di mantenere la documentazione e il codice in sincronia tra loro.

Se desideri trasformare i dati provenienti dal database utente in modo più indulgente, metti tale funzionalità in un metodo di trasformazione separato e documenta la funzionalità legata .

A un certo punto i requisiti della funzione devono essere misurati, chiaramente documentati e l'esecuzione deve continuare. Il "perdono", al suo punto, è un po 'muto, è una decisione progettuale e vorrei sostenere che la funzione non muta la sua argomentazione. Avendo la funzione mutare l'input nasconde una parte della convalida che sarebbe richiesta al client. Avere una funzione che fa la mutazione aiuta il client a farlo bene.

Il grande enfasi qui è chiarezza e per documentare ciò che il codice fa .

    
risposta data 03.08.2015 - 08:26
fonte
-1
  1. Puoi assegnare un nome a un metodo in base all'azione, ad esempio doSomething (), takeBackUp ().
  2. Per semplificare la manutenzione è possibile mantenere i contratti e la convalida comuni su diverse procedure. Chiamali come per i casi d'uso.
  3. Programmazione difensiva: la tua procedura gestisce un'ampia gamma di input compresi (Le cose minime che sono casi d'uso devono essere comunque coperte)
risposta data 03.08.2015 - 17:53
fonte

Leggi altre domande sui tag