[PHP] Hoe code verkorten?

Eigenlijk een hele simpele vraag, zie hieronder. Een gebruiker selecteert een aantal producten (checkboxes) en een actie die hierop uitgevoerd moet worden, zoals de geselecteerde producten promoten naar de hoofdpagina. De mogelijke acties staan in een dropdown menuutje. Hieronder de verwerking ervan. Hoewel overzichtelijk, moet er toch een manier zijn om dit korter op te schrijven? Ik voer nagenoeg dezelfde actie uit in elk geval! Alleen heb ik geen goed idee hoe...suggesties?
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24

<?php function story_list_post($edit) {   switch ($edit['type']) {     case 'promote':       foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, 'promote' => 1));       }     break;     case 'unpromote':       foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, 'promote' => 0));       }     break;     case 'sticky':       foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, 'sticky' => 1));       }     break;     case 'unsticky':       foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, 'sticky' => 0));       }     break;   } } ?>

Gooi je foreach in een function met wat parameters
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24

<?php function story_list_post($edit) {   switch ($edit['type']) {     case 'promote':      foo('promote', 1);     break;     case 'unpromote':      foo('promote', 0);     break;     case 'sticky':      foo('sticky', 1);     break;     case 'unsticky':      foo('sticky', 0);     break;   } } function foo($edit, $bar, $foobar)       foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, $bar => $foobar));       } } ?>

LOL @ BalusC GMTA!

Kan overigens (*bijvoorbeeld*) ook nog zo(iets):
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13

<?php function story_list_post($edit) {   $dodah = $edit['type'];   switch ($dodah) {     case 'promote':     case 'unpromote':      foo('promote', substr($dodah, 0, 2) == 'un' ? 0 : 1);     break;     case 'sticky':     case 'unsticky':      foo('sticky', substr($dodah, 0, 2) == 'un' ? 0 : 1);     break;   } } ?>

En zelfs dat kun je weer in een functie gooien die gewoon de cases als parameter pakt en de (un)actie zelf afhandelt.

RobIII wijzigde dit bericht 26-11-2008 17:26 (115%)

Kijk gewoon naar de verschillen. Kijk wat dubbel is en wat niet. Op eerste gezicht kunnen die foreach's prima refactored worden naar een nieuwe functie dat $edit, $actionname en $actievalue accepteert en zo ongeveer het volgende doet
PHP:

1 2 3 4 5

<?php function iets($edit, $actionname, $actionvalue) {     foreach ($edit['stories'] as $key => $value) {         node_edit(array('nid' => $value, $actionname => $actionvalue));     } } ?>

Ik zou het met een array doen ipv lelijke switches.
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13 14

<?php $editTypes = array (     "promote"   => array("promote", 1),     "unpromote" => array("unpromote", 0),     "sticky"    => array("sticky", 1),     "unsticky"  => array("sticky", 0) ); function story_list_post($edit) {     $type = $editTypes[$edit["type"]];     foreach ($edit['stories'] as $key => $value)         node_edit(array('nid' => $value, $type[0] => $type[1]));  } ?>

Ik zou wel rekening houden met de leesbaarheid van je code. Ieder zn smaak, maar als ik als 'vreemde' jouw code moest debuggen zou ik veel blijer zijn met een switch dan met zo'n array...

Kan je uitleggen wat er vreemd aan is? De code is wat mij betreft prima leesbaar en heeft als bonus dat alle editTypes direct bij elkaar staan wat makkelijk te onderhouden is.

Exact. Het is data, en data zet je bij elkaar in datastructuren (wel weer jammer dat PHP dan geen structs kent, maar je zou nog overwegen om er objecten van te maken). Dat stop je niet hardcoded in je programmacode midden in een functie. Wat nou als je op een andere plek een vergelijkbare switch nodig hebt, ga je dan heel leuk copypasten? Of de functie waarin de switch staat ingewikkelder maken zodat hij meerdere dingen kan doen? Dan wordt debuggen pas een feest...

Ik denk het ook meer een gevalletje "wat de boer niet kent vreet ie niet" is dat jij m'n code vreemd vindt. Er is namelijk niets vreemds aan. En dat zeg ik niet omdat ik het zelf geschreven heb , maar omdat heel veel mensen het zo doen.

Je zou trouwens nog kunnen overwegen om het zo te doen:
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18

<?php $editTypes = array (     "promote"   => array("promote" => 1),     "unpromote" => array("unpromote" => 0),     "sticky"    => array("sticky" => 1),     "unsticky"  => array("sticky" => 0) ); function story_list_post($edit) {     $node = $editTypes[$edit["type"]];     foreach ($edit['stories'] as $key => $value)     {         $node["nid"] = $value;         node_edit($node);      } } ?>

.oisyn wijzigde dit bericht 26-11-2008 20:36 (53%)

Ik zou dit in de basis omgekeerd aanpassen, even los van de vraag of een extra functie of arrays het eenvoudiger maken:
PHP:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18

<?php function story_list_post($edit) {   foreach($edit['stories'] AS $value) {     switch ($edit['type']) {       case 'promote':         node_edit(array('nid' => $value, 'promote' => 1));       break;       case 'unpromote':         node_edit(array('nid' => $value, 'promote' => 0));       break;       case 'sticky':         node_edit(array('nid' => $value, 'sticky' => 1));       break;       case 'unsticky':         node_edit(array('nid' => $value, 'sticky' => 0));       break;     }   } } ?>

Zo voorkom je dat je vier keer een foreach-loop definieert.

Zelf zou ik een functie bouwen die als input een 'nid' en een actie krijgt, waarin je de betreffende bewerking uitvoert. Dan kun je die binnen een foreach/for-loop uitvoeren en is je functie de scheiding tussen je loop en de afhandeling van de user input.