Zwracanie odpowiedzi serwisu :: 4programmers.net

0

Tak jak w temacie, co najlepiej zwracać, jeśli:

nie zostanie znaleziony obiekt o zadanym ID?
istnieje już obiekt o zadanym ID?

Prosiłbym o ocenę i porady odnośnie rozwiązania.

W poniższym kodzie chcę dodać wybrany Team do istniejącego już Season. Do kontrolera przychodzą ich Id, zawarte jako właściwości klasy AddSeasonTeamRequest. Zakładam, że ktoś przesłał do API jedno z nich niepoprawne (nieistniejące w bazie).

        [HttpPost]
        public async Task<ActionResult> AddTeam(AddSeasonTeamRequest request)
        {
            var result = await seasonsService.AddTeam(request.SeasonId, request.TeamId);

            if (!result)
                return BadRequest();

            return Ok();
        }

    public class Season
    {
        public int Id { get; private set; }
        // ...

        public virtual ICollection<SeasonTeam> Teams { get; set; }


        public void AddTeam(Team team)
        {
            if (Teams.Any(t => t.TeamId == team.Id))
                return;                                         // co należałoby zwrócić? Może boolean? Niektórzy rzucają nawet wyjątki.

            Teams.Add(new SeasonTeam()
            {
                TeamId = team.Id
            });
        }
    }

        public async Task<bool> AddTeam(int seasonId, int teamId)
        {
            var season = await context.Seasons
                            .Include(s => s.Teams)
                            .SingleOrDefaultAsync(i => i.Id == seasonId);

            if (season == null)
                return false;                           // co należałoby zwrócić? Może boolean? Niektórzy rzucają nawet wyjątki.

            var team = await context.Teams.SingleOrDefaultAsync(i => i.Id == teamId);

            if (team == null)
                return false;                          // co należałoby zwrócić? Może boolean? Niektórzy rzucają nawet wyjątki.

            season.AddTeam(team);

            context.Entry(season);
            await context.SaveChangesAsync();

            return true;
        }

3

Może coś w tym kierunku - klasa przeznaczona do obsługi błędów zawierająca podstawowe informacje typu: czy się udało, komunikat, potencjalnie dane, itd?

Functional C#: Handling failures, input errors

public class Result
{
    public bool Success { get; private set; }
    public string Error { get; private set; }
    public bool Failure { /* ... */ }
 
    protected Result(bool success, string error) { /* ... */ }
 
    public static ResultFail(string message) { /* ... */ }
 
    public static Result<T> Ok<T>(T value) {  /* ... */ }
}
 
public class Result<T> : Result
{
    public T Value { get; set; }
 
    protected internal Result(T value, bool success, string error)
        : base(success, error)
    {
        /* ... */
    }
}

0

Po pierwsze jaśniej będzie, jak będzie używać słowa webservice (service może oznaczać wewnętrzny element architektury)

Z jakiej akcji chcesz mieć te resultaty?
Jesli POST, to po poprawnym przebiegu 201 Created - jego brak to nie poszło.

https://www.restapitutorial.com/lessons/httpmethods.html

0

Też na początku myślałem że chodzi o HTTP Status Code, ale tu nie o to chodzi. — WeiXiao 3 minuty temu

O wiele bardziej niż gwałcony REST podoba mi się GraphQL, tam to jest porządnie rozwiązane - jest odpowiedzią na ubóstwo REST-a

0

Czy zwracanie obiektu takiej klasy jest powszechną praktyką? Przeglądając githuby i artykuły często widuję, iż ludzie zwracają zdefiniowane przez siebie wyjątki.
Poniżej mam kod, który ma na celu dodanie przez użytkownika recenzji do wybranego produktu. Czy o to mniej więcej chodzi, czy jest to poprawne pod względem "best practise"?

Kod w kontrolerze:

public async Task<ActionResult> AddReview(CreateReviewDto createReviewDto)
{
             var result = await productService.AddReview(createReviewDto);

             if (!result.Succeeded)
                         return BadRequest(new { errors = result.Errors });

            return Ok(result.Data);
}

Metoda serwisu, która wywoływana jest w kontrolerze:

public async Task<ReviewDto> CreateReview(CreateReviewDto createReviewDto)
{
             var product = await context.Products
                .Include(i => i.Reviews)
                .SingleOrDefaultAsync(i => i.Id == productId);

            if (product == null)
                return new Result<ReviewDto>("Product not found.", null);

            var user = await context.Users.SingleOrDefaultAsync(i => i.Id == userContext.GetCurrentUserId());

            var result = product.AddReview(user, createReviewDto.reviewText, createReviewDto.rating);
            
            if (!result.Succeed)
                return new Result<ReviewDto>(result.Errors, null);

            context.Entry(product);
            await context.SaveChangesAsync();

           return new Result<ReviewDto>(mapper.Map<ReviewDto>(review));
}

Metoda w Product, która sprawdza dane, tworzy i dodaje Review do kolekcji:

public Result<Review> AddReview(CreateReviewDto createReviewDto)
{
             if (rating > 5 || rating < 1)
                throw new ArgumentException(nameof(rating));        // wyjątek jest w tej sytuacji poprawny?

            if (string.IsNullOrEmpty(reviewText))
                return new Result<Review>("Review text cannot be null.", null);

            if (Reviews.Any(i => i.UserId == user.Id))
                return new Result<Review>("You have already rated this product.", null);

            var review = new Review(this, user, reviewText, rating);
            Reviews.Add(review);

            return new Result<Review>(review);
}

6

twoj_stary napisał(a):

Czy zwracanie obiektu takiej klasy jest powszechną praktyką?

Myślę, że powszechną w lepszych zespołach/projektach.

Przeglądając githuby i artykuły często widuję, iż ludzie zwracają zdefiniowane przez siebie wyjątki.

To nie ludzie, to wilki.

Poniżej mam kod, który ma na celu dodanie przez użytkownika recenzji do wybranego produktu. Czy o to mniej więcej chodzi, czy jest to poprawne pod względem "best practise"?

Parę rzeczy bym poprawił:

return new Result<ReviewDto>("Product not found.", null);

wywal ten null z konstruktora. W ogóle null staraj się unikać.

return new Result<ReviewDto>(result.Errors, null);

Tutaj też ten null zbędny.

I w ogóle bym new Result<ReviewDto> nigdzie w kodzie nie pisał, Result powinien mieć zdefiniowane operatory konwersję implicit na prawidłowy obiekt bądź informację o błędzie. Dzięki temu metoda może mieć sygnaturę np.: Result<ReviewDto>(), a wewnątrz jej zwracasz return new Review("ziemniaki"); albo return new Error("tak nie wolno!");.

0

@somekind: Dzięki za pomocne wskazówki. Jestem jeszcze ciekaw, jak podszedłbyś do problemu decydowania o zwracanym kodzie statusu przez kontroler w przypadku błędu zwróconego w Result?

1

No to trzeba jakoś zmapować. Jeśli będzie jakaś sprytna hierarchia tych errorów, np:

AuthenticationError
AuthorizationError
DataNotFound
UserError
GenericError

to się właściwie samo nasuwa jak to zmapować na HTTP i można mieć jedną metodę do tego.

Można też użyć enumów, ale moim zdaniem to słabe.

0

somekind napisał(a):

No to trzeba jakoś zmapować. Jeśli będzie jakaś sprytna hierarchia tych errorów, np:

AuthenticationError

AuthorizationError

DataNotFound

UserError

GenericError

to się właściwie samo nasuwa jak to zmapować na HTTP i można mieć jedną metodę do tego.

Można też użyć enumów, ale moim zdaniem to słabe.

Mógłbyś coś więcej podpowiedzieć odnośnie realizacji Twojej koncepcji?

2

No coś w tym stylu:

public class Result<TData>
{
    public ErrorBase Error { get; }
    public TData Data { get; }
    public bool IsError => Error != null;

    public Result(ErrorBase error)
    {
        Error = error;
    }

    public Result(TData data)
    {
        Data = data;
    }

    public Result<TResult> Then<TResult>(Func<TData, Result<TResult>> func)
    {
        return IsError ? Error : func(Data);
    }

    public T Match<T>(Func<ErrorBase, T> leftFunc, Func<TData, T> rightFunc)
    {
        return IsError ? leftFunc(Error) : rightFunc(Data);
    }

    public static implicit operator Result<TData>(ErrorBase error) => new Result<TData>(error);

    public static implicit operator Result<TData>(TData data) => new Result<TData>(data);

    public static implicit operator TData(Result<TData> result) => result.Data;
}

0

somekind napisał(a):

No coś w tym stylu:

public class Result<TData>
{
    public ErrorBase Error { get; }
    public TData Data { get; }
    public bool IsError => Error != null;

    public Result(ErrorBase error)
    {
        Error = error;
    }

    public Result(TData data)
    {
        Data = data;
    }

    public Result<TResult> Then<TResult>(Func<TData, Result<TResult>> func)
    {
        return IsError ? Error : func(Data);
    }

    public T Match<T>(Func<ErrorBase, T> leftFunc, Func<TData, T> rightFunc)
    {
        return IsError ? leftFunc(Error) : rightFunc(Data);
    }

    public static implicit operator Result<TData>(ErrorBase error) => new Result<TData>(error);

    public static implicit operator Result<TData>(TData data) => new Result<TData>(data);

    public static implicit operator TData(Result<TData> result) => result.Data;
}

Dzięki. Na podstawie Twoich wcześniejszych porad zrobiłem to podobnie do tego co napisałeś, i to jest spoko. Moja prośba dotyczyła rozjaśnienia kwestii mapowania tego na kody statusu HTTP w kontrolerze. Przykładowo modyfikujemy zasób. Może dojść do sytuacji, że nie odnajdziemy zasobu przez wysłanie złego ID (NotFound), albo przekażemy jakąś błędną wartość (BadRequest).

4

Jakoś tak:

return result.Match<IActionResult>(
        error => error switch
        {
            ValidationError _ => BadRequest(error),
            AuthorizationError _ => Unauthorized(error),
            DataNotFoundError _ => NotFound(error),
            _ => StatusCode(StatusCodes.Status500InternalServerError, error)
        },
        Ok
    );

0

somekind napisał(a):

Jakoś tak:

return result.Match<IActionResult>(
        error => error switch
        {
            ValidationError _ => BadRequest(error),
            AuthorizationError _ => Unauthorized(error),
            DataNotFoundError _ => NotFound(error),
            _ => StatusCode(StatusCodes.Status500InternalServerError, error)
        },
        Ok
    );

W jakiej warstwie/projekcie umieściłbyś klasę Result i powiązane z nią klasy błędów?
W jakich sytuacjach używałbyś tych klas? Zwracałbyś je tylko z serwisów do kontrolera, czy w metodach encji również?

3

Jakiejś takiej wspólnej-bazowej, dostępnej wszędzie.
Zwracałbym je prawdopodobnie z każdej publicznej metody w każdej warstwie, aby móc ładnie błędy przepychać z dołu do góry, bez potrzeby dziwnego mapowania.

0

@somekind: podany przez ciebie kod dotyczy .net framework czy core?

1

@kalimata:

Raczej oba, ale C# w wersji 8.

0

kalimata napisał(a):

@somekind: podany przez ciebie kod dotyczy .net framework czy core?

Nie sądzę, aby dla C# była różnica, na jakim frameworku pracuje.
@WeiXiao ma chyba rację, że to C# 8, w 7 zamiast switch expression trzeba by to zrobić przez statement:

return result.Match<IActionResult>(
        error => switch(error)
        {
            case ValidationError ve: 
                 return BadRequest(error);
            case AuthorizationError ar: 
                 return Unauthorized(error);
            case DataNotFoundError nf
                 return NotFound(error);
            default:
                 return StatusCode(StatusCodes.Status500InternalServerError, error);
        },
        Ok
    );

0

@somekind:
a do czego i w jaki sposób można wykorzystać metodę Then?

 public Result<TResult> Then<TResult>(Func<TData, Result<TResult>> func)
    {
        return IsError ? Error : func(Data);
    }

4

Żeby uniknać ifologi. Np. można mieć długi proces biznesowy zaimplementowany w ten sposób:

public Result<OrderId> ProcessOrder(OrderRequest request)
{
    return _customerService.CheckCustomer(order.Customer)
                  .Then(x => _paymentService.MakePayment(order.Payment))
                  .Then(x => _warehauseService.PackOrder(order))
                  .Then(x => _invoicingService.IssueInvoice(order))
}

Tak długo jak wywoływane metody będą zwracały jakiś Result<Coś>, tak długo można je tak sprytnie łączyć unikając pisania if (result.IsError) return result; po każdym wywołaniu.

0

@somekind: a jeśli chciałbym dostać wynik ze stronicowaniem czyli np jeszcze dodatkowe informacje o ilosci obiektow w wyniku itp to powinienem dodatkowo utworzyć sobie klase ktora mi opakuje rezultat i dopiero to zwracac w obiekcie Result?

np

public class Paged<T>
    {
        public  IEnumerable<T> Data { get;set; }
        public int NumberOfPages{get;set;}
        public int NumberOfElements{get;set;}
    }

i dopiero to zwracam w obiekcie Result

public Result<Paged<OrderResult>> ProcessOrder(OrderRequest request)
{
 ...

   Paged<OrderResult> orderresult = new Paged<OrderResult>();
orderResult.Data = data;
return orderresult;
}

1

Tak.

0

somekind napisał(a):

Żeby uniknać ifologi. Np. można mieć długi proces biznesowy zaimplementowany w ten sposób:

public Result<OrderId> ProcessOrder(OrderRequest request)
{
    return _customerService.CheckCustomer(order.Customer)
                  .Then(x => _paymentService.MakePayment(order.Payment))
                  .Then(x => _warehauseService.PackOrder(order))
                  .Then(x => _invoicingService.IssueInvoice(order))
}

Mam jeszcze pytania
Gdzie tu wstawić sekcję Match()?

no bo każda z tych metod może zwracać jakiś Error, czy kazdy Then powinien mieć soją sekcję Match?

i drugie pytanie. Czy metody serwisów w tym przypadku nie moga byc async?

1

kalimata napisał(a):

Gdzie tu wstawić sekcję Match()?

W warstwie odpowiedzialnej za przetworzenie obiektów na informacje dla użytkownika. W MVC czy innym WebAPI to będzie warstwa z kontrolerami.

no bo każda z tych metod może zwracać jakiś Error, czy kazdy Then powinien mieć soją sekcję Match?

O to chodzi, aby Match mapujący obiekt błędu na odpowiedź HTTP był jeden, bo ten sam typ obiektu błędu może być przecież zwracany z wielu różnych metod.

i drugie pytanie. Czy metody serwisów w tym przypadku nie moga byc async?

Mogą, tylko potrzeba więcej kodu:

public static class TaskExtensions
{
    public static async Task<Result<TOutput>> Then<TInput, TOutput>(this Task<Result<TInput>> task, Func<TInput, Result<TOutput>> func)
    {
        return (await task).Then(func);
    }

    public static async Task<Result<TOutput>> Then<TInput, TOutput>(this Task<Result<TInput>> task, Func<TInput, Task<Result<TOutput>>> func)
    {
        return await (await task).Then(func);
    }
}

0

@somekind: dziękuję.

W drugim fragmencie kodu:

public static async Task<Result<TOutput>> Then<TInput, TOutput>(this Task<Result<TInput>> task, Func<TInput, Task<Result<TOutput>>> func)
    {
        return await (await task).Then(func);
    }

kompilator podkreśla mi Then (ten w return) i daje komunikat:

Severity Code Description Project File Line Suppression State
Error CS0411 The type arguments for method 'Result<TInput>.Then<TResult>(Func<TInput, Result<TResult>>)' cannot be inferred from the usage. Try specifying the type arguments explicitly.

Możesz podpowiedzieć, co z tym zrobić?

Drugie pytanie, dlaczego oprogramowałeś 2 metody?
To jeszcze dorzucę trzecia kwestie.

somekind napisał(a):

return result.Match<IActionResult>(
        error => switch(error)
        {
            case ValidationError ve: 
                 return BadRequest(error);
            case AuthorizationError ar: 
                 return Unauthorized(error);
            case DataNotFoundError nf
                 return NotFound(error);
            default:
                 return StatusCode(StatusCodes.Status500InternalServerError, error);
        },
        Ok
    );
``

czy jeśli tych case będzie np 10 (i każda metoda kontrolera będzie tyle miała) to będzie w porządku, czy nie powinno się tak rozdrabniać błędów?

1

kalimata napisał(a):

kompilator podkreśla mi Then (ten w return) i daje komunikat:

Severity Code Description Project File Line Suppression State
Error CS0411 The type arguments for method 'Result<TInput>.Then<TResult>(Func<TInput, Result<TResult>>)' cannot be inferred from the usage. Try specifying the type arguments explicitly.

Możesz podpowiedzieć, co z tym zrobić?

Jakoś zniknęła jedna metoda z Result:

public async Task<Result<TResult>> Then<TResult>(Func<TData, Task<Result<TResult>>> func)
{
    return IsError ? Error : await func(Data);
}

Drugie pytanie, dlaczego oprogramowałeś 2 metody?

Pierwsza jest po to, aby móc wywołać metodę sync po async, druga po to, aby móc wywołać async po async.
A żeby zrobić async po sync, to wystarczy ta brakująca metoda, którą wkleiłem wyżej.

To jeszcze dorzucę trzecia kwestie.
czy jeśli tych case będzie np 10 (i każda metoda kontrolera będzie tyle miała) to będzie w porządku, czy nie powinno się tak rozdrabniać błędów?

Czemu rozdrabniać? Tych najsensowniejszych kodów HTTP do zwrócenia z API jest mniej więcej z 10.
Tyle, że oczywiście trzeba tu zrobić jakąś metodę pomocniczą, a nie kopiować tego do każdej akcji kontrolera. ;)

Zwracanie odpowiedzi serwisu

twoj_stary napisał(a):

somekind napisał(a):

somekind napisał(a):

somekind napisał(a):

kalimata napisał(a):

somekind napisał(a):

kalimata napisał(a):

somekind napisał(a):

kalimata napisał(a):

1 użytkowników online, w tym zalogowanych: 0, gości: 1

Praca dla programistów

Forum dyskusyjne

Sprawy administracyjne

O nas

Skontaktuj się z nami