Jak to przerobić funkcyjnie? cz. 2 czyli raczej: Dlaczego to przerobić funkcyjnie?

0

Aplikacja służy do synchronizacji danych pomiędzy dwoma rożnymi zewnętrznymi API.

Z definicji nie może to być czysto funkcyjnie: wszak zapisywanie do zewnetrznego API to będzie jakiś void albo w najlepszym wypadku Task.

No ale do pewnego momentu można pisać w miarę ładnie:

public async Task Synchronizuj()
{
    var daneZFoo = await Foo.PobierzDane();
    Bar.ZapiszDane(daneZFoo);

    var daneZBar = Bar.PobierzDane();
    await Foo.ZapiszDane(daneZBar);
}

(oczywiście upraszczam)

Ale teraz pojawiają się dwa wymogi, które wyraźnie utrudniają sprawy.

Wymóg 1: Aplikacja ma być odporna na błędy. Jeżeli dla przykładu w Foo mamy 10 dokumentów, z czego zapisywanie 5. w Bar rzuci wyjątkiem (jakimkkolwiek), to nie mamy prawa zrobić klasycznego "wycofania transakcji" - czyli finalnie nie są zapisane nawet te 4 dokumenty, których zapisanie się udało + wpis do logu. Wprost przeciwnie, nie tylko te 4 poprzednie dokumenty mają pozostać zapisane, ale co więcej, zamiast się crashnąć trzeba próbowac zapisywać dalej resztę dokumentów. Mało tego, potem trzeba próbować także przerzucić dane w drugą stronę, czyli z Bar do Foo. (Co z tego, że takie postępowanie może prowadzić do niespójności danych, i tak trzeba).

Wymóg 2: Z przyczyn wydajnościowych + aby uniknąć zbędnego wyścigu nie przerzucamy z powrotem z Bar do Foo danych, które przed chwilą przerzuciliśmy z Foo do Bar.

Tutaj wchodzą mutacje.

Bar.ZapiszDane musi zwrócić dokumenty, jakie zostały finalnie (w którejkolwiek podmetodzie) zapisane do Bar. A więc co? Funkcyjnie?

public async Task Synchronizuj
{
    var daneZFoo await Foo.PobierzDane();
    var zapisaneDoBar = Bar.ZapiszDane(daneZFoo);

    var daneZBar = Bar.PobierzDane(zapisaneDoBar);
    await Foo.ZapiszDane(daneZBar);
}

Nie - Bar.ZapiszDane przecież finalnie może rzucic AggregateException, wyliczającym wyjątki, które poszły gdzieś w środku niego, ale które zignorował. Nie zwracac wyjątków tez nie możemy, Synchronizuj() jednak musi wiedzieć, czy finalnie wszystko OK czy gdzieś sie jednak błędy pojawiły.

Rozwiązanie, do jakiego doszedłem, wygląda mniej więcej tak. W kodzie jakas metoda ze środka Bar.ZapiszDane:

private void ZapiszDokumenty(List<Dokument> doZapisania, List<string> zapisaneDokumenty)
{
    List<Exception> wyjatki = new List<Exception>{};

    foreach(var d in doZapisania)
    {
        try
        {
            ZapiszDokument(d, zapisaneDokumenty);
        }
        catch(Exception e)
        {
            Log.Błąd(e);
            wyjatki.Add(e);
        }
    }

    if(wyjatki.Any())
    {
        throw new AggregateException(wyjatki);
    }
    else
    {
        Log.Write("Wszystkie dokumenty zapisane prawidłowo.");
        ZapiszDatęOstatniejSkutecznejSynchronizacjiDokumentów(DateTimeOffset.Now); // potrzebne dla celów optymizacyjnych, NIE WOLNO zapisywać jeśli były błędy
    }
}

private void ZapiszDokument(Dokument doZapisania, List<string> zapisaneDokumenty)
{
    // tutaj pominięte wywołania zewnętrznego API; owo zewnętrzne API jest marnie udokumentowane, może rzucać np błędami walidacji
    zapisaneDokumenty.Add(doZapisania.numerDokumentu); // Jak wyjątek, to nie dojdziemy do tego punktu, i dobrze.
}

Oczywiście te try/catch się powtarzają w kółko i w kółko - wyabstrachowałem to (tylko nie chcę tu podawać jak wyłącznie z tego powodu, że post jest i tak dośc długi). Tutaj tylko wkleiłem niewyabstrachowane, by było wiadomo, jak to działa.

Bardziej mnie interesuje ta lista, przekazywana między metodami, do której rozmaite metody doklejają kolejne obiekty, by zapisać, co zrobiły. Chodzi mi o to, by nawet wyjątek nie był w stanie uniemozliwić zwrócenia tej listy - jeśli zapisalismy 9 dokumentów a 10. rzucił wyjatkiem, i tak musimy zwrócić, które dokumenty zapisalismy skutecznie.

Na upartego da sie to przerobić funkcyjnie: pewnie trzeba zbudować jakiegoś Eithera... cos na zasadzie:

public interface Either<T> {}

public Right<T> : Either<T>
{
    public T Value {get; private set;} 

    public Right(T value)
    {
        Value = value;
    }
}

public Left<T> : Either<T>, Exception
{
    public Left(Exception inner) : base(inner.Message, inner) {}
}

I niech ZapiszDokumenty zwraca listę takich Eitherów.

Pytanie moje brzmi: po co?

Tzn. być może od początku tak należało zrobić, ale teraz, kiedy to już wyabstrachowałem w inny sposób, czy jest sens tracić czas na przerabianie tego "funkcyjnie", byle by tylko uniknąć tego parametru - mutowalnej listy? (jest presja na jak najszybsze ukończenie wymaganej funkcjonalności = nie chcę przeznaczać czasu na przerabianie działającego kodu tak, by pasował do dogmatów takich, jak zakaz mutacji)

Jaka jest szkoda w tym, że metody dostają kolejny parametr - mutowalną listę - do której dopisują, co zrobiły?

O ile dla czytającego będzie jasne, że ten dodatkowy parametr do tego właśnie służy, wydaje mi sie, że nie ma szkody w tym, że mamy tutaj mutowanie.

To ostatnie zaś da się załatwić za pomocą konwencji nazewniczych.

Mógłby mi ktoś wytłumaczyć, dlaczego to, co zrobiłem teraz, jest szkodliwe?

Zazwyczaj starałem się unikać zbędnych mutacji, tym razem jednak musiałem sie chwile zastanowić, jak zrobić to, co było wymagane, więc w pierwszej chwili zaimplementowałem taki pomysł, jaki miałem - dopiero później przyszła refleksja z tym Eitherem.

4

czy jest sens tracić czas na przerabianie tego "funkcyjnie", byle by tylko uniknąć tego parametru - mutowalnej listy?

Próba pisania czysto funkcyjnie w języku do tego nieprzystosowanym (nie, funkcje wyższego rzędu to nie jest jeszcze "programowanie funkcyjne") imho nie ma sensu.

Jeśli uzyskałeś kod, który możesz pokazać drugiemu programiście i będzie on w stanie coś tam własnego dopisać, to nie ma czego dalej tykać :-P

3

wszak zapisywanie do zewnetrznego API to będzie jakiś void albo w najlepszym wypadku Task

Nie wiem czy w C# sa takie ortodoksyjne biblioteki ale jest jeszcze monada IO ;)

Wymóg 1: Aplikacja ma być odporna na błędy. Jeżeli dla przykładu w Foo mamy 10 dokumentów, z czego zapisywanie 5. w Bar rzuci wyjątkiem (jakimkkolwiek), to nie mamy prawa zrobić klasycznego "wycofania transakcji" - czyli finalnie nie są zapisane nawet te 4 dokumenty, których zapisanie się udało + wpis do logu. Wprost przeciwnie, nie tylko te 4 poprzednie dokumenty mają pozostać zapisane, ale co więcej, zamiast się crashnąć trzeba próbowac zapisywać dalej resztę dokumentów. Mało tego, potem trzeba próbować także przerzucić dane w drugą stronę, czyli z Bar do Foo. (Co z tego, że takie postępowanie może prowadzić do niespójności danych, i tak trzeba).

List[Try[Unit]] / List[Either[E, V]] - zreszta na cos podobnego wpadles w dalszej czesci.

Ja bym np. w Javie jakos na sile funkcyjnie nie pisal bo sie zwyczajnie nie da (i w C# pewnie lepiej nie jest ale sa chcoiaz withery). Natomiast w odpowiednim jezyku jak np. Kotlin czy Scala to po prostu po jakims czasie intuicyjnie idzie w pierwszym rzucie a nie ze dopiero po namyslach na to wpadasz. No bo w Scali np. Try { expr } to nie jest jakas imperatywna instrukcja a wartosc - po prostu robisz (bez dokladania biblioteki!) dokumenty.map(dokument => Try { zapiszDokument(dokument) }) i to jest typu List[Try[Unit]]


EDIT:

Bardziej mnie interesuje ta lista, przekazywana między metodami, do której rozmaite metody doklejają kolejne obiekty, by zapisać, co zrobiły. Chodzi mi o to, by nawet wyjątek nie był w stanie uniemozliwić zwrócenia tej listy - jeśli zapisalismy 9 dokumentów a 10. rzucił wyjatkiem, i tak musimy zwrócić, które dokumenty zapisalismy skutecznie.

A koniecznie ta lista musi byc przekazywana jako argument? Nie moze byc tak, ze lokalnie robisz sobie liste (nawet niech bedzie mutowalna) i potem zwracasz ja jako wynik?

private ImmutableList<Dokument> ZapiszDokumenty(List<Dokument> doZapisania)
{
    List<string> zapisaneDokumenty = new List<Dokument>{};

   ...

  return zapisaneDokumenty.asImmutable();

EDIT2:
Ahaa.. Bo tutaj jest cos takiego:

if(wyjatki.Any())
    {
        throw new AggregateException(wyjatki);
    }

Pytanie dlaczego? Nie da sie tego zrobic inaczej? :) Np. zapisaneDokumenty.Length != doZapisania.Length w funkcji wolajacej? Albo zwrocenie pary (zapisaneDokumenty, wyjatki) i sprawdzenie tam wyzej (gdzie i tak masz try-catcha pewnie?) czy aby czasem wyjatek sie nie pojawil?

0

Moje (może nieudolne) wyabstrachowanie try/catchów:

public class ExceptionTracker : IDisposable
{
    private List<Exception> exceptions = new List<Exception> { };
    public bool IsOk => exceptions.Count == 0;

    public void AddLoggedException(Exception e)
    {
        exceptions.Add(e);
    }

    public void Dispose()
    {
        if(exceptions.Count == 1)
        {
            throw new LoggedException(exceptions[0]);
        }
        else if(exceptions.Count > 1)
        {
            throw new LoggedException(new AggregateException(exceptions));
        }
    }
}

public class LoggedException : Exception
{
    public int ErrorCode { get; set; }

    public LoggedException(string message, int errCode) : base(message)
    {
        ErrorCode = errCode;
    }

    public LoggedException(string message, int errCode, Exception innerException) : base(message, innerException)
    {
        ErrorCode = errCode;
    }

    public LoggedException(Exception innerException) : base(innerException.Message, innerException)
    {
        ErrorCode = 0;
    }

    public LoggedException(string message, Exception innerException) : base(message, innerException)
    {
        ErrorCode = 0;
    }

}

I dalej mnóstwo wariacji na ten sam temat:

public static void CatchAndLog(Action action, ExceptionTracker et, string additionalMessage)
{
    try
    {
        action();
    }
    catch (LoggedException e)
    {
        et.AddLoggedException(e);
    }
    catch (Exception e)
    {
        Write(0, LogLevels.Error, $"{additionalMessage} {e}");
        et.AddLoggedException(e);
    }
}

public static async Task CatchAndLog(Func<Task> action, ExceptionTracker et)
{
    try
    {
        await action();
    }
    catch (LoggedException e)
    {
        et.AddLoggedException(e);
    }
    catch (Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
        et.AddLoggedException(e);
    }
}

public static async Task<T> CatchAndLog<T>(Func<Task<T>> action, ExceptionTracker et, T retOnFail, int warnLogId, string warnLogMsg)
{
    try
    {
        return await action();
    }
    catch(LoggedException e)
    {
        et.AddLoggedException(e);
        Write(warnLogId, LogLevels.Warning, warnLogMsg);
        return retOnFail;
    }
    catch(Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
        et.AddLoggedException(e);
        Write(warnLogId, LogLevels.Warning, warnLogMsg);
        return retOnFail;
    }
}

public static async Task CatchLogAndIgnore(Func<Task> action)
{
    try
    {
        await action();
    }
    catch (LoggedException)
    {

    }
    catch (Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
    }
}

public static async Task CatchLogAndThrow(Func<Task> action)
{
    try
    {
        await action();
    }
    catch(LoggedException)
    {
        throw;
    }
    catch(Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
        throw new LoggedException(e);
    }
}

public static void CatchLogAndThrow(Action action)
{
    try
    {
        action();
    }
    catch (LoggedException)
    {
        throw;
    }
    catch (Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
        throw new LoggedException(e);
    }
}

public static T CatchLogAndThrow<T>(Func<T> action)
{
    try
    {
        return action();
    }
    catch (LoggedException)
    {
        throw;
    }
    catch (Exception e)
    {
        Write(0, LogLevels.Error, e.ToString());
        throw new LoggedException(e);
    }
}

Dzięki temu metody z mojego OP piszą się dużo zwięźlej:

public async Task Synchronizuj
{
    var zapisaneDoBar = await Log.CatchLogAndIgnore(async () => {
        await Log.CatchLogAndIgnorevar daneZFoo await Foo.PobierzDane();
        return Bar.ZapiszDane(daneZFoo);
    }, new List<string>{});

    await Log.CatchLogAndIgnore(async () => {
        var daneZBar = Bar.PobierzDane(zapisaneDoBar);
        await Foo.ZapiszDane(daneZBar);
    }
}

A niżej:

private void ZapiszDokumenty(List<Dokument> doZapisania, List<string> zapisaneDokumenty)
{
    using(var et = new ExceptionTracker()
    {
        foreach(var d in doZapisania)
        {
            Log.CatchAndLog(() => {
                ZapiszDokument(d, zapisaneDokumenty);
            }, et);
        }
        
        if(et.IsOk)
        {
            Log.CatchAndLog() => {
                ZapiszDatęOstatniejSkutecznejSynchronizacjiDokumentów(DateTimeOffset.Now);
            }, et);
        }
    }
}
0

A gdyby po prostu łapać wyjątki nisko, opakowywać w Eithery, te zaś zwracać z każdej funkcji w górę aż do funkcji, która wyjątek zaloguje, a poza tym zdecyduje, czy ponowić przetwarzanie tego dokumentu, czy nie, to tego kodu byłoby 4 razy mniej i żadne abstrahowanie łapania wyjątków nie byłoby potrzebne.

0

"łapać wyjątki nisko" - to prawie każdą instrukcję trzeba w try/catch opakować? A już na pewno każde wywołanie słabo udokumentowanego zewnętrznego API.

0
somekind napisał(a):

A gdyby po prostu łapać wyjątki nisko, opakowywać w Eithery, te zaś zwracać z każdej funkcji w górę aż do funkcji, która wyjątek zaloguje, a poza tym zdecyduje, czy ponowić przetwarzanie tego dokumentu, czy nie, to tego kodu byłoby 4 razy mniej i żadne abstrahowanie łapania wyjątków nie byłoby potrzebne.

No właśnie ciekaw jestem jak wygląda taki realny kod na najwyższym poziomie? Coś takiego?

Either<DupaException, Either<SQLException, Either<RPCException, Either<EDivisionByZero, Either<UserNotFoundException, User>>>>> maybeUser = getBasketOwner();

Przykład wg IBMa niby to potwierdza: https://www.ibm.com/developerworks/library/j-ft13/index.html

1
kmph napisał(a):

"łapać wyjątki nisko" - to prawie każdą instrukcję trzeba w try/catch opakować? A już na pewno każde wywołanie słabo udokumentowanego zewnętrznego API.

Ciężko mi sobie wyobrazić, aby musiał być więcej niż jeden try/catch na klasę łączącą się z danym API.
Ewentualnie jeśli każdy endpoint tamtego API ma swoją klasę, to można zrobić dekorator na wszystkie obiekty uderzające w to API i konwertujące Exception na odpowiedni Either, ale to też będzie jeden try/catch.

vpiotr napisał(a):

No właśnie ciekaw jestem jak wygląda taki realny kod na najwyższym poziomie? Coś takiego?

Either<DupaException, Either<SQLException, Either<RPCException, Either<EDivisionByZero, Either<UserNotFoundException, User>>>>> maybeUser = getBasketOwner();

Dziwny pomysł... Po co zagnieżdżać te Eithery, skoro jest jeden typ dla prawidłowego wyniku? Either<Error, User> w zupełności wystarczy.

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