Que pensez-vous de la qualité de ce code php?

Question

Que pensez-vous de ce code?

Je sais que cela a html mélangé avec php. Comment pouvons-nous séparer html de php? Que faut-il faire d'autre pour améliorer la qualité de ce code?

J'espère que certains experts PHP ici peuvent m'aider.

Edit:

Je voulais juste ajouter que dans ce code particulier la logique métier est déjà dans des fichiers séparés comme la classe AppointmentManager qui prend soin de tout ce qui concerne les rendez-vous, la création suppression, etc., etc.

L'élément de code (manage.php) que je mets ici affiche simplement les détails des rendez-vous et permet leur édition, les annulant. Maintenant, je ne suis pas sûr de ce que nous appelons cette page que ce soit la logique d'affichage ou la logique de présentation ou autre chose .... mais ce que je dois savoir, c'est s'il est possible de séparer plus loin html et php?

Puis-je faire quelque chose par exemple pour obtenir ce morceau de html à partir de ce fichier ...

<script language="JavaScript" src="ajax/core2.js"></script> 

<h2 align='center'> Appointment Control Panel V 1.0</h2> 
<h2 align='center'>Your Next Appointments</h2> 
<div id=detail></div> 

Et De même reste ce ...... je veux dire html et php complètement séparé de sorte que il est facile de maintian et d'étendre l'application. Je ne veux pas utiliser Smarty, je pense que cela complique inutilement les choses. Puis-je créer des fonctions php qui rendent ce code HTML quand il est appelé et peut-être les mettre dans un fichier de fonctions commun comme displayhtml.php?

D'autres meilleures suggestions peut-être?

<?php 
/* 
* manage.php 
* 
* Author: Bob : 2010-11-11 
* 
*/ 

ob_start(); 

$page_title = "Appointments Management"; 

require_once('appts/coreincs.inc'); 
require_once('appts/pagetop.inc'); 

$am = AppointmentManager::getInstance(); 
$appts = $am->getPendingAppointments($g_userID); 
?> 

<script language="JavaScript" src="ajax/core2.js"></script> 

<h2 align='center'> Appointment Control Panel V 1.0</h2> 
<h2 align='center'>Your Next Appointments</h2> 
<div id=detail></div> 

<?php 
if ($appts === NULL)  
{ 
    echo <<<EOEMPTY 
<p align='center'> 
    You have no upcoming appointments. 
</p> 
EOEMPTY; 
} 
else 
{ 
    /** 
    * List the appointments. Start the table. 
    */ 
    echo <<<EOT 
<table align='center' width='80%' border='0' cellspacing='0' 
     cellpadding='3' class='apptTable'> 
<tr> 
    <td width='25%' class='apptDispHeader'>When:</td> 
    <td width='30%' class='apptDispHeader'>Title:</td> 
    <td width='15%' class='apptDispHeader'>Action:</td> 
    <td width='25%' class='apptDispHeader'>Where:</td> 
</tr>  
EOT; 

    /** 
    * Zip through all the appointments and print them out. 
    */ 

    foreach ($appts as $appt) 
    { 
    //if ($appt->StartTime->sameDay($appt->EndTime)) 
    //{ 

     $start = $appt->FullTimestart; 
     $end = $appt->FullTimeend;  

    echo <<<EOAPPT  
<tr>  
    <td>$start ==> $end</td>  
    <td>  
    <div id=title_{$appt->AppointmentID} loaded="t_{$appt->AppointmentID}"> 
    <div id=title_{$appt->AppointmentID}_t_{$appt->AppointmentID}>  
    <a class='apptDispLink' 
     href='showappt.php?aid={$appt->AppointmentID}'> 
    {$appt->Title} 
    </a> 
</div> 
    </div> 
    </td> 

    <td><a onclick=Core2.loadXMLDoc('ajaxrequest.php?aid={$appt->AppointmentID}',['edit'],['detail'],['edit_form_{$appt->AppointmentID}'],"reLoad","hide_all")>Edit</a> | <a onclick=Core2.loadXMLDoc('cancel.php?aid={$appt->AppointmentID}',['delete'],['title_{$appt->AppointmentID}'],['cancel_{$appt->AppointmentID}'],"reLoad","hide_all")>Cancel</a></td> 
    <td> 
    {$appt->Location} 
    </td> 
</tr> 
EOAPPT;  
    }  

    /** 
    * Close out the Table:  
    */ 

    echo <<<EOT 
</table>  
<iframe name=main width="0" height="0" marginwidth="1" marginheight="1" scrolling="no" border="0" frameborder="0"></iframe> 

EOT; 
} 

require_once('appts/pagebottom.inc'); 
ob_end_flush(); 

?> 

Answer 1

1. Veuillez ne pas utiliser Smarty. Utilisez simplement PHP's alternate syntax. Les développeurs PHP partout vous remercieront. N'utilisez pas require_once ou include_once, ils sont plus lents. De plus, s'ils sont «nécessaires» pour que votre code fonctionne, il y a probablement un problème avec votre conception.

2. Gardez votre attribut quoting constant. Vous avez un peu avec «certaines avec» et certains ne sont pas cotées du tout. Je recommande toujours à double citer tout.

3. Il est aussi une bonne idée de séparer votre logique métier sous forme de votre filewise modèle ainsi.

Je recommande quelque chose comme:

require('coreincs.inc'); 
$am = AppointmentManager::getInstance(); 
$appts = $am->getPendingAppointments($g_userID); 
// ..more stuff here 

require('view_dir/view.php'); 

à votre avis, vous pouvez inclure votre en-tête/pied de page

Answer 2

Pour séparer php de HTML, vous pouvez utiliser un cadre de service complet (Zend, gâteau, etc.) ou d'un système de modèles (Smarty, il y a d'autres que je ne peux pas penser). - Ce sera un problème litigieux en raison de personnes abusant de la capacité des systèmes de modèles à analyser la logique causant des modèles qui prennent plus de temps à lire que le mélange php/html.

Si vous n'avez qu'une simple page ou deux, cela ne vaut pas la peine d'investir du temps pour les faire fonctionner.

require_once('appts/coreincs.inc'); 

.inc est une prise laide au-dessus de PHP4, au mieux sa laide au pire, il peut servir votre php en texte clair aux clients. Il suffit donc de rester avec .php

echo <<<EOT 

Je trouve cela un peu dégueulasses sur les yeux et ses bits comme celui-ci qui sont nettoyés un peu plus lors de l'utilisation d'un système de modèle. Sinon, vous pouvez le faire

<?php if ($appts === NULL): ?> 
<p align='center'>  
    You have no upcoming appointments.  
</p> 
<?php endif; ?> 

Je vais vous avouer que cela est encore laid, mais la syntaxe du côlon peut commencer à faire votre php/html mélanger un peu plus clair. Surtout si vous tirez toutes les déclarations de fonctions dans des fichiers séparés et que vous construisez simplement les méthodes/conditions d'appel html + dans le fichier php principal.Ne fermez pas le tag php (en particulier dans les fichiers inclus) cela peut entraîner des espaces vides et causer des problèmes. Je n'ai pas vraiment cru que c'était un problème jusqu'à ce que je l'ai vu causer des problèmes avec l'envoi des en-têtes.

/** 

    * List the appointments. Start the table. 

    */ 

Ne gaspillez pas tant de lignes sur une utilisation simple commentaire // pour commentaires monolignes toujours, ne pas utiliser le double ** à moins que son commentaire doc expliquant ce qu'est une fonction fait.