[PHP] One level of indentation

[JSchut]

Ik probeer mij momenteel zoveel mogelijk te houden aan max 1 indentation level per method. Dit lukt aardig. Echter ben ik heel benieuwd of een bepaald stukje code ook naar 1 level kan zonder dat dit onnodig ingewikkeld word.

Het gaat om deze code:

code:

    protected function getDates($current_year, $past_year) {
        $dates = [];

        for ($year = $current_year; $year >= $past_year; $year--) {
            $dates[$year] = [
                'active' => false,
                'months' => []
            ];
            
            for ($month = 12; $month >= 1; $month--) {
                $dates[$year]['months'][$month]['active'] = false;
                $dates[$year]['months'][$month]['date'] = Carbon::create($year,$month,1,0,0,0);
            }
        }

        return $dates;
    }

Mijn eerste idee was array_fill.

code:

    $dates = array_fill_keys(range($current_year,$past_year),[
        'active' => false,
        'months' => array_fill(1,12,[
            'active' => false,
            'date' => null
        ])
    ]);

Echter mis ik dan de datum want die kan je niet met array_fill toevoegen. Dan krijg je dus nog zoiets als onderstaande om de datum toe te voegen:

code:

    array_walk($dates,function(&$date,$year) {
        array_walk($date['months'], function (&$month_data, $month) use ($year){
            $month_data['date'] = Carbon::create($year,$month,1,0,0,0);
        });
    });

Maar dan heb ik weer 2 levels.

Hoe zou deze code teruggebracht kunnen worden naar 1 level? Nu heb ik het gevoel dat er een andere oplossing mogelijk is voor de nested array_walks. Maar geen idee hoe.

[armageddon_2k1]

Je hebt een visueel probleem dat je met code op probeert te lossen. Zou het niet zo zijn dat functionaliteit voor moet gaan? Wie weet ga je wel van een O(1) naar een O(n) oplossing puur omdat het niet mooi staat.

Kijkende naar wat ik lees over One Level of Indentation

Having too many levels of indentation in your code is often bad for readability, and maintainability.

bron

Lijkt het me dat het meer een richtlijn is. Geen doel op zich. Dus, probeer het toe te passen, maar ga het niet als ultiem doel zien.

[JSchut]

armageddon_2k1 schreef op vrijdag 6 oktober 2017 @ 12:26:
Je hebt een visueel probleem dat je met code op probeert te lossen. Zou het niet zo zijn dat functionaliteit voor moet gaan? Wie weet ga je wel van een O(1) naar een O(n) oplossing puur omdat het niet mooi staat.

Kijkende naar wat ik lees over One Level of Indentation

[...]

bron

Lijkt het me dat het meer een richtlijn is. Geen doel op zich. Dus, probeer het toe te passen, maar ga het niet als ultiem doel zien.

Vandaar dat ik zeg dat ik het zoveel mogelijk probeer. Het is zeker niet altijd de beste oplossing en zeker niet een geschreven regel waar niet van afgeweken mag worden. Veel is echter wel eenvoudig te reduceren naar 1 level. Vaak bijvoorbeeld door guards te gebruiken. Echter op het moment dat het if/for/foreach statements zijn zonder return die een array opbouwen dan lijkt het soms wat lastiger te zijn.

Vandaar dat ik mij afvraag of er een betere manier is om bovenstaande code te schrijven. Het liefst met 1 level. Maar als dat niet kan/practisch is dan met meer.

[crisp]

Gewoon met meerdere methods?

PHP:

protected function getDates($current_year, $past_year) {
    $dates = [];

    for ($year = $current_year; $year >= $past_year; $year--) {
        $dates[$year] = [
            'active' => false,
            'months' => $this->getMonthsForYear($year),
        ];
    }

    return $dates;
}

private function getMonthsForYear($year) {
    $months = [];

    for ($month = 12; $month >= 1; $month--) {
        $months[$month] = [
            'active' => false,
            'date'   => Carbon::create($year,$month,1,0,0,0),
        ];
    }

    return $months;
}

[Janoz]

PHP:

    protected function getDates($current_year, $past_year) {
        $dates = [];

        for ($year = $current_year; $year >= $past_year; $year--) {
            $dates[$year] = [
                'active' => false,
                'months' => getMonths($year)
            ];
        }
        return $dates;
    }

    private function getMonths($year) {
        $months = [];

        for ($month = 12; $month >= 1; $month--) {
            $months[$month]['active'] = false;
            $months[$month]['date'] = Carbon::create($year,$month,1,0,0,0);
        }
        return $months;
    }

Maar goed, ik vond het wel meevallen met de cyclomatic complexity van de originele methode.


--edit... hmm spuit 11. Moet sneller hacken blijkbaar

[ Voor 4% gewijzigd door Janoz op 06-10-2017 12:40 ]

[JSchut]

Dit is inderdaad wat ik in eerste instantie ook had bedacht. Echter vond ik dit overkill.

Ik hou het wel gewoon bij de originele code. Was benieuwd of er ene betere manier was om zo dit soort arrays op de bouwen.

[Hopscotch]

Ik hou sowieso niet erg van dit soort array structuren. Dat kan je volgens mij beter in objecten stoppen. Dan hoef je als je een dergelijke structuur krijgt niet te gaan zoeken of raden wat voor keys er mogelijk bestaan, maar heb je een object waarvan duidelijk is wat voor functies je er op aan kunt roepen, dan verdwijnt dit probleem volgens mij automatisch.