[PHP] Correcte manier van code afbreken?

[Verwijderd]

Ik heb nooit een informatica opleiding gevolgd (slechts hobbyist) en vraag me dus af wat de "correcte" manier is om een snippet als dit af te breken:

PHP:

if (($menu[$item]['type'] == MENU_SHOW || $menu[$item]['type'] == MENU_DEFAULT_LOCAL_TASK) && $menu[$item]['access'] == 1 && ($menu[$item]['callback'] || $visible)) {
  $style  = ($visible) ? (menu_in_active_trail($item) ? 'expanded' : 'collapsed') : 'leaf';
  $title = str_replace("'", '‘', $menu[$item]['title']);
}

Ik zag ergens deze manier:

PHP:

if (($menu[$item]['type'] == MENU_SHOW || $menu[$item]['type'] == MENU_DEFAULT_LOCAL_TASK) && 
     $menu[$item]['access'] == 1 && ($menu[$item]['callback'] || $visible)) {
  $style  = ($visible) ? (menu_in_active_trail($item) ? 'expanded' : 'collapsed') : 'leaf';
  $title = str_replace("'", '‘', $menu[$item]['title']);
}

Maar dat vind ik niet echt veel duidelijker omdat regel 2 dan meer geindent is dan 3 en 4. Toen dacht ik iets als:

PHP:

if (($menu[$item]['type'] == MENU_SHOW || $menu[$item]['type'] == MENU_DEFAULT_LOCAL_TASK) && 
     $menu[$item]['access'] == 1 && ($menu[$item]['callback'] || $visible)
   ) {
  $style  = ($visible) ? (menu_in_active_trail($item) ? 'expanded' : 'collapsed') : 'leaf';
  $title = str_replace("'", '‘', $menu[$item]['title']);
}

Maar dan heeft regel 3, drie spaties, en regel 4 en 5, twee spaties (zoals het hoort, ik indent met 2 spaties). bovendien is regel 1 en 2 nog steeds vrij lang. Wat is de "best practice" manier om dit op te schrijven?

Dezelfde vraag voor een regel als:

PHP:

return $a['weight'] < $b['weight'] ? -1 : ($a['weight'] > $b['weight'] ? 1 : ($a['title'] < $b['title'] ? -1 : 1));

[ Voor 5% gewijzigd door Verwijderd op 03-06-2009 20:27 ]

[Verwijderd]

Dit is een kwestie van persoonlijke smaak en afspraken die je maakt met je collega's of andere developers. Het heeft totaal geen zin hierover te discussiëren, elke developer zal het erover eens zijn dat leesbaarheid belangrijk is. Hoe makkelijker code te lezen is, hoe minder kans op domme fouten.

[BCC]

Waarom giet je het niet in een functie?

[link0007]

PHP:

<?php
if (($menu[$item]['type'] == MENU_SHOW || $menu[$item]['type'] == MENU_DEFAULT_LOCAL_TASK) && 
     $menu[$item]['access'] == 1 && ($menu[$item]['callback'] || $visible)
   ) 
{
  $style  = ($visible) ? (menu_in_active_trail($item) ? 'expanded' : 'collapsed') : 'leaf';
  $title = str_replace("'", '&lsquo;', $menu[$item]['title']);
}
?>

problem solved

(ik heb nooit gesnapt waarom die opening bracket op dezelfde regel als de statement moet.. Zolang de code editor het toelaat, doe ik het zelf ook niet)

[Koetjeboe]

Het is inderdaad nogal persoonlijk. Aangezien overzichtelijkheid het belangrijkste is imho, zou ik het zelfs zo doen:

PHP:

if(
    ($menu[$item]['type'] == MENU_SHOW || $menu[$item]['type'] == MENU_DEFAULT_LOCAL_TASK) 
     &&
     $menu[$item]['access'] == 1 
     && 
    ($menu[$item]['callback'] || $visible)
  ) 
{
    $style = ($visible) ? (menu_in_active_trail($item) ? 'expanded' : 'collapsed') : 'leaf';
    $title = str_replace("'", '‘', $menu[$item]['title']);
}

Ook gebruik ik vrijwel nooit die 'korte' if constructie, ook weer omdat het gewoon niet overzichtelijk leest.

[BCC]

Verwijderd schreef op woensdag 03 juni 2009 @ 20:27:
Ik heb nooit een informatica opleiding gevolgd (slechts hobbyist) en vraag me dus af wat de "correcte" manier is om een snippet als dit af te breken:
[Dezelfde vraag voor een regel als:

PHP:

return $a['weight'] < $b['weight'] ? -1 : ($a['weight'] > $b['weight'] ? 1 : ($a['title'] < $b['title'] ? -1 : 1));

Dit soort zaken wil je echt in een functie doen:

PHP:

function compare_weirdness($a, $b)
{
  if ($a['weigth'] < $b['weigth']) {
    return -1;
  }

  if ($a['weigth'] > $b['weigth']) {
    return 1;
  }

  if ($a['title'] < $b['title']) {
    return -1;
  }
  
  return 1;
}

return compare_weirdness($a, $b)

Nou zie je namelijk ook dat er mogelijk een loze statement in staat, kun je commentaar toevoegen waarom je soms naar het gewicht en soms naar de titel kijkt en kun je unit tests bakken. Daarnaast is het VEEL leesbaarder. Ik denk dat je probleem vooral is dat je teveel logica in een expressie probeert te duwen, waardoor het geheel ernstig onoverzichtelijk wordt.

[user109731]

Ik zie dat je (zoals wel meer PHP-ers doen...) veel arrays gebruikt voor objecten. Het is IMHO veel mooier en overzichtelijker om daar classes van te maken.

Probeer verder de regels niet te lang te maken. Ook al heb je een 24" scherm, het is vaak leesbaarder om het in kortere statements op te delen zoals BCC laat zien

[DukeBox]

^ dat is ook weer persoonlijk en meestal gerelateerd aan kennis in andere talen
Regels niet te lang ? Daar heb je toch 'wrapping' voor in je editor ?

[Verwijderd]

ja, en een op een willekeurige plek gebroken regel leest goed?

[link0007]

JanDM schreef op woensdag 03 juni 2009 @ 20:48:
Ik zie dat je (zoals wel meer PHP-ers doen...) veel arrays gebruikt voor objecten. Het is IMHO veel mooier en overzichtelijker om daar classes van te maken.

Probeer verder de regels niet te lang te maken. Ook al heb je een 24" scherm, het is vaak leesbaarder om het in kortere statements op te delen zoals BCC laat zien

mooier wel, maar arrays zijn gewoon zo briljant veelzijdig in php Ik gebruik ze ook veels te vaak eigenlijk, terwijl ik weet dat het mooier kan

En inderdaad, lange if-statements kan je beter een functie voor gebruiken. Stel je hebt het 13x nodig, en het is een belangrijk deel van de beveiliging, dan is het gevaarlijk om 13x iets te moeten wijzigen. Stel je slaat er een over, en dat veroorzaakt een lek in de beveiliging.. Not cool

[DukeBox]

Verwijderd schreef op woensdag 03 juni 2009 @ 21:12:
ja, en een op een willekeurige plek gebroken regel leest goed?

Dat hangt van je editor af.

[user109731]

DukeBox schreef op woensdag 03 juni 2009 @ 21:05:
Regels niet te lang ? Daar heb je toch 'wrapping' voor in je editor ?

Zoals ik al zei, niet alleen voor de breedte van je scherm of editor, maar lange regels worden snel onoverzichtelijk / onleesbaar.

[BCC]

Inderdaad. Features van een editor zorgen niet voor betere of beter leesbare code.

[ Voor 34% gewijzigd door BCC op 04-06-2009 00:46 ]

[jb044]

Hou je gewoon aan de drupal richtlijnen, is nl. geen kwestie van beter maar simpelweg een kwestie van consistentie. Best belangrijk als het op opensource projecten aankomt waar velen aan hetzelfde werken....

[Leftblank]

jb044 schreef op donderdag 04 juni 2009 @ 00:48:
Hou je gewoon aan de drupal richtlijnen, is nl. geen kwestie van beter maar simpelweg een kwestie van consistentie. Best belangrijk als het op opensource projecten aankomt waar velen aan hetzelfde werken....

Of die van Zend, CodeIgniter, MediaWiki, oftewel, kies een standaard die je leuk/fijn vindt (of aansluit bij waar je code voor gemaakt wordt). Drupal is zeker niet de enige, en naar mijn mening ook niet de meest elegantie manier van code formatting, keuze is er wel genoeg verder.

[Nick_S]

Als je code formatter het onleesbaar maakt, moet je er een functie van maken. Eerder mag altijd, natuurlijk. Een aanpassing aan je code style kan natuurlijk ook, maar dan moet je je overige code ook reformatten.

Ik heb er een hekel aan als ik ergens code aanpas en de (binnen onze omgeving) correcte code formatter er overheen haal, het halve bestand wijzigt. Dit maakt je versiebeheer er ook niet duidelijker op.

[jb044]

Leftblank schreef op donderdag 04 juni 2009 @ 01:10:
[...]

Of die van Zend, CodeIgniter, MediaWiki, oftewel, kies een standaard die je leuk/fijn vindt (of aansluit bij waar je code voor gemaakt wordt). Drupal is zeker niet de enige, en naar mijn mening ook niet de meest elegantie manier van code formatting, keuze is er wel genoeg verder.

Zekers, maar als je dan toch aan drupal werkt kun je je beter aan de heersende formatting standaard houden. Tenminste tenzij ik me sterk vergis is de code die OP aanhaalt een drupal fragment, dus daar sloeg mijn comment op.

[Mei]

Leftblank schreef op donderdag 04 juni 2009 @ 01:10:
[...]

Of die van Zend, CodeIgniter, MediaWiki, oftewel, kies een standaard die je leuk/fijn vindt (of aansluit bij waar je code voor gemaakt wordt). Drupal is zeker niet de enige, en naar mijn mening ook niet de meest elegantie manier van code formatting, keuze is er wel genoeg verder.

Deze code is van Drupal, dus dan is het logisch om die coding standards te gebruiken

JanDM schreef op woensdag 03 juni 2009 @ 20:48:
Ik zie dat je (zoals wel meer PHP-ers doen...) veel arrays gebruikt voor objecten. Het is IMHO veel mooier en overzichtelijker om daar classes van te maken.

Zolang je in PHP nog geen objecten on the fly aan kan maken... Arrays zijn vreselijk flexibel en handig in het gebruik, ondanks dat het semantisch niet altijd klopt en het minder leesbaar is/kan zijn.

Ontopic: Ik zou regels nooit zomaar afbreken. Wat op één regel hoort, hoort gewoon op die regel, ook al is het vervelend lang. Wil je het korter hebben, dan raad ik aan de structuur van je code aan te passen.

Moet die $menu[$item]['access'] trouwens niet met TRUE vergeleken worden?

[RobIII]

Mei schreef op woensdag 17 juni 2009 @ 23:39:
Arrays zijn vreselijk flexibel en handig in het gebruik, ondanks dat het semantisch niet altijd klopt en het minder leesbaar is/kan zijn.

Het concept "Arrays" bestaat volgens mij (maar ik ben geen PHP expert) ook niet echt binnen PHP. Volgens mij is de "Array achtige notatie" die gebruikt wordt achter de schermen niet veel meer dan ordered(?) map ofzo.

[Gomez12]

Mei schreef op woensdag 17 juni 2009 @ 23:39:
[...]
Ontopic: Ik zou regels nooit zomaar afbreken. Wat op één regel hoort, hoort gewoon op die regel, ook al is het vervelend lang. Wil je het korter hebben, dan raad ik aan de structuur van je code aan te passen.

Tja, dan heb ik nog wel wat codevoorbeelden van eenmalige dingetjes die rustig uitlopen tot over de 1000 tekens, zolang het eenmalig blijft en zinnig onderbroken is ( persoonlijk zou ik op de || afbreken en misschien een 2e keer op de && )

Dingen op 1 regel houden of je structuur aanpassen is imo iets te zwart-wit, je kan ( bij eenmalige dingen ) rustig structuur aanbrengen in die ene regel, hoe dramatisch die ook is.
Elk structuur wat je in de regel aanbrengt helpt je volgens mij al op het moment dat je echt de code wil gaan herstructureren

[.oisyn]

RobIII schreef op woensdag 17 juni 2009 @ 23:56:
Volgens mij is de "Array achtige notatie" die gebruikt wordt achter de schermen niet veel meer dan ordered(?) map ofzo.

't Is een hashtable, waarbij elke entry in de hashtable ook prev/next pointers heeft naar het vorige en volgende element. Hij is dus ordered in de zin dat je over de elementen heen kunt lopen in chronologische volgorde (de volgorde waarin de elementen zijn toegevoegd) - niet ordered op basis van de key oid.

[ Voor 29% gewijzigd door .oisyn op 18-06-2009 00:47 ]

[RobIII]

.oisyn schreef op donderdag 18 juni 2009 @ 00:40:
[...]

't Is een hashtable, waarbij elke entry in de hashtable ook prev/next pointers heeft naar het vorige en volgende element. Hij is dus ordered in de zin dat je over de elementen heen kunt lopen in chronologische volgorde - niet ordered op basis van de key oid.

Een linked hashtable achtig iets dus. Ok. Nice to know Zat ik er niet ver langs

[ Voor 3% gewijzigd door RobIII op 18-06-2009 00:46 ]

[roy-t]

BCC schreef op donderdag 04 juni 2009 @ 00:41:
Inderdaad. Features van een editor zorgen niet voor betere of beter leesbare code.

Pardon, dat is toch juist iets wat een goede editor mogelijk maakt, code highlighting, scrollbars ipv indenting, aangeven welke haakjes bij elkaar horen, allemaal erg belangrijk in een goede editor en maakt alle code een stuk leesbaarder.

Zelfs op t.net zie je dat:

Messagebox.Show("test123");

vs:

code:

Messagebox.Show("test123");

vs

C#:

Messagebox.Show("test123");

Maargoed nu ga ik teveel ontopic. (Sorry ik vind het altijd heel iritant om te moeten werken in een kale editor als notepad of kwrite ofzo wat veel mensen waar ik code van inspecteer doen, zelf pleur ik zelfs de meeste simpele code toch het liefst in op zijn minst notepad++)

[.oisyn]

RobIII schreef op donderdag 18 juni 2009 @ 00:46:
[...]

Een linked hashtable achtig iets dus. Ok. Nice to know Zat ik er niet ver langs

Let wel, eentje waarbij integer keys eerst naar string geconverteerd worden om vervolgens te hashen
En nóg leuker, als je een float key gebruikt, dan wordt ie eerst naar int geconvert om vervolgens weer naar string te converteren

[ Voor 18% gewijzigd door .oisyn op 18-06-2009 01:07 ]