First off, I'd change the logic so that the deleteComment function does just that: deletes a comment. You may have it check that the user trying to delete the comment has the access rights to do so. But it should not check superglobals like $_POST to decide what to do, and a function called deleteComment should definitely not output a form.
Changing the program flow of your code should make it easier to spot the problem, keep the functionality of the function down to only what the function name implies as well as reduce code repetition etc.
function deleteComment($id, $type)
{
static $comment_types = array('note_comments', 'project_comments');
if (!in_array($type, $comment_types))
{
# throw exception, raise error or whatever way you see fit to
# implement error handling ...
# ... then
return false;
}
$query = sprintf('DELETE FROM %s WHERE id=%d',
$type,
(int) $id
);
# Do NOT call mysql_query() here, for several reasons
# 1. If you ever decide to change to another DBMS or API,
# you will have thousands of places to change your
# DB function calls == bad
# 2. Code repetition: Having the calls everywhere you talk to
# the DB, means also having to handle errors in all those
# places.
# 3. You should not use mysql_* to begin with since it's outdated
# by several years. Go with mysqli_ or PDO instead. The upside is
# that as you replace all mysql_ function calls with either
# mysqli_ or PDO, you will understand what item 1. talks about!
# Several ways to handle keeping db calls in one place, this is one
# but not necessarily the best
$result = DB::exec($query);
if ($result)
{
return DB::affectedRows();
}
else
{
return false;
}
}
# very simple DB class using only static functions
class DB {
# private constructor means noone can create instances of this class
# (except for the class itself
private function __construct() {}
static $db = null;
private static connect()
{
if (self::$db === null)
{
self::$db = new PDO('connection details here');
}
}
# returns PDO statement object
public static function query($query)
{
# self::connect goes first in all the public functions
self::connect();
# everything went ok
if ($result = self::$db->query($query))
{
return $result;
}
else
{
self::error($query);
return false;
}
}
# returns number of affected rows
public static function exec($query)
{
# self::connect goes first in all the public functions
self::connect();
# everything went ok
if ($result = self::$db->exec($query))
{
return $result;
}
else
{
self::error($query);
return false;
}
}
private static function error($query)
{
$error = sprintf('(%d) %s%s%s',
self::$db->errorCode(),
self::$db->errorInfo(),
PHP_EOL,
$query
);
# raises error of type E_USER_NOTICE and uses standard error handling according
# to php.ini / custom error handler function.
# They should always be set to log errors, and if not in production environment,
# possibly output them.
# The error itself may be more severe than E_USER_NOTICE (such as failure when logging in)
# but that has to be decided bu the calling code, in this example inside deleteComment()
# But this way you are certain that the error will be logged together with the full
# SQL statement.
trigger_error($error);
}
}
# In target page of the form submit
$feedback = array();
$errors = array();
if (isset($_POST['submit']))
{
if (!isset($_POST['comment']))
{
$errors[] = 'No comment selected';
}
# I realize this may be implied by the system and handled in some other
# way, making this check needless.
if (!isset($_POST['comment_type']))
{
$errors[] = 'No comment type specified';
}
# No errors, proceed
if (count($errors) == 0)
{
$result = deleteComments($_POST['comment'], $_POST['comment_type']);
# Compare both value and type since both false and 0 are possible return values
if ($result === false)
{
$errors[] = 'There was an error deleting the comment';
}
else
{
$feedback[] = sprintf('%d comment%s deleted',
$result,
($result > 1 ? 's were' : ' was')
);
}
}
}
# And in the main content part of every page, you keep
foreach ($errors as $e)
{
printf('<div class="error">%s</div>', $e);
}
foreach ($feedback as $f)
{
printf('<div class="feedback">%s</div>', $f);
}
# Form output goes here
