Głębokie zagnieżdżenia typów w deklaracjach metod

Cześć,
tworzę właśnie API dla nauki i całkiem nieświadomie napisałem takie coś:

public interface IUsersRepository
{
  public Task<Result<IEnumerable<User>>> GetUsers();
}

Czy tak głębokie zagnieżdżenia typów są uznawane za złą praktykę? Nie mogę się tego doszukać w internecie, ale mam wrażenie, że nie wygląda to za zgrabnie :)

Result wynika z zastosowania Result Pattern:

public class Result
{
  public bool IsSuccess { get; }
  public bool IsFailure => !IsSuccess;

  public Error Error { get; }
  ...
}

public class Result<T> : Result
{
  public T Value { get; }
  ...
}

Nie piszę w C# ale nie miałbym z tym problemu.

W przypadku ogólnym tak.
W przypadku szczególnym np Twoim można uzasadnic. Task jest wymagany przez jezyk, Result przez Twoj framoerk Userzy to faktyczne dane. Jezeli nie bedziesz leniwy i result bedzie uzywany zgodnie z przeznaczeniem nie wpakujesz sie w wieksze kłopoty

Ponieważ user jest dana, a Taska nie kontrolujesz to jedynym czym mozesz manewrowa to Result, i Enumerable. Teoretycznie mozesz napisać sobie klase ResultCollection<T>. odpanie jedna prara klamerek. Na duża skale moze warte świeczki.

A jaką masz alternatywe?

Jak używasz c# 12 to wprowadzili type aliasy. Nie wiem czy to dobra praktyka ale zawsze jakaś alternatywa

Z innej beczki: Result to nie powinien miec metod getError / getValue (czyli domyslam sie, ze uzywasz tego tak : if (r.isSuccess) r.Value), a raczej to powinny byc map, flatMap, fold

Dlaczego? Zalozmy ze masz funkcje:
foo: () -> Result<X>
bar: X -> Result<Y>
buzz: Y -> Result<Z>
fin: Z -> FinalValue

W jaki sposob to skomponujesz? Bo ja tak:

foo()
  .flatMap(bar)
  .flatMap(buzz)
  .map(fin)

Albo tak:

for {
  x <- foo
  y <- bar(x)
  z <- buzz(y)
  v = fin(z)
} yield v

To wyzej, to skladnia Scalowa, ale AFAIK w C# mozna to osiagnac za pomoca LINQ - cc @KamilAdam jak to sie robi?

stivens napisał(a):

w C# mozna to osiagnac za pomoca LINQ

from x in foo()
from y in bar(x)
from z in buzz(y)
select fin(z);

Możesz też zrobić class Users : ReadOnlyCollection<User> i zwracać Task<Result<Users>>

Możesz też zrobić class Users : ReadOnlyCollection<User> i zwracać Task<Result<Users>>.

To tez jest jakieś rozwiazanie. Taki pattern jak proponujez skaluje sie lepiej niz IEnumerable, bo łatwiej go rozszerzyć. Wadą jest to że musiałby pisac klase dla każdej kolekcji i 80% z nich miała pustą implementacje, dziedziczacą po bazówke. Pisanie ich jest upierdliwe samo w sobie, dlatego wieszkość osob tego nie robi, mimo że to dobra praktyka.

_flamingAccount napisał(a):

Ponieważ user jest dana, a Taska nie kontrolujesz to jedynym czym mozesz manewrowa to Result, i Enumerable. Teoretycznie mozesz napisać sobie klase ResultCollection<T>. odpanie jedna prara klamerek. Na duża skale moze warte świeczki.

A co, jeśli zewnętrzna metoda będzie zwracała Result<string>? Będzie przepakowywał errory z ResultCollection zamiast po prostu zwrócić?

A co, jeśli zewnętrzna metoda będzie zwracała Result<string>? Będzie przepakowywał errory z ResultCollection zamiast po prostu zwrócić?

Nie rozumiem pytania. Jak chcesz wzracać wynik wyniku wyniku itp. mozesz napisac Result<Result<Result.. ale to głupie. Jezli jednak faktycznie bedzie miał taki przypadek, to moze napisac metode w tej kolekcji która przepakuje zagnieżdzone wyniki i wystarczy zrobic to raz na cały projekt.

to czego nie rozumiem:
zewnętrzna metoda będzie zwracała Result<string>? i dlaczego trzeba coś przepakowywać

W zasadzie pytanie - dokumentacja MS jest dla mnie niezrozumiła, ale po sygnaturach metod wnioskuję,
że Task w zasadzie posiada funkcjonalność "jak" Result ( w sensie, może być failed, z podaną przyczyną).

Przy okazji ten Result w oryginalnym poście strasznie źle wygląda -> w sensie, że zawsze jest pole Error, nawet jak to success.

Qbelek napisał(a):

stivens napisał(a):

w C# mozna to osiagnac za pomoca LINQ

Kopiuj

from x in foo()
from y in bar(x)
from z in buzz(y)
select fin(z);

A czat mówi iż bez linqu to będzie:

var result = foo
    .SelectMany(x => bar(x)
        .SelectMany(y => buzz(y)
            .Select(z => fin(z))));

To dopiero intuicyjne nazwy (Select zamiast map, i SelectMany zamiast flatmap), co nie @somekind ?

KamilAdam napisał(a):

To dopiero intuicyjne nazwy (Select zamiast map, i SelectMany zamiast flatmap), co nie @somekind ?

To akurat wynika z projektu. LINQ nie było robione aby pisać monad comprehension - tylko "zapytania" jak w SQL.
To, że można to wykorzystać do monad to raczej nieprzewidziane "nadużycie"
Ale zawsze mieć lepiej takie LINQ niż nic jak w JAVIE, albo korutyńskie g**no jak w kotlinie.

_flamingAccount napisał(a):

to czego nie rozumiem:
zewnętrzna metoda będzie zwracała Result<string>? i dlaczego trzeba coś przepakowywać

W sensie, że ResultCollection to coś innego niż Result<Collection>, więc jeśli wewnętrzna metoda zwróci error, to nie będzie się go dało po prostu zwrócić, trzeba będzie najpierw przepakować z ResultCollection.Error do Result.Error. Czyż nie?

KamilAdam napisał(a):

To dopiero intuicyjne nazwy (Select zamiast map, i SelectMany zamiast flatmap), co nie @somekind ?

One nie są zamiast map i flatMap, bo te w ogóle nie funkcjonują w ekosystemie. Co zrobi Select wiadomo od razu, nie trzeba się zastanawiać.

jarekr000000 napisał(a):

W zasadzie pytanie - dokumentacja MS jest dla mnie niezrozumiła, ale po sygnaturach metod wnioskuję,
że Task w zasadzie posiada funkcjonalność "jak" Result ( w sensie, może być failed, z podaną przyczyną).

Nie, absolutnie nie. Task to operacja asynchroniczna, zawiera informacje jedynie o tym, czy operacja jako taka się wykonała (czyli nie poleciał nieobsłużony wyjątek). Nie da się w nim przekazać informacji o błędzie tak jak w Result.

Przy okazji ten Result w oryginalnym poście strasznie źle wygląda -> w sensie, że zawsze jest pole Error, nawet jak to success.

To prawda, ale to najpopularniejsza implementacja (na hinduskich blogach). Zrobienie tego dobrze wymaga bardzo dużych zdolności programistycznych. A poza tym i tak by Ci się nie spodobało, więc nie warto. :P

@somekind Ja wyobrazlem to sobie tak: ResultCollection<T> : Result<IEnumerable<T>. Wszystko powinno banglać. Jedynym celem tej konstukcji jest obnizenie ilosci klamerek. Jeżeli typ trzeba podać dziesiatki, setki czy tysiace razy(np. jezeli nie uzywasz var), to moze jest to warte świeczki.

Tasków użyłem, bo metody w moim repozytorium są asynchroniczne. Na jednych stronach piszą, że interfejsy nie powinny narzucać implementacji, więc Tasków nie powinno się zwracać (bo w C# narzucają one asynchroniczność), ale na drugich znowu sugerują, że jest to OK... jak żyć :)

Jeśli chodzi o te nieszczęsne Result, to szczerze mówiąc, jest to pierwszy projekt, w którym staram się zastosować ten wzorzec. Do tej pory rzucałem wszędzie wyjątkami typu ValidationException lub NotFoundException, ale przeczytałem, że taka praktyka, szczególnie w ASP.NETcie jest bardzo kosztowna, więc chciałem sprawdzić, czy z Result Pattern będzie lepiej.

Poniżej moja implementacja (zaczerpnięta z filmów pana Milana Jovanovica ( ) ):

public class Error
{
    public Error(string code, string? description = null, ErrorType type = ErrorType.ValidationError)
    {
        Code = code;
        Description = description;
        Type = type;
    }

    public enum ErrorType
    {
        ValidationError,
        NotFound,
        Other
    }

    public string Code { get; }
    public string? Description { get; }
    public ErrorType Type { get; }

    public static Error None = new Error(string.Empty);
}

public class Result
{
    public Result(bool isSuccess, Error error)
    {
        if (isSuccess && error != Error.None ||
            !isSuccess && error == Error.None)
        {
            throw new ArgumentException("Invalid error arguments!");
        }

        IsSuccess = isSuccess;
        Error = error;
    }

    public bool IsSuccess { get; }
    public bool IsFailure => !IsSuccess;
    public bool IsNotFound => IsFailure && Error.Type == Error.ErrorType.NotFound;

    public Error Error { get; }

    public static Result Success()
    {
        return new Result(true, Error.None);
    }

    public static Result Failure(Error error)
    {
        return new Result(false, error);
    }

    public static implicit operator Result(Error error)
    {
        return Failure(error);
    }
}

public class Result<TValue> : Result
{
    private TValue? _value;

    public Result(TValue? value, bool isSuccess, Error error)
        : base(isSuccess, error)
    {
        _value = value;
    }

    public TValue Value
    {
        get
        {
            if (IsFailure)
                throw new InvalidOperationException("Cannot access value of a failed result!");

            return _value!;
        }
    }

    public static Result<TValue> Success(TValue value)
    {
        return new Result<TValue>(value, true, Error.None);
    }

    public static new Result<TValue> Failure(Error error)
    {
        return new Result<TValue>(default, false, error);
    }

    public static implicit operator Result<TValue>(Error error)
    {
        return Failure(error);
    }

    public static implicit operator Result<TValue>(TValue value)
    {
        return Success(value);
    }
}

public class ResultCollection<T> : Result
{
    private IEnumerable<T>? _list;

    public ResultCollection(IEnumerable<T>? list, bool isSuccess, Error error)
        : base(isSuccess, error)
    {
        _list = list;
    }

    public IEnumerable<T> List
    {
        get
        {
            if (IsSuccess)
                return _list!;
            else
                throw new InvalidOperationException("Cannot access value of a failed result!");
        }
    }

    public static ResultCollection<T> Success(IEnumerable<T> list)
    {
        return new ResultCollection<T>(list, true, Error.None);
    }

    public static new ResultCollection<T> Failure(Error error)
    {
        return new ResultCollection<T>(null, false, error);
    }

    public static implicit operator ResultCollection<T>(List<T> list)
    {
        return Success(list);
    }

    public static implicit operator ResultCollection<T>(PaginatedList<T> list)
    {
        return Success(list);
    }

    public static implicit operator ResultCollection<T>(Error error)
    {
        return Failure(error);
    }
}

... ale zrobił się z tego taki bałagan, że chyba jednak wrócę do swoich wyjątków :)

wywal te result collection bo to jakas fanaberia, a reszta jest w miare

mi tam zazwyczaj wystarcza taki result

public class Result<T>
{
    private Result() { }

    public bool Success { get; private set; }

    public string? Message { get; private set; }

    public T? Value { get; private set; }

    public static Result<T> Error(string error)
    {
        return new Result<T>()
        {
            Success = false,
            Message = error,
            Value = default(T)
        };
    }

    public static Result<T> Ok(T val)
    {
        return new Result<T>()
        {
            Success = true,
            Message = "",
            Value = val
        };
    }
}

bbhzp napisał(a):

Tasków użyłem, bo metody w moim repozytorium są asynchroniczne. Na jednych stronach piszą, że interfejsy nie powinny narzucać implementacji, więc Tasków nie powinno się zwracać (bo w C# narzucają one asynchroniczność), ale na drugich znowu sugerują, że jest to OK... jak żyć :)

1.1 .Typowy bloger programowania zakłada strony z poradami jak programowć gdzieś tak 3 tygodnie po tym jak nauczy się programować w HTML, dlatego szczególnie w programowaniu nie warto się przejmować wszystkim co gdzieś piszą.
2.2. Task nie narzuca żadnej asynchroniczności w implementacji -> masz metodę Task.FromResult

Jeśli chodzi o te nieszczęsne Result, to szczerze mówiąc, jest to pierwszy projekt, w którym staram się zastosować ten wzorzec. Do tej pory rzucałem wszędzie wyjątkami typu ValidationException lub NotFoundException, ale przeczytałem, że taka praktyka, szczególnie w ASP.NETcie jest bardzo kosztowna, więc chciałem sprawdzić, czy z Result Pattern będzie lepiej.

2.1 Result to nie jest żaden pattern. To po prostu klasa, którą można napisać raz i używać. Zaiste nie ogarniam dlaczego nie wziąć tego z jakiejś biblioteki, nie wierze, że nie ma.
2.2. Napisanie tego, tak, żeby było bezpieczne i wygodne w użyciu, może nie jest rocket science, ale wymaga trochę klepania.
Dorzucanie kolejnych, coraz bardziej kulawych implementacji - jak w tym wątku nie pomaga. (Dobre ćwiczenie - fakt).
Z drugiej strony szukając na szybko w internecie nie znalazłem naprawdę sensownego Result w C#
Najbliżej (na pierwszej stronie w google) to to https://github.com/KeRNeLith/Here/blob/master/src/Here/Result/README.md

WeiXiao napisał(a):

Kopiuj

public class Result<T>
{
    private Result() { }

    public bool Success { get; private set; }

    public string? Message { get; private set; }

    public T? Value { get; private set; }

    public static Result<T> Error(string error)
    {
        return new Result<T>()
        {
            Success = false,
            Message = error,
            Value = default(T)
        };
    }

    public static Result<T> Ok(T val)
    {
        return new Result<T>()
        {
            Success = true,
            Message = "",
            Value = val
        };
    }
}

Ło, panie, a kto panu tak spierdolił? Settery w result? Serio?

Fajny też ten bool Success. Dzięki temu można nawet dodać kolejny konstruktor, aby osiągnąć idealny stan "Błąd - operacja ukończona pomyślnie."

A poza tym, skoro nie wiadomo czy Value jest ustawione to może powinno być getValue zwracające Result<Value> (bo przecież może się nie powieść).

Ale wy serio nie widzicie teho jaki ten kod jest durny? Na kiego grzyba tam ten bool?

@jarekr000000 o ze ten kon mozna napisać lepiej, isSucces wyliczać a nie zapisywać, zrobic wszystko readonly itp. to jedno.

Za to sam patten z metodami statystycznimi zamiast publicznych konstruktrów ma sens, argument o dodawniu konstuktorow w sytuacji gdy nie robi sie kontruktorow sensu nie ma.