tags:

views:

358

answers:

4

Is there a better way to write this code?

I want to show a default value ('No data') for any empty fields returned by the query:

$archivalie_id = $_GET['archivalie_id'];

$query =    "SELECT 
                a.*, 
                ip.description AS internal_project,
                o.description AS origin,
                to_char(ad.origin_date,'YYYY') AS origin_date  

            FROM archivalie AS a 
            LEFT JOIN archivalie_dating AS ad ON a.id = ad.archivalie_id                
            LEFT JOIN internal_project AS ip ON a.internal_project_id = ip.id
            LEFT JOIN origin AS o ON a.origin_id = o.id               

            WHERE a.id = $archivalie_id";

$result = pg_query($db, $query);

while ($row = pg_fetch_object($result))
{
    $no_data = '<span class="no-data">No data</span>';

    $internal_project = ($row->internal_project != '') ? $row->internal_project : $no_data; 
    $incoming_date = ($row->incoming_date != '') ? $row->incoming_date : $no_data; 
    $origin = ($row->origin != '') ? $row->origin : $no_data; 
}
+3  A: 

You could use a small helper function

function dbValue($value, $default=null)
{
    if ($default===null) {
        $default='<span class="no-data">No data</span>';
    }
    if (!empty($value)) {
        return $value;
    } else {
        return $default;
    }
}
Stefan Gehrig
+1  A: 

If this is not just example code then you surely want to sanitize this query by writing...

$archivalie_id = pg_escape_string($_GET['archivalie_id']);

or you want to convert $archivalie_id with intval() if it is clearly always an integer.

furthermore I suggest to replace 'No data' with a constant like '_MYPROJECT_NODATA' so you can easily change the way your no data label looks or implement internationalisation.

You would then use

define('_MYPROJECT_NODATA', '<span class="no-data">No data</span>');
tharkun
He's using the PostgreSQL extension, so the function to escape DB strings should be pg_escape_string().
Stefan Gehrig
Does it still need escaping if converted to an integer first?
meleyal
@sgehrig: thanks, edited
tharkun
@wdm: no, intval() is enough if it is indeed an integer, I adapted the post
tharkun
+1  A: 

One approach would be to select the default right on the database server.

SELECT 
  IFNULL(NULLIF(a.field1, ''), 'No data')       AS field1, 
  IFNULL(NULLIF(a.field2, ''), 'No data')       AS field2, 
  IFNULL(NULLIF(ip.description, ''), 'No data') AS internal_project,
  IFNULL(NULLIF(o.description, ''), 'No data')  AS origin,
  to_char(ad.origin_date,'YYYY') AS origin_date  
FROM 
  archivalie AS a 
  LEFT JOIN archivalie_dating AS ad ON a.id = ad.archivalie_id                
  LEFT JOIN internal_project  AS ip ON a.internal_project_id = ip.id
  LEFT JOIN origin            AS o  ON a.origin_id = o.id               
WHERE 
  a.id = $archivalie_id

This way you can output values right away, and you do not have to touch existing code. The IFNULL(NULLIF()) turns empty strings to NULL, and NULL to 'No data'. If you want to leave empty strings alone, use the IFNULL() only.

From an architectural perspective, this may lack some elegance (depending on how you look at it), but it is effective.

Tomalak
A: 

You can use standard SQL COALESCE function to return a special string instead of null, like this:

$query =    "SELECT 
            a.*, 
            COALESCE(ip.description,'NO_DATA') AS internal_project,
            COALESCE(o.description,'NO_DATA') AS origin,
            COALESCE(to_char(ad.origin_date,'YYYY'),'NO_DATA') AS origin_date

Then you could replace 'NO_DATA' by the appropriate HTML in your program as others have suggested.

Tony Andrews
What if the string 'NO_DATA' is a legitimate origin value, though?
micahwittman