patternphpMinor
Volunteer Signup Report
Viewed 0 times
signupreportvolunteer
Problem
I was creating a Volunteer Signup Report report in Microsoft Excel yesterday for my job and I had a moment of inspiration. What if I turned it into a website and made it dynamic? The volunteers sign up, the data is logged in a MySQL database, and then the report is automatically updated for the managers to view.
Obviously a website like this is a big project and requires a lot of pages, so I needed a place to start. So I chose to script the report and get it working.
It's been years since I did serious coding in PHP, so a lot of my practices may be out of date. I'd like to run it by you for a thorough code review.
HTML output:
report.php:
```
assign('company_name', $companies['company_name']);
$smarty->assign('today', format_date());
///// EACH RACE /////
$races_t = array();
$query = mysql_query("SELECT DISTINCT race_id FROM shifts WHERE company_id = $company_id ORDER BY race_id ASC;");
while ( $races = mysql_fetch_array($query) ) {
//// RACE HEADER /////
$races['race_name'] = get_race_name($races['race_id']);
///// RACE ROWS (VOLUNTEER SHIFTS) /////
// TODO: change Signed Up and Needed amounts from integer in SQL table to SQL COUNT(*) later on
$shifts_t = array();
$race_id = $races['race_id'];
$query2 = mysql_query("SELECT * FROM shifts WHERE race_id = $race_id ORDER BY shift_date ASC;");
while ( $shifts = mysql_fetch_array($query2) ) {
$races['signed_up_total'] += $shifts['shift_enrolled'];
$races['needed_total'] += $shifts['shift_needed'];
$shifts['shift_date'] = format_date(strtotime($shifts['shift_date']));
$shifts['percent'] = ($shifts['shift_needed'] == 0) ? 0 : round($shifts['shift_enrolled'] / $shifts['shift_needed'] * 100);
$shifts['percent_formatting'] = get_percent_formatting($shifts['percent']);
$shifts['percent'] .= "%";
// TODO: Something right here is broken. Smarty prints the same shifts for every race. Currently debugging on smarty.net forums.
$
Obviously a website like this is a big project and requires a lot of pages, so I needed a place to start. So I chose to script the report and get it working.
It's been years since I did serious coding in PHP, so a lot of my practices may be out of date. I'd like to run it by you for a thorough code review.
HTML output:
report.php:
```
assign('company_name', $companies['company_name']);
$smarty->assign('today', format_date());
///// EACH RACE /////
$races_t = array();
$query = mysql_query("SELECT DISTINCT race_id FROM shifts WHERE company_id = $company_id ORDER BY race_id ASC;");
while ( $races = mysql_fetch_array($query) ) {
//// RACE HEADER /////
$races['race_name'] = get_race_name($races['race_id']);
///// RACE ROWS (VOLUNTEER SHIFTS) /////
// TODO: change Signed Up and Needed amounts from integer in SQL table to SQL COUNT(*) later on
$shifts_t = array();
$race_id = $races['race_id'];
$query2 = mysql_query("SELECT * FROM shifts WHERE race_id = $race_id ORDER BY shift_date ASC;");
while ( $shifts = mysql_fetch_array($query2) ) {
$races['signed_up_total'] += $shifts['shift_enrolled'];
$races['needed_total'] += $shifts['shift_needed'];
$shifts['shift_date'] = format_date(strtotime($shifts['shift_date']));
$shifts['percent'] = ($shifts['shift_needed'] == 0) ? 0 : round($shifts['shift_enrolled'] / $shifts['shift_needed'] * 100);
$shifts['percent_formatting'] = get_percent_formatting($shifts['percent']);
$shifts['percent'] .= "%";
// TODO: Something right here is broken. Smarty prints the same shifts for every race. Currently debugging on smarty.net forums.
$
Solution
IDs must be unique
Your document will not validate because you've reused certain ids multiple times. If you need to reuse selectors, use a class instead.
Overusing the id selector
All of your selectors are ids and they all have the same specificity (deja vu?). The C in CSS is what makes it beautiful, but you're not letting it happen by using nothing but ids (seriously, look at all of that repetition).
Semantic names
Classes and ids should be named after the element's purpose, not what it looks like. Why would an element have an id of "red"? Is it an error? Is it a danger level? Is it nearly empty? Is it underpopulated?
Misusing the table element
Tables should only be used for tabular data. If your content doesn't make sense in a spreadsheet then it doesn't belong in a table. Your first table does not contain tabular data.
You're also using the td element incorrectly. Cells that are intended to be used as headings for a particular row or column should be marked up with the th element.
There's an element for that...
HTML5 added a time element that's appropriate for marking up dates/times.
Learn to love shorthand
Becomes:
Also, you really should provide a complete font stack that contains suitable fallback fonts for systems that don't have Calibri installed.
Your document will not validate because you've reused certain ids multiple times. If you need to reuse selectors, use a class instead.
Overusing the id selector
All of your selectors are ids and they all have the same specificity (deja vu?). The C in CSS is what makes it beautiful, but you're not letting it happen by using nothing but ids (seriously, look at all of that repetition).
Semantic names
Classes and ids should be named after the element's purpose, not what it looks like. Why would an element have an id of "red"? Is it an error? Is it a danger level? Is it nearly empty? Is it underpopulated?
Misusing the table element
Tables should only be used for tabular data. If your content doesn't make sense in a spreadsheet then it doesn't belong in a table. Your first table does not contain tabular data.
You're also using the td element incorrectly. Cells that are intended to be used as headings for a particular row or column should be marked up with the th element.
There's an element for that...
HTML5 added a time element that's appropriate for marking up dates/times.
Learn to love shorthand
body {
font-family: Calibri;
font-size: 11pt;
}Becomes:
body {
font: 11pt Calibri;
}Also, you really should provide a complete font stack that contains suitable fallback fonts for systems that don't have Calibri installed.
Code Snippets
body {
font-family: Calibri;
font-size: 11pt;
}body {
font: 11pt Calibri;
}Context
StackExchange Code Review Q#100753, answer score: 5
Revisions (0)
No revisions yet.