smelly C # pattern - consiglio di refactoring per favore

Question

smelly C # pattern - consiglio di refactoring per favore

#1 da (9 voti)
#2 da (3 voti)

1

Ho aggiunto questo metodo a una classe C #:

public bool CanAddLeave(Leave newLeave, out AddLeaveResult result)
{
    result = _repository.CreateSqlQuery("CanSendLeaveRequest ")
            .SetParameter("employeePositionId", newLeave.EmployeePosition.EmployeePositionId)
            .UniqueResult<AddLeaveResult> ();

    return (result == AddLeaveResult.Success);
}

Il metodo restituisce un bool che il cliente utilizzerà per determinare rapidamente se una richiesta di ferie (ferie / festività, ecc.) può essere inviata al manager del dipendente tramite e-mail.

Il parametro di output AddLeaveResult è un enum con i seguenti valori Success = 0, NoManagersWithEmailAddress = 1, NoManagersToDeliverTo = 2

Durante la scrittura di questo ho avuto in mente il pattern TryParse, ma questo codice ha un cattivo odore. Il mio istinto mi sta dicendo che c'è un modo migliore, ma non riesco a pensarne uno. Puoi suggerire un modello migliore e più pulito da usare?

c# design-patterns

posta Scott 18.01.2017 - 12:37

fonte

2 risposte

3

Restituisce il risultato:

public AddLeaveResult CanAddLeave(Leave newLeave) { }

Qual è la vera ragione per cui un booleano deve essere restituito alla GUI? Un altro controllo result == AddLeaveResult.Success nella GUI è sufficiente per determinare se è possibile inviare un'e-mail. Sembra che il tipo di ritorno booleano non sia stato scelto per una buona ragione a parte il fatto che risuoni nel momento giusto in cui la persona che lo ha implementato ci ha pensato per un secondo.

Inoltre, penso che tu stia usando la parola "pattern" troppo spesso come se tornasse in ogni angolo del tuo codice. Non tutto è un modello.

La pulizia e la leggibilità sono due soggetti soggettivi ma il consenso generale è che:

L'intento di una funzione dovrebbe essere chiaro leggendo la sua firma (tipo di ritorno, nome e parametro (i)). Nel tuo esempio dovrei leggere il corpo delle funzioni per sapere quando / cosa rappresenta il booleano e quando / cosa rappresenta l'AddLeaveResult.
Una funzione (o un metodo) dovrebbe fare UNA cosa (e quindi non avere effetti collaterali)
Devi ridurre al minimo la quantità di parametri che un metodo usa
Utilizza solo i parametri di rif e out se non riesci a risolverlo diversamente senza distruggere il resto del codice
etcetera

risposta data 18.01.2017 - 12:51

fonte

Leggi altre domande sui tag c# design-patterns

quando deve essere eseguita la modellazione dei dati? Git - Motivo per vietare l'uso di client GUI diversi da Git Bash \ CMD

score 9 · Accepted Answer

Restituisci il AddLeaveResult direttamente e rilascia il parametro out.

public AddLeaveResult CanAddLeave(Leave newLeave)
{
    AddLeaveResult result = _repository.CreateSqlQuery("CanSendLeaveRequest")
            .SetParameter("employeePositionId", newLeave.EmployeePosition.EmployeePositionId)
            .UniqueResult<AddLeaveResult>();
    return result;
}

In questo modo tutte le informazioni disponibili vengono inviate al chiamante. Il valore booleano precedentemente restituito può essere dedotto controllando result == AddLeaveResult.Success .

I metodi

TryParse sono diversi, non possono restituire un valore significativo in caso di errore, quindi devono restituire bool (con esito positivo o negativo) e compilare il parametro out in caso di successo.