Convalida dei parametri del costruttore in C #: best practice

32

Qual è la migliore pratica per la validazione dei parametri del costruttore?

Supponi un semplice bit di C #:

public class MyClass
{
    public MyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            throw new ArgumentException("Text cannot be empty");

        // continue with normal construction
    }
}

Sarebbe accettabile lanciare un'eccezione?

L'alternativa che ho incontrato era la pre-validazione, prima dell'istanziazione:

public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
        {
            MessageBox.Show("Text cannot be empty");
            return null;
        }
        else
        {
            return new MyClass(text);
        }
    }
}
    
posta MPelletier 23.02.2011 - 19:56
fonte

9 risposte

24

Tendo ad eseguire tutte le mie convalide nel costruttore. Questo è un must perché creo quasi sempre oggetti immutabili. Per il tuo caso specifico, penso che sia accettabile.

if (string.IsNullOrEmpty(text))
    throw new ArgumentException("message", "text");

Se stai usando .NET 4 puoi farlo. Ovviamente ciò dipende dal fatto che si consideri che una stringa che contiene solo lo spazio bianco non è valida.

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
    
risposta data 23.02.2011 - 20:03
fonte
20

Molte persone affermano che i costruttori non dovrebbero lanciare eccezioni. KyleG su questa pagina , ad esempio, fa proprio questo. Onestamente, non riesco a pensare a una ragione perché no.

In C ++, lanciare un'eccezione da un costruttore è una cattiva idea, perché ti lascia con una memoria allocata contenente un oggetto non inizializzato a cui non hai alcun riferimento (ad esempio è una perdita di memoria classica). Probabilmente è da lì che nasce lo stigma: un gruppo di sviluppatori C ++ della vecchia scuola ha approfondito il loro apprendimento in C # e ha applicato solo ciò che sapevano dal C ++. Al contrario, in Objective-C Apple ha separato la fase di allocazione dalla fase di inizializzazione, quindi i costruttori in quella lingua possono generare eccezioni.

C # non può perdere memoria da una chiamata non riuscita a un costruttore. Persino alcune classi nel framework .NET generano eccezioni nei loro costruttori.

    
risposta data 23.02.2011 - 21:09
fonte
12

Getta un'eccezione IFF la classe non può essere messa in uno stato coerente per quanto riguarda il suo uso semantico. Altrimenti no. Non consentire MAI che un oggetto esista in uno stato incoerente. Ciò include non fornire i costruttori completi (come avere un costruttore vuoto + initialize () prima che il tuo oggetto sia in realtà completamente costruito) ... SOLO NO!

In un pizzico, lo fanno tutti. L'ho fatto l'altro giorno per un oggetto molto usato in un ambito ristretto. Un giorno o l'altro, io o qualcun altro probabilmente pagheremo il prezzo per quel tagliando in pratica.

Devo notare che con "costruttore" intendo la cosa che il client chiama per costruire l'oggetto. Questo potrebbe altrettanto facilmente essere qualcosa di diverso da un vero costrutto che va sotto il nome di "Costruttore". Ad esempio, qualcosa di simile in C ++ non viola il principio IMNSHO:

struct funky_object
{
  ...
private:
  funky_object();
  bool initialize(std::string);

  friend boost::optional<funky_object> build_funky(std::string);
};
boost::optional<funky_object> build_funky(std::string str)
{
  funky_object fo;
  if (fo.initialize(str)) return fo;
  return boost::optional<funky_object>();
}

Poiché l'unico modo per creare un funky_object è chiamare build_funky il principio di non consentire mai un oggetto non valido rimane intatto anche se il "Costruttore" non termina il lavoro.

Questo è un sacco di lavoro extra per guadagni discutibili (forse anche per perdita). Preferirei comunque la route di eccezione.

    
risposta data 23.02.2011 - 20:12
fonte
9

In questo caso, userei il metodo factory. Fondamentalmente, imposta la tua classe ha solo costruttori privati e ha un metodo factory che restituisce un'istanza del tuo oggetto. Se i parametri iniziali non sono validi, basta restituire null e fare in modo che il codice chiamante decida cosa fare.

public class MyClass
{
    private MyClass(string text)
    {
        //normal construction
    }

    public static MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            return null;
        else
            return new MyClass(text);
    }
}
public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        var cls = MyClass.MakeMyClass(text);
        if(cls == null)
             //show messagebox or throw exception
        return cls;
    }
}

Non lanciare eccezioni a meno che le condizioni siano eccezionali. Sto pensando che in questo caso, un valore vuoto può essere passato facilmente. In tal caso, l'utilizzo di questo modello eviterà eccezioni e le penalità relative alle prestazioni che incorre nel mantenere valido lo stato di MyClass.

    
risposta data 25.02.2011 - 10:01
fonte
1
  • Un costruttore non dovrebbe avere effetti collaterali.
    • Qualunque cosa in più dell'inizializzazione del campo privato dovrebbe essere vista come un effetto collaterale.
    • Un costruttore con effetti collaterali rompe il Principio di Responsabilità Singola (SRP) e funziona in modo contrario allo spirito della programmazione orientata agli oggetti (OOP).
  • Un costruttore dovrebbe essere leggero e non dovrebbe mai fallire.
    • Ad esempio, rabbrividisco sempre quando vedo un blocco try-catch all'interno di un costruttore. Un costruttore non dovrebbe lanciare eccezioni o errori di registro.

Si potrebbe ragionevolmente mettere in dubbio queste linee guida e dire: "Ma io non seguo queste regole e il mio codice funziona bene!" A questo, vorrei rispondere, "Beh, potrebbe essere vero, finché non lo è."

  • Le eccezioni e gli errori all'interno di un costruttore sono molto inaspettati. A meno che non gli venga detto di farlo, i futuri programmatori non saranno inclini a circondare queste chiamate del costruttore con un codice difensivo.
  • Se qualcosa non funziona in produzione, la traccia dello stack generata può essere difficile da analizzare. La parte superiore della traccia dello stack può puntare alla chiamata del costruttore, ma molte cose accadono nella funzione di costruzione e potrebbe non puntare alla LOC effettiva che ha avuto esito negativo.
    • Ho analizzato molte tracce dello stack .NET in questo caso.
risposta data 06.07.2014 - 01:33
fonte
0

Dipende da cosa sta facendo MyClass. Se MyClass era in realtà una classe di repository di dati e il testo del parametro era una stringa di connessione, la soluzione migliore sarebbe quella di generare un'eccezione ArgumentException. Tuttavia, se MyClass è la classe StringBuilder (ad esempio), puoi semplicemente lasciarlo in bianco.

Quindi dipende da quanto il testo del parametro sia essenziale per il metodo - l'oggetto ha senso con un valore nullo o vuoto?

    
risposta data 23.02.2011 - 20:04
fonte
0

La mia preferenza è impostare un valore predefinito, ma so che Java ha la libreria "Apache Commons" che fa qualcosa del genere, e penso che sia anche un'idea abbastanza buona. Non vedo un problema con il lancio di un'eccezione se il valore non valido pone l'oggetto in uno stato inutilizzabile; una stringa non è un buon esempio, ma se fosse per DI? Non è possibile operare se è stato passato un valore di null al posto di, diciamo, un'interfaccia ICustomerRepository . In una situazione come questa, direi un'eccezione è il modo corretto di gestire le cose.

    
risposta data 12.08.2011 - 22:22
fonte
-1

Quando lavoravo in c ++, non ero solito gettare un'eccezione nel costruttore a causa di un problema di perdita di memoria che poteva portare a, l'avevo imparato nel modo più difficile. Chiunque lavori con c ++ sa quanto può essere difficile e problematico perdere memoria.

Ma se sei in c # / Java, allora non hai questo problema, perché il garbage collector raccoglierà la memoria. Se stai usando C #, penso che sia preferibile usare la proprietà in modo piacevole e coerente per assicurarti che i vincoli sui dati siano garantiti.

public class MyClass
{
    private string _text;
    public string Text 
    {
        get
        {
            return _text;
        } 
        set
        {
            if (string.IsNullOrWhiteSpace(value))
                throw new ArgumentException("Text cannot be empty");
            _text = value;
        } 
    }

    public MyClass(string text)
    {
        Text = text;
        // continue with normal construction
    }
}
    
risposta data 13.03.2012 - 16:28
fonte
-3

Lanciare un'eccezione sarà un vero dolore per gli utenti della tua classe. Se la convalida è un problema, in genere eseguo la creazione dell'oggetto in un processo in 2 passaggi.

1 - Crea l'istanza

2 - Chiama un metodo di inizializzazione con un risultato di ritorno.

o

Chiama un metodo / funzione che crea la classe per te che può eseguire la convalida.

o

Hai un flag isValid, che non mi piace particolarmente ma che alcune persone fanno.

    
risposta data 23.02.2011 - 20:06
fonte

Leggi altre domande sui tag