5 poziom zagnieżdżenia pętli - wzorce, które pomogą mi tego uniknąć, proszę o prady

Cześć, poczyniłem taki beznadziejny kod, mam świadomość tego, że coś takiego nie powinno w ogole powstać.
Czy ktoś jest w stanie podpowiedzieć co powinienem doczytać aby uniknać takich sytuacji w przyszłości?

Kod działa i robi swoją robotę, wiec mogę sie zabezpieczyć testem, natomiast chciałbym go napisać lepiej.

private function build()
{
    foreach ($this->classes as $classes) {
        foreach ($classes as $schema) {
            $schema['classes'] = array_values($schema['classes']);
            $specializations = !empty($schema['specializations']) ? $schema['specializations'] : [];

            $schema['specializations'] = [];
            foreach ($specializations as $specialization) {
                foreach ($specialization as $specializationPrograms) {
                    $specs = [];
                    foreach ($specializationPrograms as $specializationProgram) {
                        $classes = array_values($specializationProgram['classes']);
                        $specializationProgram['classes'] = $classes;
                        $specs[] = $specializationProgram;
                    }
                    $schema['specializations'] = $specs;
                }
            }

            $this->classesToGenerate['specialities'][] = $schema;
        }
    }
}

Czy ja wiem czy taki beznadziejny - mieści się na jednym ekranie i dla mnie jest całkiem czytelny; może jednie bym wydzielił generację na per klasę:

foreach ($this->classes as $class) {
    $this->buildClass($class);
}

/* ... */

... i zamienił tablicę-tablic-tablic-tablic na trochę więcej obiektów:

foreach ($class->schemas as $schema) {
    /* ... */
}

/* ... */

foreach ($specialization->programs as $specializationProgram) {
    /* ... */
}

wiec mogę sie zabezpieczyć testem

Test mógł był powstać pierwszy, kod potem :-) - ale tak, kilka testów powinieneś mieć, niezależnie od tego czy obecna forma kodu "jest czytelna", czy nie.

To pole $this->classes, to jest cześć jakiejś biblioteki z której korzystasz? Czy toć całkiem Twój wytwór? Bo jeśli Twój wytwór to ja bym to przerobił nastrój obiekty.

Dodatkowo napisałabym funkcje które nie modyfikujący istniejącego stanu, tylko zwracają nowy, tak żeby struktury były w miarę immutable.

@Patryk27 no czytelny jest to prawda, ale mamy aż 5 pętli zagnieżdżonych co daje złożoność N^5 czy jakoś tak
Dlatego wydaje mi się, żę warto było by to jakoś przerobić. Dlatego zastanawiam się czy są jakieś wzorce na to

@TomRiddle
Tak to mój wytwór.
Czyli "classes" mogłoby być obiektem, w którym hermetyzowałbym logikę , która aktualnie jest w tych pętlach tak?

A w ogóle potrzebujesz tworzyć taką tablicę tablic? Może warto zastanowić się czy nie da rady tego uprościć u podstaw tworzenia tej tablicy.

no czytelny jest to prawda, ale mamy aż 5 pętli zagnieżdżonych co daje złożoność N^5 czy jakoś tak

Czyli zmierzyłeś wydajność Twojego programu, doszedłeś do wniosku, że w przeciętnym przypadku jest zbyt wolna (całość zajmuje sekundy / minuty / godziny) i postanowiłeś, że pora na optymalizację, tak? :-)

... czy jedynie zauważyłeś pięć pętli i pomyślałeś ayy to będzie złe?

Jeśli to drugie, no to zachęcam do poczytania na temat premature optimization - optymalizacja bez potrzeby ("cały proces musi się skończyć poniżej X minut") ani benchmarków ("poprzednia wersja kodu była Y razy szybsza") nie ma sensu, bo będziesz w kółko przepisywał działający, dobry kod (co nikomu nie przyniesie zysku) w sytuacji, gdy równie dobrze mógłbyś pisać dodatkowe ficzery (co przyniesie zysk Tobie oraz użytkownikom Twojej aplikacji / biblioteki).

zastanawiam się czy są jakieś wzorce na to

FWIW, większość wzorców dotyczy control flow aplikacji - czyli zaczynając z n^5 po zastosowaniu wzorca na 99% też będziesz miał n^5, tylko rozwalone na dziesięć klas.

@Patryk27: hmm czyli chcesz mi powiedzieć, że póki kod działa i nie jest zbyt wolny to pomimo 5 pętli powinienem go zostawić?
Bo generalnie tablica nie ma duzo danych, problem tkwi w jej zagnieżdżeniu. Sam program bardzo szybko się wykonuje bez większych problemów.

że póki kod działa

... i, przynajmniej dla mnie, jest czytelny; tak, takiego kodu nie ma imo sensu przepisywać.

pomimo 5 pętli powinienem go zostawić?

Pięć zagnieżdżeń to pięć zagnieżdżeń niezależnie od tego czy są wewnątrz jednej funkcji czy są rozsiane po pięciu osobnych funkcjach; oczywiście sprawa wyglądałaby inaczej, gdybyś miał kod w stylu:

...

foreach (...) {
    ...
        
    foreach (...) {
        if (...) {
           ...   
        } else {
            if (...) {
              ...
            }

            ...

            foreach (...) {
            
            }

            ...

            if (...) {
               ...
            }
        }
    }

    foreach (...) {
        ...
    }
}

...

... ale w Twoim przypadku imo na pierwszy rzut oka widać co kod robi i liczba zagnieżdzeń nie gra specjalnie dużej roli, ponieważ odruchowo widać "a, tu iteracja, tam filtrowanie i jakaś agregacja, luzik"; ten sam kod rozsiany po kilku osobnych funkcjach byłoby dużo trudniej zrozumieć, ponieważ analizując funkcję C trzeba by pamiętać "a, to jest odpalane z wewnątrz B wtedy gdy X, ale nie Y; a B jest odpalane z A chyba że Z, to wtedy leci D".

Refaktoryzacja w celu zmniejszenia głębokości pętli polega na wyniesieniu wnętrza do funkcji / metody.
Ku temu BARDZO pomaga struktura danych lepsza niż tablice na prymitywach

natomiast nie powiedziałeś jaka jest motywacja do zadania tego pytania:
a) wydajność
b) ocena elegancji, czytelności kodu

luposlaw napisał(a):

@Patryk27: hmm czyli chcesz mi powiedzieć, że póki kod działa i nie jest zbyt wolny to pomimo 5 pętli powinienem go zostawić?
Bo generalnie tablica nie ma duzo danych, problem tkwi w jej zagnieżdżeniu. Sam program bardzo szybko się wykonuje bez większych problemów.

Ja uważam inaczej niż @Patryk27. Owszem, kod rozumiem i jest w miarę prosty, ale ja bym go tak nie zostawił.

Mogę go zrefaktorować jeśli chcesz i pokazać jak ja bym go wrzucił

ZrobieDobrze napisał(a):

Refaktoryzacja w celu zmniejszenia głębokości pętli polega na wyniesieniu wnętrza do funkcji / metody.
Ku temu BARDZO pomaga struktura danych lepsza niż tablice na prymitywach

natomiast nie powiedziałeś jaka jest motywacja do zadania tego pytania:
a) wydajność
b) ocena elegancji, czytelności kodu

Żadna z nich.

Ja bym powiedział że

c) kod przede wszystkim powinien wyrażać intencje - czytając go, chciałbym wiedzieć co autor chciał zrobić
b) powinien być łatwy do zmiany, a 5 zagnieżdżonych for takiej nie jest

Przerobiłem go nieco i wyniosłem zagnieżdżenia do obiektu
Nadal jest ich 5, ale teraz nazwy metod mniej więcej mówią co sie dzieje
Według mnie jest lepiej

Oczywiście mozna sie jeszcze przyczepić do nazw i samej struktury jaką jest tablica $specialities, jednakże póki co tak zostawiam.
Jeżeli pojawią się problemy będę kombinował dalej.

public function buildSpecialities(array $specialities): array
{
    $builtSpecialities = [];
    foreach ($specialities as $specialityProgramWithClasses) {
        foreach ($specialityProgramWithClasses as $schema) {
            $specialitySchema = SpecialityProgramSchema::ofArray($schema);
            $schema = $specialitySchema->preparedSchema();
            $builtSpecialities[] = $schema;
        }
    }
    return $builtSpecialities;
}

public function preparedSchema(): array
{
    $schema['moduleId'] = $this->moduleId;
    $schema['moduleNameId'] = $this->moduleNameId;
    $schema['classes'] = $this->classes;
    $schema['specializations'] = $this->specializations;

    foreach ($this->specializations as $specialization) {
        foreach ($specialization as $specializationPrograms) {
            $specs = $this->generateSpecializations($specializationPrograms);
            $schema['specializations'] = $specs;
        }
    }

    return $schema;
}

private function generateSpecializations(mixed $specializationPrograms): array
{
    $specs = [];
    foreach ($specializationPrograms as $specializationProgram) {
        $classes = array_values($specializationProgram['classes']);
        $specializationProgram['classes'] = $classes;
        $specs[] = $specializationProgram;
    }
    return $specs;
}

luposlaw napisał(a):

Przerobiłem go nieco i wyniosłem zagnieżdżenia do obiektu
Nadal jest ich 5, ale teraz nazwy metod mniej więcej mówią co sie dzieje
Według mnie jest lepiej

Oczywiście mozna sie jeszcze przyczepić do nazw i samej struktury jaką jest tablica $specialities, jednakże póki co tak zostawiam.
Jeżeli pojawią się problemy będę kombinował dalej.

public function buildSpecialities(array $specialities): array
{
    $builtSpecialities = [];
    foreach ($specialities as $specialityProgramWithClasses) {
        foreach ($specialityProgramWithClasses as $schema) {
            $specialitySchema = SpecialityProgramSchema::ofArray($schema);
            $schema = $specialitySchema->preparedSchema();
            $builtSpecialities[] = $schema;
        }
    }
    return $builtSpecialities;
}

public function preparedSchema(): array
{
    $schema['moduleId'] = $this->moduleId;
    $schema['moduleNameId'] = $this->moduleNameId;
    $schema['classes'] = $this->classes;
    $schema['specializations'] = $this->specializations;

    foreach ($this->specializations as $specialization) {
        foreach ($specialization as $specializationPrograms) {
            $specs = $this->generateSpecializations($specializationPrograms);
            $schema['specializations'] = $specs;
        }
    }

    return $schema;
}

private function generateSpecializations(mixed $specializationPrograms): array
{
    $specs = [];
    foreach ($specializationPrograms as $specializationProgram) {
        $classes = array_values($specializationProgram['classes']);
        $specializationProgram['classes'] = $classes;
        $specs[] = $specializationProgram;
    }
    return $specs;
}

Lepiej, ale ciągle, według mnie należałoby to rozbić jescze bardziej

A jesteś w stanie napisać, chociażby pseudo kod, w oparciu o ten mój?
Chętnie bym zobaczył jak Ty to widzisz

@luposlaw: Tak jak w komentarzu zapytał się @TomRiddle - czemu jest taka a nie inna postać danych wejściowych? Popraw mnie jeśli się mylę, Twój kod służy do generowania (nie wiem jak nazwać finalny wynik także sorki) tablicy zawierającej klasy (nie te class z PHP :D ). Każda klasa zawiera specjalizacje. Każda specjalizacja zawiera jakieś tam programy, zgadza się?

Rozbiłeś to na pomniejsze funkcje (w porównaniu do pierwotnego kodu) i odwołujesz się do tych funkcji. Poprawnie ale ja bym to rozbił jeszcze bardziej. Główny generator to jeden obiekt, specjalizacja to drugi obiekt natomiast program to trzeci obiekt i na tych obiektach robił te wszystkie foreach'e :)

Czemu? Ponieważ prościej będzie takie coś później utrzymać.

Edit. Browar dla tego, co wymyślił feature w edytorze, że jak napiszę () i kursor jest pomiędzy nawiasami i dodam coś i odruchowo "zamknę" nawias, to przeskakuje tak jak w PHP Storm :D

w najbardziej zagnieżdżonej pętli masz 3 przypisania. Tak jest czy w rzeczywistości jest ich 300 a podałeś tylko kod przykładowy? jak to pierwsze to czytelny jest kod który się mieści na jednym ekranie a nie kod do którego żeby się dostać trzeba się przedzierać przez hierarchię funkcji, i osoba coś poprawiająca traci cza na przełączanie się pomiędzy plikami

Tak jak piszesz, jest ich 3, a nie 300

Przede wszystkim wklej nam dane wejsciowe bo tak to se mozna gdybac
Bo po co te petle

foreach ($specialities as $specialityProgramWithClasses) {
        foreach ($specialityProgramWithClasses as $schema) {

dawaj co masz na wejsciu