Il mio metodo deve restituire void o bool per indicare che ha avuto successo?

1

Sto integrando un ERP con un rivenditore di terze parti. Al termine dell'elaborazione degli ordini provenienti dalla terza parte, inviamo 2 chiamate API:

  1. Imposta lo stato dell'ordine su ReadyToShip per il quale trasmettiamo le informazioni sulla spedizione.
  2. Imposta lo stato dell'ordine su Shipped .

Non sono sicuro di quale dovrebbe essere il tipo di ritorno di questo metodo. Sto usando le eccezioni per notificare al codice chiamante che c'era un problema nell'impostazione dello stato di questi ordini. Per questo motivo, il mio metodo ExportShipments dovrebbe semplicemente restituire void? O non dovrei restituire nulla, ma forse un bool o anche un oggetto di reso personalizzato?

Il mio codice appare come questo, che usa attualmente bool, comunque penso che l'uso del bool non stia facendo nulla qui perché non restituirà mai false (perché solleverà un'eccezione):

    public bool ExportShipments(int iconicOrderId, string shippingProvider, string trackingNumber)
    {
        TheIconicModels.Order order = orderRepository.GetOrderById(iconicOrderId);
        bool setStatusToReadyToShipResult = true, setStatusToShippedResult = true;
        if ( order.Statuses.Status == OrderStatusConstants.PENDING)
        {
            setStatusToReadyToShipResult = orderRepository.SetStatusToReadyToShip(order, shippingProvider, trackingNumber);
        }
        if ( order.Statuses.Status == OrderStatusConstants.READY_TOS_SHIP)
        {
            setStatusToShippedResult = orderRepository.SetStatusToShipped(order);
        }

        return setStatusToReadyToShipResult && setStatusToShippedResult;
    }

    public bool SetStatusToReadyToShip(Order order, string shippingProvider, string trackingNumber)
    {
        var parameters = new List<AbstractParam>() {
            new OrderItemsParam(order),
            new ShippingProviderParam(shippingProvider),
            new TrackingNumberParam(trackingNumber),
            new DeliveryTypeParam("dropship")
        };

        TheIconicApiResult result = this.apiService.SendPostRequest("SetStatusToReadyToShip", parameters, String.Empty);
        var jsonResponse = JsonConvert.DeserializeObject<RootObject>(result.ResponseBody);

        return jsonResponse.SuccessResponse != null;
    }

Devo sapere se l'operazione è andata a buon fine come a un livello più alto, contrassegno questa spedizione come inviata.

Idealmente, quale dovrebbe essere il mio metodo di ritorno? Sto pensando a void e ho appena ottenuto alcune eccezioni da un livello superiore, ma apprezzerei qualche consiglio.

    
posta Lock 25.05.2017 - 23:52
fonte

5 risposte

6

Vorrei anche menzionare l'idea di Separazione delle query di comando

Idealmente,

  • Le query restituiscono un risultato (e non hanno effetti collaterali)
  • I comandi cambiano lo stato di un sistema.

Come mi sembra che i tuoi metodi siano essenzialmente comandi , vorrei che avessero una firma di ritorno vuota e sollevasse eccezioni sugli errori.

Di nuovo, anche se le tue query incontrano errori, anche loro dovrebbero sollevare eccezioni e non "restituire" qualcosa che indica un errore. I programmatori a volte usano null per indicare che una query non è riuscita, ma penso che sia mal consigliato.

    
risposta data 26.05.2017 - 10:06
fonte
1

Il metodo esiste per fare qualcosa. Questo è il suo contratto. Se non riesce a fare qualsiasi cosa si aspettasse di fare, dovrebbe essere lanciata un'eccezione. Il problema con i codici di ritorno è che le persone possono ignorarli, il che nasconde silenziosamente problemi con l'esecuzione del programma.

    
risposta data 26.05.2017 - 02:05
fonte
0

Se ci si aspetta che i guasti si verifichino spesso, per una normale attività, un valore di ritorno potrebbe essere un approccio OK. Ma se i guasti sono un'eccezione, basta lanciare un'eccezione. Questo è ciò per cui sono destinati.

    
risposta data 26.05.2017 - 00:28
fonte
0

Non mi piace usare le eccezioni per eventi non eccezionali. La mancata validazione aziendale non è eccezionale. Un'API di terze parti non è eccezionale. Il tuo database che non è disponibile è PROBABILE eccezionale, a meno che tu non abbia anticipato che potrebbe essere offline di volta in volta. In quanto tale, mi piace utilizzare un oggetto restituito / risultato generico tipizzato per comunicare l'esito positivo / errore della maggior parte delle chiamate come livelli nelle app.

public class CallResult
    {
        public bool Success { get; set; }
        public ValidationResult[] TransactionErrors { get; private set; }
        public string[] SuccessMessages { get; private set; } // in case you need to signal a partial success, or a custom result message

        public CallResult()
        {
            TransactionErrors = new ValidationResult[0];
            SuccessMessages = new string[0];
            Success = true; // Easier to let it be 'true' by default, and 'false' only when you add an error or set it manually!
        }

        public void AddTransactionError(string errorMessage, string[] memberNames = null)
        {
            var newError = new ValidationResult(errorMessage, memberNames ?? new[] { "" });
            TransactionErrors = TransactionErrors.Concat(new[] { newError }).ToArray();
            Success = false;
        }

        public void MergeWithErrorsFrom(CallResult transaction)
        {
            if (transaction == null || !transaction.TransactionErrors.Any())
                return;

            SuccessMessages = SuccessMessages.Concat(transaction.SuccessMessages).ToArray();
            TransactionErrors = TransactionErrors.Concat(transaction.TransactionErrors).ToArray();
            if (TransactionErrors.Any())
                Success = false;
        }

        public void AddSuccessMessage(string message)
        {
            SuccessMessages = SuccessMessages.Concat(new[] { message }).ToArray();
        }
    }

    public class CallResult<T> : CallResult
    {
        public T ResultObject { get; set; }

        public void MergeWithResultsFrom(CallResult<T> transaction)
        {
            MergeWithErrorsFrom(transaction);
            ResultObject = transaction.ResultObject;
        }

        public CallResult<T> AttemptLoad(Func<T> loadFunc, string emptyLoadMessage)
        {
            ResultObject = loadFunc();
            if (ResultObject == null)
                AddTransactionError(emptyLoadMessage);
            return this;
        }

        public static CallResult<T> Attempt(Func<T> loadFunc, string emptyLoadMessage)
        {
            return (new CallResult<T>()).AttemptLoad(loadFunc, emptyLoadMessage);
        }
    }
    
risposta data 26.05.2017 - 15:42
fonte
0

I am using exceptions to notify the calling code that there was a problem in setting the status of these orders.

Prima di riformulare, leggermente, cosa ha detto @Graham

Non utilizzare eccezioni per il controllo del flusso.

Stai confondendo il significato del bool e dell'eccezione. Fai in modo di creare il futuro, tu per un mondo di dolore.

Il bool dice che l'ordine è stato spedito o meno - vero o falso. L'eccezione dice che il recupero del database non è riuscito. Uno non è l'altro. Certo se il recupero fa esplodere l'ordine non verrà spedito. Questa è una relazione di causa ed effetto semmai. Ma non sono la stessa cosa.

Definisci sempre tutti i tuoi stati. vero e falso. Soffrirai di codice che non definisce completamente le cose. Ho a che fare con un sistema ora che definisce 2 variabili di stato come "sì" e vuote. Fai questo tipo di cazzate e ti occuperai sempre di codice incoerente e leggermente distorto per gestire "valori non di dominio".

Un giorno dovrai gestire quell'eccezione per quello che realmente è e scoprire improvvisamente che devi virtualmente riscrivere il codice di gestione dello stato di spedizione.

modifica

I'm not entirely sure I'm understand what your point of view here is

Restituisce un bool. Le eccezioni riguardano problemi che non consentiranno l'esecuzione o l'esecuzione del programma. Eccezioni di cattura per abilitare il grazioso errore del programma.

I need to know if the operation was successful as at a higher level, I mark this shipment as sent. ... its really never going to return false (because it will raise an exception) ... Because of this, should my method ExportShipments simply return void?

Quale addetto alle spedizioni ovunque, quando viene richiesto "è stato spedito?" Risposte, ho ricevuto un'eccezione databaseNotFound. O spedito - vero, o non spedito - falso.

Quindi presumi che non ci sarà mai uno stato "non spedito". Perché ti preoccupi di chiamare il metodo? Perché non si effettua una chiamata per verificare una connessione al database?

Quindi con una return void + una potenziale eccezione, il design del codice significa dire "se il database ha incasinato in qualche modo, non è stato spedito". Un problema con il database è un problema di implementazione tecnica che non ha assolutamente nulla a che fare con l'attività di spedizione. Dovresti codificare in termini di attività, che è la spedizione senza chiamare database.

modifica fine

    
risposta data 27.05.2017 - 01:22
fonte

Leggi altre domande sui tag