I controlli di integrità dei dati dovrebbero essere separati dai metodi che inseriscono i dati?

4

Ho un semplice esempio di due configurazioni dello stesso blocco di codice, ma non sono sicuro di quale sarebbe l'opzione "migliore", anche se soggettiva, ho letto di metodi che spesso fanno troppo di quanto descrivono.

Quindi l'esempio è dove voglio aggiungere dati e fare un controllo di sanità mentale prima ... le due scelte sarebbero:

public void Add(Data data)
{
    if(sanitycheck(data))
    {
       // ADD to list
    }
}

La seconda opzione - questa separa la logica in 2 metodi:

public void Process(Data data)
{
   if(sanitycheck(data))
   {
      Add(data);
   }
}
private void Add(Data data) //private method
{
  // ADD to list
}

Mi sembra che il primo faccia "più di quello che dice", ma il secondo potrebbe non essere necessario e tagliare i metodi un po 'troppo sottili per il loro scopo.

Questo esempio è crudo rispetto alle situazioni della vita reale, ma sono curioso che generalmente è un approccio più intelligente e una procedura ottimale in quanto la complessità del codice aumenta?

    
posta Dave 27.05.2017 - 21:49
fonte

4 risposte

4

Dal POV del client, l'unica cosa che stai facendo è ridenominare Add in Process (e, naturalmente, potresti evitare la ridenominazione se hai usato qualche variazione per il nuovo metodo, come InternalAdd o sovraccarico il metodo).

Dal punto di vista interno, i motivi per creare metodi stanno riducendo la complessità del riutilizzo del codice e del codice. Potrebbe avere senso se il codice Add è molto complesso, o se Add potrebbe essere chiamato con altri metodi senza eseguire i controlli di integrità. Dato che il controllo sembra piuttosto semplice, la domanda successiva dovrebbe essere la più importante: è un bug se, in qualsiasi momento, viene chiamato Add senza aver fatto i controlli? Se la risposta è sì, quasi l'unico risultato della divisione del metodo è il rischio di introdurre un bug.

Ora, l'opzione che preferisco è attenersi al principio di Responsabilità singola ( qualunque esso sia ). Ciò significherebbe spostare la convalida dei dati (sanitycheck) in una classe diversa.

Alcuni dei vantaggi di cui sopra sono:

  • Le tue classi sono più piccole e più semplici.

  • È possibile convalidare i dati in più fasi, si spera il prima possibile. Non ha senso se si esegue alcuna elaborazione se è possibile controllare i dati nella fase della GUI e avvisare l'utente.

  • Puoi riutilizzare il codice di convalida in un altro progetto anche se la parte di elaborazione è completamente diversa.

risposta data 28.05.2017 - 02:22
fonte
1

Il problema con il tuo primo metodo, e sospetto che il motivo per cui percepisca il problema, ma per il quale non sei in grado di trovare la motivazione esatta, è che opera in due completamente diversi livelli di astrazione. Potrebbe non essere il più noto dei principi di progettazione, ma io tendo a pensare che il requisito di qualsiasi metodo operi in un livello singolo di astrazione è piuttosto importante - è legato al principio di responsabilità singola (come descritto nella risposta di SJuan76) ma è molto più facile da capire e individuare le violazioni di.

Un altro commento riguardante la tua domanda: suggerisci che il tuo secondo approccio potrebbe "tagliare i metodi troppo sottili", ma nella mia esperienza non esiste una cosa del genere: anche se un metodo chiama semplicemente altri due metodi, ha un valore perché offre l'opportunità di nominare la combinazione di questi metodi e perché fornisce un punto in cui è possibile introdurre in modo pulito una modifica del processo - ad es. se diventa necessario un terzo metodo - diciamo che è necessario inviare un evento ad alcuni ascoltatori sottoscritti quando vengono aggiunti nuovi dati, ciò aggiungerebbe ancora più complessità in un singolo metodo usando il tuo primo progetto, ma è un cambiamento semplice ed elegante nel tuo secondo .

    
risposta data 28.05.2017 - 03:51
fonte
0

Non sono d'accordo con Jules o SJuan76. Convalidare i dati e iniziare qualsiasi azione su di esso non sono responsabilità separabili. Il tuo problema è che non stai lanciando!

I dati di elaborazione potrebbero non riuscire per una serie di motivi e i dati come falsi in primo luogo sono solo uno di questi. Per qualsiasi problema non risolvibile che si verifica, lanciare un'eccezione. Non solo un'eccezione, ma una dattiloscritta che dice esattamente al chiamante qual è il problema.

    
risposta data 29.05.2017 - 18:38
fonte
0

Mi chiedo in particolare quali controlli di sanità mentale debbano essere effettuati. Per iniziare, la classe Data dovrebbe fare già una serie di controlli di integrità per garantire che i dati abbiano effettivamente un senso. Ma capisco che ci sono casi in cui si applicano limitazioni aggiuntive, ad esempio quando Data può avere valori mancanti ma non vuoi che siano aggiunti.

Personalmente, uso un terzo modo per farlo. In pseudo codice:

public void Add(Data data)
{
   if( not(sanitycheck(data)))
   {
      throwException()
   }

//add code to add here
}

Quindi:

  • se si applicano limitazioni aggiuntive, controlla questi primi
  • se queste limitazioni non sono soddisfatte, chiedi al chiamante cosa non va
  • altrimenti, esegui

Le ulteriori limitazioni devono essere correttamente documentate. Ma in questo modo c'è un piccolo motivo per creare un vuoto extra privato.

Questo è diverso anche se il metodo Add può anche essere chiamato da altre posizioni con diversi controlli di integrità. Questa sarebbe l'unica ragione per me per creare un metodo privato separato.

    
risposta data 29.05.2017 - 19:53
fonte

Leggi altre domande sui tag