Jak sensownie zaimplementować maszynę stanów?

Wg mnie wewnętrznie obiekty mogą sobie być w różnych stanach, jednak to, w jaki sposób dane obiekty współpracują ze sobą, to trochę co innego. Możliwe, że nie ma sensu tworzyć globalnie stanów, w których jest cała aplikacja (stany mogą być poukrywane w poszczególnych obiektach).

Jeśli obiekt A jest zależny od obiektu B, to niekoniecznie musi znać jego przestrzeń stanów. B może reprezentować pewną akcję (np. przygotowanie dokumentu) i może obiekt A jest zainteresowany tylko tym, czy się powiodła akcja albo jaką wartość finalnie zwróciła (albo jaki błąd).

Plus wydaje mi się, że dyskusja dotyczy częściowo maszyn stanów, a częściowo architektury opartej o eventy. Może to właśnie o eventach powinniśmy więcej pisać.

Ile będzie trwało zaimplementowanie prostego wymagania biznesowego "wyślij email do autora wniosku przy wywołaniu cancel na dowolnym ze stanów"?

No to łapiemy globalnie ten event cancel i go obsługujemy.

@_flamingAccount

Graf to dobry punkt wyjścia ale to nie pełny model. Graf w nie reprezentuje ani stanu ani algorytmu, a maszna stanów ma oba

Do reprezentacji aktualnego stanu (wierzchołka w grafie) potrzebujesz 1 pola. Algorytm poruszania się po grafie, to 1 linijka mieszcząca się na standardowym ekranie.

Minute, 5min? znajdujesz klase reprezentujaca anulowanie, do pisujesz wysłanie maila.

NIe bardzo, bo przy 30 stanach masz 30 Klas, z których każda ma własną implementację. Akcja X może powodować zmian A -> B, C->D.

@WeiXiao

switch statement

Tak, można mieć switch po typach sealed. O punkt lepiej.

@_flamingAccount

Kolejne mało metytoryczne pytanie.

Moim zdaniem bardzo merytoryczne - podatność na zmiany, to jedno z podstawowych kryteriów jakości kodu.

Argumentacja ze stan tez jest grafem mnie nie przekona, bo nie zaimplemetujesz 4 miliardow mozliwych roznic w intach jako graf.

Co to wnosi do dyskusji? Wiadomo, że kalkulator działający na int, czy float dałoby się teoretycznie zapisać jako graf stanów. Wiadomo też, że to bez sensu. Hindusi aż tak tani nie są.

Jeżeli zamiast metod miało byc którego stanu uzyc, przed pobraniem danych. to to jest głupie. Nie da sie, najpiew pobierasz potrzebna informacje potem decydujes z definicji.

No to jest głupie, dlatego to krytykuję. Pobieram jakieś dane, odczytuję stan, pakuję to w odpowiadający mu wrapper z metodami możliwymi do wywołania i go zwracam. No i w jaki sposób metoda która tego obiektu potrzebuje, ma wiedzieć co to za typ specyficzny?

@LukeJL

Wg mnie wewnętrznie obiekty mogą sobie być w różnych stanach, jednak to, w jaki sposób dane obiekty współpracują ze sobą, to trochę co innego. Możliwe, że nie ma sensu tworzyć globalnie stanów, w których jest cała aplikacja (stany mogą być poukrywane w poszczególnych obiektach).

No możliwe. Można przecież taki wniosek urlopowy przypisywać do stanu na podstawie daych w nim zawartych, według jakiejś reguły, np. "jeżelii data zatwierdzenia przez kierownika not null, to zatwierdzony". Zwykle prowadzi to do wypasionej drabinki if'ów, ale są też przypadki, w których taka prosta walidacja wystarczy.

No to łapiemy globalnie ten event cancel i go obsługujemy.

Owszem, jeżeli ten event istnieje i jest jakieś miejsce w którym da się go przechwycić globalnie. W moim przekonaniu, mając 30 stanów, 30 klas je reprezentujących jest też 30 miejsc w których należałoby dopisać wyślijMaila()

No i właściwie jaki macie problem z implementacją, którą proponuję? jedna funkcja eval(stan, akcja) -> stan? Jakie niedostatki tutaj widzicie i dlaczego zrobienie obiektowej struktury klas niby jest lepsze?

NIe bardzo, bo przy 30 stanach masz 30 Klas, z których każda ma własną implementację. Akcja X może powodować zmian A -> B, C->D.

Nie rozumiem Cie...
albo: zafixowałeś sie na tym grafie tak bardzo że nie do puszczasz do siebie informacji. Dostałeś nawet insturkcje jak dodac kolejny stan jezli masz zwalony model i to ignorujesz.
albo: mieszasz tak wiele różnych tematów naraz ze nie ma sznas w formie pisanej sie z tobą dogadać.
albo: bronisz chochoła, wymyśliłes sobie głupi przykład i ududowaniasz ze jest głupi, super fajnie przestań tak robić,
albo: po prostu nie potrafisz programować obiektowo? Twoje przykłady by na to wskazywały
albo: nowinka z przed 5 lat z Net, nie dostarła jeszcze do JVM, z przerzucanie pracy z progamisty na kompilator to nie znany koncept i nie wiesz ze sie da i to działa.
albo: po trochu wszystko naraz.

Co to wnosi do dyskusji? Wiadomo, że kalkulator działający na int, czy float dałoby się teoretycznie zapisać jako graf stanów. Wiadomo też, że to bez sensu. Hindusi aż tak tani nie są.

Miało uniknać idiotycznych kometarzy, ale musialeś. Jak sam argumentujesz, stan twojej aplikacji i stan maszyny stanów to kwestie rozłaczne.

Do reprezentacji aktualnego stanu (wierzchołka w grafie) potrzebujesz 1 pola. Algorytm poruszania się po grafie, to 1 linijka mieszcząca się na standardowym ekranie.

Dyskuja z Toba nie ma sensu bo ignorujesz dostarczane do Ciebie informacje, 3? raz ignorujesz informacje ze stan appliakcji i maszy stanów to rozłaczne kwestie oraz nie wiem ile razy juz z ignorwałes wzmanki o warunkach przejscia/algorytmnie. Twoje rozumowanie ma olbrzymie luki, ale nie trzeba sie nimi przejmowac gdy w kólko powtarza sie to samo ignorujac rozmówców...

Moim zdaniem bardzo merytoryczne - podatność na zmiany, to jedno z podstawowych kryteriów jakości kodu.

Mam w pracy kolege co uzywa podbnych argumentow i tez nikt go nie rozumie i dogadać sie z nikim nie moze. To co robisz nie jest merytoryczne.

Ech...

@somekind wrzuciłbyś jakiś prosty przykład? Zainteresowało mnie Twoje podejście ale nie rozumiem jakby to miało działać np. dostajemy request, że chcemy anulować wniosek o ID: 123. Skąd kompilator ma wiedzieć czy ten wniosek można anulować skoro na etapie kompilowania nie ma takiej informacji?

Tutaj znalazłem nową libkę, którą ktoś wrzucił na reddita RUST
https://github.com/eboody/statum

Posiada validacje na etapie kompliacji czyli się da. Ale nadal bez ifów się nie obędzie w przypadku czytania z DB w przykładzie w README jest np:

    if self.state != "review" {
        return Err(Error::InvalidState);
    }

Zupełnie przypadkiem trafiłem na występ Sławka Sobótki, który dotyczył modelowania procesu obiegu dokumentu, czyli w sumie to o czym tutaj była mowa od początku.

Tam w pewnym momencie pojawiło się wymaganie dotyczące konfigurowalności takiego procesu, więc naturalnie skończyło się na wersji podobnej do tej sugerowanej tutaj przez @piotrpo, gdzie klasa reprezentująca stan przechowywała możliwe przejścia wraz z predykatami i akcjami wykonywanymi po udanej zmianie stanu (znacznik czasowy - 44:50). Całość konfigurowalna z kodu.

Pokazane zostało również podejście:

• z jedną klasą mającą metody na wszystkie możliwe akcje do wykoanania, a w nich ifowanie (znacznik czasowy - 11:00)
• klasa per stan dokumentu (znacznik czasowy - 41:55), ale trochę inne niż to proponowane tutaj przez @somekind, bo tam klasa dokumentu ma w sobie obiekt reprezentujący stan na którym wykonywane są akcje

Ja osobiście szczerze mówiąc gdybym implementował coś podobnego to pewnie zaczął bym od:

• jednej klasy z metodami określającymi akcje biznesowe i ifami
• dopiero gdyby proces obiegu się komplikował, albo miał być konfigurowalny to bym refaktorował w kierunku innych podejść

anonimowy napisał(a):

@somekind wrzuciłbyś jakiś prosty przykład? Zainteresowało mnie Twoje podejście ale nie rozumiem jakby to miało działać np. dostajemy request, że chcemy anulować wniosek o ID: 123. Skąd kompilator ma wiedzieć czy ten wniosek można anulować skoro na etapie kompilowania nie ma takiej informacji?

Kompilator nie będzie wiedział, czy wniosek 123 można anulować na poziomie otrzymania requestu. Po prostu nasz kod odpowiadający za obsługę żądań anulowania wczyta z warstwy danych wniosek nadający się do anulowania, a potem wywoła odpowiednią metodę.
Mam na myśli coś takiego:
A. Domena:

public abstract class HolidayConclusionBase
{
    public Guid Id { get; protected set; }
}

public interface IApprovableHolidays
{
    ApprovedHolidayConclusion Approve(string approvedBy, DateTimeOffset approvedDate);
};

public interface ICancellableHolidays
{
    CancelledHolidayConclusion Cancel(string cancelledBy, DateTimeOffset cancelledDate);
};

public class NewHolidayConclusion : HolidayConclusionBase, IApprovableHolidays, ICancellableHolidays
{
    public string EmployeeName { get; init; }
    public DateTimeOffset StartDate { get; init; }
    public DateTimeOffset EndDate { get; init; }

    public NewHolidayConclusion(Guid id, string employeeName, DateTimeOffset startData, DateTimeOffset endData)
    {
        Id = id;
        EmployeeName = employeeName;
        StartDate = startData;
        EndDate = endData;
    }

    public RejectedHolidayConclusion Reject(string rejectedBy, DateTimeOffset rejectedDate, string rejectedReason) =>
        new(Id, rejectedBy, rejectedDate, rejectedReason);

    public ApprovedHolidayConclusion Approve(string approvedBy, DateTimeOffset approvedDate) => new(Id, approvedBy, approvedDate);

    public CancelledHolidayConclusion Cancel(string cancelledBy, DateTimeOffset cancelledDate) => new(Id, cancelledBy, cancelledDate);
}

public class ApprovedHolidayConclusion : HolidayConclusionBase, ICancellableHolidays
{
    public string ApprovedBy { get; init; }
    public DateTimeOffset ApprovedDate { get; init; }

    public ApprovedHolidayConclusion(Guid id, string approvedBy, DateTimeOffset approvedDate)
    {
        Id = id;
        ApprovedBy = approvedBy;
        ApprovedDate = approvedDate;
    }

    public CancelledHolidayConclusion Cancel(string cancelledBy, DateTimeOffset cancelledDate) => new(Id, cancelledBy, cancelledDate);
}

public class RejectedHolidayConclusion : HolidayConclusionBase, IApprovableHolidays
{
    public string RejectedBy { get; init; }
    public DateTimeOffset RejectedDate { get; init; }
    public string RejectionReason { get; }

    public RejectedHolidayConclusion(Guid id, string rejectedBy, DateTimeOffset rejectedDate, string rejectionReason)
    {
        Id = id;
        RejectedBy = rejectedBy;
        RejectedDate = rejectedDate;
        RejectionReason = rejectionReason;
    }

    public ApprovedHolidayConclusion Approve(string approvedBy, DateTimeOffset approvedDate) => new(Id, approvedBy, approvedDate);
}

public class CancelledHolidayConclusion : HolidayConclusionBase
{
    public CancelledHolidayConclusion(Guid id, string cancelledBy, DateTimeOffset cancelledDate)
    {
        Id = id;
        CancelledBy = cancelledBy;
        CancelledDate = cancelledDate;
    }

    public string CancelledBy { get; }
    public DateTimeOffset CancelledDate { get; }
}

B. Logika aplikacji:

public record CreateHolidaysRequest(string EmployeeName, DateTimeOffset StartDate, DateTimeOffset EndDate)
    : IRequest<Guid>;

public class CreateHolidaysUseCase(IConclusionsRepository conclusionsRepository)
    : IRequestHandler<CreateHolidaysRequest, Guid>
{
    public async Task<Guid> Handle(CreateHolidaysRequest request, CancellationToken cancellationToken)
    {
        var holidayConclusion = new NewHolidayConclusion(Guid.NewGuid(), request.EmployeeName, request.StartDate, request.EndDate);
        var id = await conclusionsRepository.Upsert(holidayConclusion);
        return id;
    }
}

public record ApproveHolidaysRequest(Guid Id) : IRequest<ApproveHolidaysRequest>;

public class ApproveHolidayUseCase(IConclusionsRepository conclusionsRepository)
    : IRequestHandler<ApproveHolidaysRequest, ApproveHolidaysRequest>
{
    public async Task<ApproveHolidaysRequest> Handle(ApproveHolidaysRequest request, CancellationToken cancellationToken)
    {
        var existingConclusion = await conclusionsRepository.GetForApproval(request.Id);
        var approvedConclusion = existingConclusion.Approve("Dyrektorek", DateTimeOffset.UtcNow);
        await conclusionsRepository.Upsert(approvedConclusion);
        return request;
    }
}

public record RejectHolidaysRequest(Guid Id, string Reason) : IRequest<RejectHolidaysRequest>;

public class RejectHolidayUseCase(IConclusionsRepository conclusionsRepository)
    : IRequestHandler<RejectHolidaysRequest, RejectHolidaysRequest>
{
    public async Task<RejectHolidaysRequest> Handle(RejectHolidaysRequest request, CancellationToken cancellationToken)
    {
        var existingConclusion = await conclusionsRepository.GetForRejection(request.Id);
        var approvedConclusion = existingConclusion.Reject("Dyrektorek", DateTimeOffset.UtcNow, request.Reason);
        await conclusionsRepository.Upsert(approvedConclusion);
        return request;
    }
}

public record CancelHolidaysRequest(Guid Id) : IRequest<CancelHolidaysRequest>;

public class CancelHolidayUseCase(IConclusionsRepository conclusionsRepository)
    : IRequestHandler<CancelHolidaysRequest, CancelHolidaysRequest>
{
    public async Task<CancelHolidaysRequest> Handle(CancelHolidaysRequest request, CancellationToken cancellationToken)
    {
        var existingConclusion = await conclusionsRepository.GetForCancellation(request.Id);
        var approvedConclusion = existingConclusion.Cancel("Pokorny pracownik", DateTimeOffset.UtcNow);
        await conclusionsRepository.Upsert(approvedConclusion);
        return request;
    }
}

public record GetAllHolidaysRequest : IRequest<IEnumerable<HolidaysViewModel>>;

public class GetAllHolidaysUseCase(IHolidaysViewModelReader vmReader)
    : IRequestHandler<GetAllHolidaysRequest, IEnumerable<HolidaysViewModel>>
{
    public async Task<IEnumerable<HolidaysViewModel>> Handle(GetAllHolidaysRequest request, CancellationToken cancellationToken)
    {
        return await vmReader.GetAll();
    }
}

Jak widać, w przypadkach użycia nie musimy sprawdzać żadnych statusów wniosku. Skoro wczytaliśmy z bazy, to możemy na nim wywołać metodę domeny i wszystko. Nie trzeba do tego testów jednostkowych, skompilowało się, więc działa.

PS, jeśli komuś wydaje się, że wniosek urlopowy to nie jest holiday conclusion, to niech się lepiej dwa razy zastanowi. Nie takie rzeczy widziałem w kodzie pisanym przez polskich señorów z angielskim C3. ;)