Page 1 of 1

Displaying results by year

Posted: Sat Feb 03, 2007 2:08 pm
by Shenz
For my website I would like to give the visitors the possibility to view events by year. For example, if a user clicks on the link "2006" he will get a list with all the events we did in 2006.

This is what I already have now:

Code: Select all

<?php	
  require_once('../private/open_testdb.inc');
  $db = OpenDb();
  
  $query1 = mysql_query("SELECT DISTINCT DATE_FORMAT(FROM_UNIXTIME(date), '%Y') AS year FROM web_press ORDER BY year DESC");

  while ($result1 = mysql_fetch_object($query1))
  {
	$years[] = $result1->year; //building an array of the distinct years, we'll use this array for the pagination
  }
  //print_r($years); //output array for debugging

  echo '<div class="pagination">' . implode(" | ", $years). "</div>\n";
  
  $year=$_GET["year"];

  if(!isset($year))
  	$year = $years[0];
  
  $sql = "SELECT pressId,header,page FROM web_press WHERE page='events' AND DATE_FORMAT(FROM_UNIXTIME(date), '%Y')=$year ORDER BY date DESC";
  $result = mysql_query($sql) or die("Invalid query: " . mysql_error());
  
  $max = mysql_num_rows($result);
  $i = 0;

  while($row = mysql_fetch_array($result))
  {
	if($i < $max-1) {
		echo "\t<div class=\"event\">\n";
		$i++;
	}
	else
		echo "\t<div class=\"event noborder_bottom\">\n";
	echo "\t<h4 id=\"p".$row["pressId"]."\">".$row["header"]."</h4>\n\t";
	include sprintf("./press/%s_%04d.php", $row["page"], $row["pressId"]);
	echo "\n\t<div class=\"clearer\">&nbsp;</div>";
	echo "\n\t</div>\n";
  }
  
  echo '<div class="pagination">' . implode(" | ", $years). "</div>\n";
?>
Now my questions:
  • I want to add hyperlinks to the years listed in my pagination so peaple can jump from one year to another one. What would be the best way to achieve this (maybe certain PHP functions I can use?)?
  • The current year in my pagination should be unlinked, all the other should have a link. How to do this?
  • What do you think of my code? Where can things be improved? I'm a newbie but I love to hear how I can write things better :)

Posted: Sat Feb 03, 2007 2:57 pm
by louie35
make another query to the database and select DISTINCT year only, then loop and display it with links to the search page.

Posted: Sat Feb 03, 2007 3:29 pm
by Ollie Saunders
What do you think of my code? Where can things be improved? I'm a newbie but I love to hear how I can write things better
Well for a start you can do this:

Code: Select all

SELECT YEAR(`date`) AS `year` ORDER BY `year` DESC
instead of your complicated query. This...

Code: Select all

while ($result1 = mysql_fetch_object($query1))
  {
        $years[] = $result1->year; //building an array of the distinct years, we'll use this array for the pagination
  }
...could become a separate function:

Code: Select all

function getAllOf($fieldName, $result)
{
    if (!$result) {
        return null;
    }
    $set = array();
    while ($row = mysql_fetch_object($result)) {
        $set[] = $row->$fieldName;
    }
    return $set;
}
Always brace your control statements:
This is bad wrote:

Code: Select all

if ($test) something();
else somethingElse();
This is good wrote:

Code: Select all

if ($test) {
    something();
} else {
    somethingElse();
}
This code:
You wrote:

Code: Select all

echo "\t<div class="event noborder_bottom">\n";
        echo "\t<h4 id="p".$row["pressId"]."">".$row["header"]."</h4>\n\t";
        include sprintf("./press/%s_%04d.php", $row["page"], $row["pressId"]);
        echo "\n\t<div class="clearer">&nbsp;</div>";
        echo "\n\t</div>\n";
can probably be written more clearly in either template style:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $row['pressId']?>" /><?php echo $row['header']?></h4>
or heredoc:

Code: Select all

echo <<< HTML
<div class="event noborder_bottom">
  <h4 id="{$row['pressId']}" />{$row['header']}</h4>
HTML;
Or you could implement a crude view system by using templates; here's something very simple:

Code: Select all

function runTemplate(array $data, $file) 
{
    extract((array)$data);
    ob_start();
    include TEMPLATE_BASEDIR . DIRECTORY_SEPARATOR . $file . '.php';
    $content = ob_get_contents();
    ob_end_clean();
    return $content;
}
You would have a template like this:
templates/mysqlGenHeader.php wrote:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $pressId?>" /><?php echo $header?></h4>
This way you don't have to keep saying $row and you can make separate template files that can be interchanged. Here's how you would use it

Code: Select all

define('TEMPLATE_BASEDIR', 'templates');
echo runTemplate($row, 'mysqlGenHeader');
You wrote: I want to add hyperlinks to the years listed in my pagination so peaple can jump from one year to another one. What would be the best way to achieve this (maybe certain PHP functions I can use?)?
Generate links for each with a request variable that PHP can use to distinguish later.

Code: Select all

<li><a href="?orderBy=year">Year</a></li>
<li><a href="?orderBy=name">Name</a></li>
...

Code: Select all

$legalOrderBys = array('year' => true, 'name' => true); 
if (!isset($_GET['orderBy'], $legalOrderBys[$_GET['orderBy']]) {
    // fail somehow
}
// construct query using $_GET['orderBy']
Wow how did that get so big?! Meh...
Have a good evening :)

Posted: Sun Feb 04, 2007 10:14 am
by Shenz
ole wrote:Well for a start you can do this:

Code: Select all

SELECT YEAR(`date`) AS `year` ORDER BY `year` DESC
Correct me if I'm wrong but I could not use just YEAR('date') because I've Unix timestamps in my db. But YEAR(FROM_UNIXTIME(date)) is doing fine and seems to be a bit faster.
...could become a separate function:

Code: Select all

function getAllOf($fieldName, $result)
{
    if (!$result) {
        return null;
    }
    $set = array();
    while ($row = mysql_fetch_object($result)) {
        $set[] = $row->$fieldName;
    }
    return $set;
}
Ok, looks like my problem is that I'm almost never using functions, right? Is it always useful to create functions even if you will use that function only one time on your page?
Or you could implement a crude view system by using templates; here's something very simple:
Although I will probably not use a template system here, I want to know how it works. What I don't get is how you make from:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $row['pressId']?>" /><?php echo $row['header']?></h4>
this:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $pressId?>" /><?php echo $header?></h4>

define('TEMPLATE_BASEDIR', 'templates');
echo runTemplate($row, 'mysqlGenHeader');
What's the variable $row doing exactly? I'm missing something I guess :s

Then about the hyperlinks. I want to have them this way:

Code: Select all

<li><a href="?orderBy=2005">Year</a></li>
<li><a href="?orderBy=2006">Name</a></li>
I know how to use $_GET['orderBy'] to construct my query, that's not the problem. But the year currently selected should be unlinked, all the others should have a link. So this way:

Code: Select all

<a href="?orderBy=2007">2007</a>| 2006 |<a href="?orderBy=2005">2005</a>
How can I achieve this? Do Ihave to make use of the array keys to check which year is selected?

Already thanks for helping me!

Posted: Sun Feb 04, 2007 10:45 am
by Ollie Saunders
Shenz wrote:Correct me if I'm wrong but I could not use just YEAR('date') because I've Unix timestamps in my db. But YEAR(FROM_UNIXTIME(date)) is doing fine and seems to be a bit faster.
OK well these are just suggestions. I would have thought using a DATETIME field type and the YEAR() function would be easier and better performing. If you have a reason to use an INT field type and UNIX timestamps then fine.
Ok, looks like my problem is that I'm almost never using functions, right?
In my opinion functions don't go far enough and you should really be using classes. I jumped from monolithic code (like yours) to object orientated because I found functions too limiting.
Is it always useful to create functions even if you will use that function only one time on your page?
Yes because it allow you to put functionality under a certain name. Also you can keep functions (or classes) under separate files in your include path and take advantage of reusability across all your projects.
Although I will probably not use a template system here, I want to know how it works. What I don't get is how you make from:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $row['pressId']?>" /><?php echo $row['header']?></h4>
this:

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $pressId?>" /><?php echo $header?></h4>

define('TEMPLATE_BASEDIR', 'templates');
echo runTemplate($row, 'mysqlGenHeader');
What's the variable $row doing exactly? I'm missing something I guess :s
Yes you lumped together two separate blocks of code. I was trying to communicate that those 2 separate pieces of code would be in 2 separate files. So this runs the template...

Code: Select all

define('TEMPLATE_BASEDIR', 'templates');
echo runTemplate($row, 'mysqlGenHeader');
and this...

Code: Select all

?>
<div class="event noborder_bottom">
  <h4 id="<?php echo $pressId?>" /><?php echo $header?></h4>
...is the the template itself, in a separate file.
But the year currently selected should be unlinked, all the others should have a link. How can I achieve this? Do I have to make use of the array keys to check which year is selected?
Something like this:

Code: Select all

foreach ($years as $year) {
    if ($currentYear == $year) {
        // echo year
    } else {
        // echo year as link
    }
}
Again that is something you could put in a function....good programmers would try and generalise that as much as possible so that it can be reused. Thinking about it a little more...here is probably an example of where a single function wouldn't really cut it because there are so many things that could vary. So if you wanted to do that you would probably have to use a class else you will end up with a monster function that is slow and difficult to use.