PHP Best practices

[Vial]

Hei

 

Har lagt merke til at det florerer mye forskjellige måter å skrive PHP-kode på, spesielt dette med HTML-output virker å være veldig varierende.

 

Ifra ressurser på nettet er det ofte litt vanskelig å avgjøre hva som er den "beste" måten å gjøre ting på. En annen grunn til at jeg vil vite mer om dette er dette med at short-tag <?= ?> nå skal bort.

 

Bruk gjerne denne tråden til å legge inn dine tips om PHP Best practices, det er aldri dumt å dele god kunnskap, kanskje man t.om får igjen for det når man skal ta over noen andres php-prosjekt i fremtiden...

 

 

Q: Hva er best practice for HTML-output? Her har jeg sett mye rart, f.eks:

while($k = mysql_fetch_array())
{
    echo "<span id=".$k["person_id"]." >".$k["navn"]."</span>";
}

Samme også med print. Slik jeg har forstått er echo å foretrekke da print() returnerer en verdi i motsetning til echo.

 

Måten jeg gjør dette på er:

while($k = mysql_fetch_array())
{
    ?>
    <span id="<?=$k["person_id"]?>" > <?=$k["navn"]?> </span>
    <?php
}

Nå som short-tags forsvinner blir jeg å bruke <?php echo .... ?>, men burde jeg egentlig skrive alt i PHP?

[Jonas]

Jeg finner ingen ting i dokumentasjonen om at short tags skal fases ut. Og grunnen til at jeg ikke gjør det er sannsynligvis meget god - nemlig at short tags er vanvittig bra, i motsetning til hva du sikkert har hørt tidligere, gitt at man bruker de korrekt. PHP var i utgangspunktet et templatespråk og bruken av templates for å separere logikk fra presentasjon er akkurat hva som er best practise.

 

Stikkord her er alternativ syntax og short tags.

 

http://php.net/manual/en/control-structures.alternative-syntax.php

 

Her er et eksempel på hvordan en template-fil kan se ut som.

 

<?php if ( $error ) : ?>
   <p class="error"><?= $error ?></p>
<?php elseif ( $success ) : ?>
   <p class="success"><?= $success ?></p>
<?php elseif ( $notice ) : ?>
   <p class="notice"><?= $notice ?></p>
<?php endif; ?>

<form method="get" action="index.php" class="frame_form">
   <p class="input">
       <label for="check_ticket">BillettID</label>
       <input type="text" name="ticketId" id="check_ticket">
   </p>
   <p class="submit">
       <input type="submit" name="submit" class="submit" value="Sjekk inn" />
       <input type="submit" name="cancel" class="cancel" value="Avbryt" />
   </p>
   <input type="hidden" name="canvas" value="admin" />
   <input type="hidden" name="event" value="check" />
   <input type="hidden" name="session" value="<?= htmlentities ( $_GET['session'] ) ?>" />
</form>

<script type="text/javascript">
   $(function(){
       $('#check_ticket').focus();
   });
</script>

<table>
   <tr>
       <th>Id</th>
       <th>Navn</th>
       <th>Brukernavn</th>
       <th>E-mail</th>
       <th>Innsjekk</th>
   </tr>
   <?php if ( is_array ( $participants ) && count ( $participants ) > 0 ) : ?>
       <?php foreach ( $participants as $participant ) : ?>
           <tr>
               <td><?= $participant['ticketId']; ?></td>
               <td><?= $participant['realname']; ?></td>
               <td><?= $participant['username']; ?></td>
               <td><?= $participant['email']; ?></td>
               <td>[ <a href="index.php?canvas=admin&event=check&ticketId=<?= $participant['ticketId']; ?>&session=<?= htmlentities ( $_GET['session'] ) ?>"><?= ( $ticketstatus[$participant['ticketId']] ? '<img src="/system/images/icons/tick.png" /> Sjekket inn' : '<img src="/system/images/icons/cross.png" /> <strong>Ikke</strong> sjekket inn' ) ?></a> ]</td>
           </tr>
       <?php endforeach; ?>
   <?php else : ?>
       <tr>
           <td colspan="4" class="msg">Det finnes ingen deltakere til dette arrangementet.</td>
       </tr>
   <?php endif; ?>
</table>

[Vial]

Les her http://cubicspot.blogspot.com/2009/06/maximum-failure-php-6-deprecates-short.html

 

og se her http://www.slideshare.net/thinkphp/php-53-and-php-6-a-look-ahead

 

Fant dette ut når jeg installerte nyeste build av XAMMP. Plutselig validerte ikke PHP'en min. Etter mye om og men fant jeg følgende i php.ini:

; This directive determines whether or not PHP will recognize code between

; <? and ?> tags as PHP source which should be processed as such. It's been

; recommended for several years that you not use the short tag "short cut" and

; instead to use the full <?php and ?> tag combination. With the wide spread use

; of XML and use of these tags by other languages, the server can become easily

; confused and end up parsing the wrong code in the wrong context. But because

; this short cut has been a feature for such a long time, it's currently still

; supported for backwards compatibility, but we recommend you don't use them.

; Default Value: On

; Development Value: Off

; Production Value: Off

; http://php.net/short-open-tag

; short_open_tag = On

 

Hvis ikke det har skjedd en endring i nyere tid?

[Jonas]

Altså, alt det du siterer og linker til er jeg meget klar over. Det sier bare at short tags blir deprecated i PHP 6. Men det vil fortsatt være mulig å bruke short tags og det kommer neppe til å endres før om veldig, veldig lenge - om ikke aldri. Med denne kursen vil short tags kanskje fjernes fullstendig i PHP 7, men innen den tid så vil PHP-teamet byttes ut og sannsynligvis komme til fatning.

Endret 3. mai 2010 av Jonas

[BlueEAGLE]

Q: Hva er best practice for HTML-output? Her har jeg sett mye rart, f.eks:

$query = "SELECT * FROM member_table WHERE ID = $SESSION['member_id'] LIMIT 1";
$result = mysql_query($query) or $fatal = log_db_failure($query, mysql_error());
if(!$fatal)
{
  while($member_row = mysql_fetch_array($result))
  {
      echo "<span id=".$member_row["id"]." >".$member_row["name"]."</span>";
  }
}
else
{
  //Not being able to reciev a respons was fatal in this instance and should be handled gracefully.
}

 

Jeg mener at eksemplet ditt devierer fra "best practice" på to ting. Det første er variabelnavnet $k som kanskje gir mening der og da, men om to måneder så kan du banne på at du ikke aner hva pokker $k er. Det andre er bruken av norske variabelnavn. Dette er en uvesentlighet på små prosjekt som aldri skal lenger enn til en tjener, men det er veldig ugunstig når du trenger hjelp på for ekempel IRC #php@freenode. Der er det de færreste som vet hva $kunde egentlig er men $customer gir en mye klarere ide om hva de kan forvente å finne i varabelen.

 

I tillegg så har jeg lagt på litt kode rundt spørringen. Dette fordi jeg mener at:

* Spørringen bør legges i en egen variabel for å lette feilsøkingen. Når det er flere variabler i en spørring så vil flestepaten av feilene relatert til spørringen komme fra at variabelen ikke inneholder det du trodde den skulle inneholde og da er det enklere å se det når du skriver ut spørringen. I dette tilfellet så er $SESSION['member_id'] nok ikke det du vil ha men $_SESSION['member_id'] og det ser du lett(ere) når du ser spørringen i feilloggen.

 

* Hvis spørringen feiler så bør dette logges og håndteres slik at feilårsaken kan identifiseres i ettertid. Se forøvrig referanse til feilloggen over.

 

* Hvis det at en spørring feiler gjør at siden ikke kan vise noe meningsfylt innhold så bør den vise noe annet. En "Sorry Mac, men dette funker ikke for meg i dag, men jeg har fortalt høvdingen om det så kanskje det funker bedre i morgen." er mye bedre enn "WARNING: $result contains usless info in do_amazing_stuff.php on line 14".

 

* Spesifiser resultatsettet du vil hente data fra. Hvis du senere finner ut at du trenger å legge spørringen i en løkke basert på en tidligere spørring så har du allerede gjort arbeidet du trenger for å legge den inn i denne løkken og slipper å tenke på det i ettertid.

 

...og igjen så går jeg over mine bredder når jeg egentlig hadde tenkt å stoppe etter variabelnavnene dine.

 

Edit: Jeg glemte å ta med LIMIT. Hvis du kun ser etter EN oppføring så begrens søket til 1. Spesielt når du søker på unike verdier. Det er ingen grunn til å søke igjennom 500 rader hvis du treffer det du søker etter på tredje rad. Hvis du allerede har LIMIT inne så er det mindre arbeid å legge inn sideinndeling ved en senere anledning også.

 

Edit2: Det er også greit å merke seg at, siden vi kun bruker ID og name så kan det være greit å "SELECT id,name FROM...". Jeg synes det er greit å spesifisere feltene man skal bruke når man bruker få felt og hvis tabellen er veldig stor (noe den sjelden bør være, men det er en annen historie)

Endret 3. mai 2010 av BlueEAGLE

[Vial]

La oss spille hverandre gode

 

Jeg er enig i det du sier om variabelnavn, spørsmålet mitt dreide seg imidlertid kun om man skulle bruke echo eller kutte av med <? ?> men jeg er helt enig i at selve koden også bør revideres.

 

Jeg mener at eksemplet ditt devierer fra "best practice" på to ting. Det første er variabelnavnet $k som kanskje gir mening der og da, men om to måneder så kan du banne på at du ikke aner hva pokker $k er

 

1. Enkelt variabelnavn bør vel absolutt unngås for fremtidig lesbarhet, helt enig.

 

Det andre er bruken av norske variabelnavn. Dette er en uvesentlighet på små prosjekt som aldri skal lenger enn til en tjener, men det er veldig ugunstig når du trenger hjelp på for ekempel IRC #php@freenode. Der er det de færreste som vet hva $kunde egentlig er men $customer gir en mye klarere ide om hva de kan forvente å finne i varabelen.

 

 

2. Ang. det du nevner som norske variabelnavn så kommer jo dette veldig an på databasen man er tilkoblet, det er imidlertid et godt poeng at man når man oppetter databaser bruker engelske kolonnenavn.

 

I tillegg så har jeg lagt på litt kode rundt spørringen. Dette fordi jeg mener at:

* Spørringen bør legges i en egen variabel for å lette feilsøkingen. Når det er flere variabler i en spørring så vil flestepaten av feilene relatert til spørringen komme fra at variabelen ikke inneholder det du trodde den skulle inneholde og da er det enklere å se det når du skriver ut spørringen. I dette tilfellet så er $SESSION['member_id'] nok ikke det du vil ha men $_SESSION['member_id'] og det ser du lett(ere) når du ser spørringen i feilloggen.

 

3. Det du skriver om $SESSION og $_SESSION forstår jeg ikke helt. Lyst til å utdype?

 

* Hvis spørringen feiler så bør dette logges og håndteres slik at feilårsaken kan identifiseres i ettertid. Se forøvrig referanse til feilloggen over.

4. Feilhåndtering, veldig viktig tema. Er det slik å forstå at 'or' erstatter try{}catch i PHP?

4.1 Jeg er litt uenig i måten du utfører feilsjekken på. Hvis man skal ha mye kode i den første if-testen vil klammeparentesen } lett drukne vekk. Istedenfor burde man vel heller legge funksjonalitet som dette til funksjoner og returnere true eller false med feilmelding?

 

* Hvis det at en spørring feiler gjør at siden ikke kan vise noe meningsfylt innhold så bør den vise noe annet. En "Sorry Mac, men dette funker ikke for meg i dag, men jeg har fortalt høvdingen om det så kanskje det funker bedre i morgen." er mye bedre enn "WARNING: $result contains usless info in do_amazing_stuff.php on line 14".

 

5. Enig i at når spørringen feiler bør den absolutt gi brukeren en relevant feilmelding. Man bør helst ha en fast struktur på hvordan man håndterer feil slik at det oppleves sømløst for brukeren.

 

* Spesifiser resultatsettet du vil hente data fra. Hvis du senere finner ut at du trenger å legge spørringen i en løkke basert på en tidligere spørring så har du allerede gjort arbeidet du trenger for å legge den inn i denne løkken og slipper å tenke på det i ettertid.

 

6. Ser at jeg faktisk har glemt å spesifisere resultatsettet, dårlig dårlig.. Helt enig med du.

 

Edit: Jeg glemte å ta med LIMIT. Hvis du kun ser etter EN oppføring så begrens søket til 1. Spesielt når du søker på unike verdier. Det er ingen grunn til å søke igjennom 500 rader hvis du treffer det du søker etter på tredje rad. Hvis du allerede har LIMIT inne så er det mindre arbeid å legge inn sideinndeling ved en senere anledning også.

 

7. Nå er jeg ingen ekspert på MySQL og hvordan den behandler queries men hvis man i SQL'en spesifiserer WHERE ID = xxx og ID er en unik primærnøkkel så skal vel ikke LIMIT ha noe å si fra eller til? At det er fint å ha den der for å tillate flere pages er jeg enig i men burde man ikke da dele opp queryen i flere biter (F.eks: if($rowcount > 100) $sql .= " LIMIT 1,50 "; ) ?

 

 

Edit2: Det er også greit å merke seg at, siden vi kun bruker ID og name så kan det være greit å "SELECT id,name FROM...". Jeg synes det er greit å spesifisere feltene man skal bruke når man bruker få felt og hvis tabellen er veldig stor (noe den sjelden bør være, men det er en annen historie)

8. Enig. Det er bortkastet prosesseringstid å hente samtlige felter.

 

 

-- Oppfatt meg ikke som arrogant her, jeg er overhodet ikke noen ekspert på PHP og MySQL men absolutt interessert i å bli det

[BlueEAGLE]

3. Det du skriver om $SESSION og $_SESSION forstår jeg ikke helt. Lyst til å utdype?

Hvis du ser i koden så har jeg laget spørringen

$query = "SELECT * FROM member_table WHERE ID = $SESSION['member_id'] LIMIT 1";

Her ser du at jeg bruker variabelen $SESSION men superglobalen som du sannsynligvis vil bruke i dette tilfellet er $_SESSION. Den første er sannsynligvis en feilskriving av den siste, men etter et godt strekk med koding så er det ikke bestandig like lett å få øye på den manglende "_"-en. Hvis du allerede har spørringen i en streng og ser strengen i en feillogg så vil du se at der står:

The query "SELECT * FROM memeber_table WHERE ID = LIMIT 1" failed with the error "...."

Da er det mye enklere å se at her mangler det en ID og da vil du vite nøyaktig hvor i koden denne ligger. Dette er mer relevant når du har spørringer som involverer mange mange fler variabler men praksisen er den samme.

4. Feilhåndtering, veldig viktig tema. Er det slik å forstå at 'or' erstatter try{}catch i PHP?

4.1 Jeg er litt uenig i måten du utfører feilsjekken på. Hvis man skal ha mye kode i den første if-testen vil klammeparentesen } lett drukne vekk. Istedenfor burde man vel heller legge funksjonalitet som dette til funksjoner og returnere true eller false med feilmelding?

Det er ikke slik at or erstatter try/catch. Du kan selvfølgelig bruke try/catch til å fange denne feilen, men hvis du ser i manualen til mysql_query() så returnerer den false ved feil. Det vil si at vi kan bruke dette ved å henge på en "or" for å gjøre noe fornuftig i de tilfellene.

 

Man kan selvsagt argumentere at en "mer riktig" måte å gjøre dette på er å sjekke resultatet av spørringen spesielt f.eks slik:

$query = "SELECT * FROM member_table WHERE ID = $SESSION['member_id']";
try
{
 $result = mysql_query($query);
 if(result === false)
   throw(new db_exception("query '$query' failed with the error '".mysql_error()."'", true);
 else if(mysql_num_rows($result)===0)
   throw(new db_exception("query 'query' returned 0 results.", false);
 }
 catch(Exception $exception)
 {
   if($exception->fatal)
   {
     //handle fatal db exceptions here
   }
   else
   {
     //handle non-fatal exceptions here
   }
 }

 

Du vil da behøve å overstyer exception-klassen slik at konstruktøren tar to argumenter. En streng som angir en feilmelding og en boolean som angir om unntaket er fatalt eller ikke. Igjen så er det avhengig av hvor stort kodeprosjektet ditt er. Hvis du har et modulbasert system så er slike try/catch-metoder en veldig god måte å få en felles behandling av feil på plass fordi du har da sentrale feilbehandlere som du kan endre ett sted og få endringen reflektert over hele fjøla. Har du derimot kun et fåtall sider og ta hensyn til så vil den første måten gi mer smidig kode med tanke på lesbarhet.

 

7. Nå er jeg ingen ekspert på MySQL og hvordan den behandler queries men hvis man i SQL'en spesifiserer WHERE ID = xxx og ID er en unik primærnøkkel så skal vel ikke LIMIT ha noe å si fra eller til? At det er fint å ha den der for å tillate flere pages er jeg enig i men burde man ikke da dele opp queryen i flere biter (F.eks: if($rowcount > 100) $sql .= " LIMIT 1,50 "; ) ?

Det er ikke meg bekjent at Mysql slutter å lete hvis den finner en verdi og man søker i kun kolonner med unike verdier. Hvis spørringen består av "WHERE field1=$something OR fileld2=$something_else" så vil man kunne få to resultater selv om man kun søker i unike kolonner.

 

Hvis du skal spre resultatene fra et søk over flere sider så vil du alltid bruke variabler. Det er ikke hensiktsmessig å bruke en variabel $rowcount før spørringen er kjørt da du ikke nødvendigvis vet hvor mange rader spørringen gir før den kjøres.

 

Vanlig måte å gjøre dette på er å spesifisere det maksimale antallet du vil ha på en side og bruke "LIMIT $page_number*$results_per_page, $results_per_page" for så å sjekke om mysql_num_rows() gir $results_per_page for å avgjøre om det er flere sider. I farten så er jeg usikker hvordan man skulle behandle tilfeller der antall rader er et multiplikat av $results_per_page, men det vet sikkert google.

 

 

Håper dette hjelper.