Page 1 of 1

Laying out php code for readability and maintenance...

Posted: Tue Sep 04, 2007 6:12 am
by CoderGoblin
OK not sure where to really post this so mods feel free to move it....

As a fairly long member, if not regular member poster, on this forum it has come to my attention that often people do not structure code in such a way as to make it easily readable and maintainable. Te purpose of this topic is to simply give people something to think about. I am not saying that this is the only way to do things but I have found it useful. This is often called templating and some people argue against it but that is for you to decide...

Php and HTML can mix and this can often lead to the following looking PHP code:

Code: Select all

<html>
  <head>
    <title>Mix</title>
  </head>
  <body>
<?php 
 
  // This function simple outputs different strings
  function decide() {
    if (!empty($_GET['testvar']) {
      echo '<p>This is the test var: '.$_GET['testvar'].'</p>';
    } else {
      echo "<p>Well you didn't pass in a test var</p>";
    }
  }

  // The $out variable is used to show some output
  if (!empty($_GET['myvar']==1) {
?>
     <p>hello myvar has a value of one</p>
<?php
  } else {
?>
    <p>hello myvar does not have a value of one</p>
<?php 
}
decide(); 
?>
  </body>
</html>
If I haven't done any typos then this is perfectly valid and will work but is it really readable ? I don't think so, so I'll start to implement just a couple of rules...

First rule...
1. Have a main PHP block at the top of the file. This should contain all your 'logic' often called business logic.
2. All functions go at the top of the php block.
3. Where necessary set variables for output later.

Compare the previous output with the following...

Code: Select all

<?php
  function decide() {
    if (!empty($_GET['testvar']) {
      echo '<p>This is the test var: '.$_GET['testvar'].'</p>';
    } else {
      echo "<p>Well you didn't pass in a test var</p>";
    }
  }

  // The $out variable is used to show some output
  if (!empty($_GET['myvar']==1) {
     $out="<p>hello myvar has a value of one</p>";
  } else {
    $out="<p>hello myvar does not have a value of one</p>";
  }
 
?>
<html>
  <head>
    <title>Mix</title>
  </head>
  <body>
<?php echo $out; ?>
<?php decide(); ?>
  </body>
</html>
I'll leave you to decide which one prefer to read but if you don't like the second one consider this...

Often you need to use something like sessions or the 'header' command to redirect the user based on parameters passed in, either through a form or directly set via $_GET. Both session_start and headers need to be sent before any output. Even a blank line will throw this. If you have a <?php start on the first line and you do not directly output and output within the php block you are less likely to run into any problems with the error message 'headers already sent'.

Posted: Tue Sep 04, 2007 7:32 am
by s.dot
You're following along the lines that coding logic should be separated from markup. Such is the idea of views and templating. Your suggestion is a step shy of a template.

Of course though, I agree, it is much more readable.

Re: Laying out php code for readability and maintenance...

Posted: Wed Sep 05, 2007 3:39 am
by CoderGoblin
CoderGoblin wrote: This is often called templating and some people argue against it but that is for you to decide...
I'd already mentioned that :lol: Must say that my purpose in creating this topic was to allow others to reference it when people post failing code which is very difficult to read with their mix of PHP and HTML. Also possibly for header redirection errors.

Posted: Tue Sep 18, 2007 10:40 pm
by dekindia
very readable,thx

Posted: Wed Sep 19, 2007 12:33 am
by mrkite
I personally can't stand echoing out html code in quotes.

Code: Select all

<?
function blah()
{
   //do something
   echo "<p>something else</p>";
  // do more stuff
}
?>
That drives me nuts, especially when people start escaping quotes on html attributes.

I always prefer to break out of php:

Code: Select all

<?
function blah()
{
   //do something
?>
    <p>something else</p>
<?
  // do more stuff
}
?>

Posted: Wed Sep 19, 2007 3:43 am
by CoderGoblin
Down to personal preference but most people I have been in contact with find echo to be a lot more readable. When you start mixing php and HTML you can easily lose readability and therefore make the code a lot harder to maintain. When trying to echo/create a variable out a large block of text you could use the Heredoc syntax which means you don't need to escape any quotes. When it's a single line with HTML and attributes you can use single quotes.
* single quoted
* double quoted
* heredoc syntax
.

The other thing to bear in mind is to produce variables rather than output text directly in the main php block. Functions should normally return values as opposed to directly outputting. This makes life a lot easier when also adding things like translation.

Re: Laying out php code for readability and maintenance...

Posted: Wed Sep 19, 2007 3:58 am
by Christopher
CoderGoblin wrote:
CoderGoblin wrote: This is often called templating and some people argue against it but that is for you to decide...
I'd already mentioned that :lol: Must say that my purpose in creating this topic was to allow others to reference it when people post failing code which is very difficult to read with their mix of PHP and HTML. Also possibly for header redirection errors.
I think scottayy was perhaps thinking of something like this. A small, but I think conceptually significant difference.

view.php

Code: Select all

<?php
  function decide() {
    if (!empty($_GET['testvar']) {
      return '<p>This is the test var: '.$_GET['testvar'].'</p>';
    } else {
      return "<p>Well you didn't pass in a test var</p>";
    }
  }

  // The $out variable is used to show some output
  if (!empty($_GET['myvar']==1) {
     $out="<p>hello myvar has a value of one</p>";
  } else {
    $out="<p>hello myvar does not have a value of one</p>";
  }

  include 'template.php';
template.php

Code: Select all

<?php
<html>
  <head>
    <title>Mix</title>
  </head>
  <body>
<?php echo $out; ?>
<?php echo decide(); ?>
  </body>
</html>

Posted: Wed Sep 19, 2007 4:01 am
by mrkite
CoderGoblin wrote:Down to personal preference but most people I have been in contact with find echo to be a lot more readable. When you start mixing php and HTML you can easily lose readability and therefore make the code a lot harder to maintain. When trying to echo/create a variable out a large block of text you could use the Heredoc syntax which means you don't need to escape any quotes.
The benefit for me is when it comes to syntax highlighting. You'll lose all syntax highlighting on your html with heredoc or singlequote echos.

Of course the better solution is to use a templating system like smarty or an MVC framework like cake.

Posted: Wed Sep 19, 2007 4:16 am
by CoderGoblin
mrkite wrote:Of course the better solution is to use a templating system like smarty or an MVC framework like cake.
The initial suggestion is indeed a step short of templating but as stated, the whole idea is to make code display more readable/therefore more maintainable and to try to avoid problems like "headers already sent". Just bear in mind that templating does not necessarily involve using Cake or Smarty, you can achieve templating with simple PHP as shown by aborint's example. Most of the legacy code I maintain actually has a body variable. This is appended to throughout the main php area and simply output at the end. Not a templating system but uses the same principle of never stopping/starting PHP just to output HTML. It is still easily readable.