cgm225 Posted September 13, 2008 Share Posted September 13, 2008 So I created a new comments class for my blog, and I am looking for some constructive criticism about it. In general, I don't feel like it is very good, and am looking for a lot of feedback. The class is included below, and implemented here. Example of instantiation:: //Instantiates new Comments class $this->comments = new comments($this->mysqli); $this->comments->moderatorViewing($this->registry->get('notesModerator')); $this->registry->set('commentsObject', $this->comments); Comments Class:: <?php class Comments { //Declaring variable(s) private $connection; private $data = array(); private $forWhat; private $commentId; private $name; private $email; private $url; private $comment; private $ip; public function __construct(mysqli $connection) { //Sets MySQLi object $this->connection = $connection; //Sets commentId variable $this->setCommentId(); //Sets POST data to variables $this->setPostData(); } public function findCommentsFor($what) { //Sets forWhat variable $regex = '/[^-_A-z0-9]++/'; //Allowed characters: letter, number, underscore or hyphen $this->forWhat = preg_replace($regex, '', $what); //Sets data array $this->data = array(); //Queries database for comments $this->findComments(); //Adds a new comment or updates and existing comment $this->addUpdate(); } public function setCommentId() { //Sets commentId variable from either POST data or GET data $regex = '/[^-_A-z0-9]++/'; if (!empty($_POST['commentId'])) { $this->commentId = preg_replace($regex, '', $_POST['commentId']); } elseif (isset($_GET['comment'])) { $this->commentId = preg_replace($regex, '', $_GET['comment']); } else { $this->commentId = null; } } public function getCommentId() { return $this->commentId; } public function setPostData() { //Checks for, filters, and strips POST data $this->name = !empty($_POST['name']) ? strip_tags($_POST['name']) : null; $this->email = (isset($_POST['email']) AND filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) ? $_POST['email'] : null; $this->url = (isset($_POST['url']) AND filter_var($_POST['url'], FILTER_VALIDATE_URL)) ? $_POST['url'] : null; $this->comment = !empty($_POST['comment']) ? strip_tags($_POST['comment']) : null; $this->ip = $_SERVER['REMOTE_ADDR']; } public function addUpdate() { //Checks that all required data is present if (isset($this->name) AND isset($this->email) AND isset($this->comment) AND isset($this->forWhat)) { /* Adds a new comment to the database, or else updates an exisiting * comment in the database */ if (empty($this->commentId) AND $this->checkAddPermission()) { $this->addComment(); $this->setDisplayError(false); } elseif ($this->checkUpdatePermission($this->commentId, $this->moderatorViewing)) { $this->updateComment(); $this->setDisplayError(false); } else { $this->setDisplayError(true); } //Checks if data was posted } elseif ($_POST) { //An incomplete set of data was posted $this->setDisplayError(true); } else { /* Either an edit was attempted on an expired comment, or no * addition or update was tried */ if (!empty($this->commentId) AND !$this->checkUpdatePermission($this->commentId, $this->moderatorViewing)) { $this->setDisplayError(true); } else { $this->setDisplayError(false); } } } public function checkUpdatePermission($id, $moderator = false) { /* Sets array position to null if there is no value at the provided key * value** */ if (!isset($this->data[$id])) { $this->data[$id] = null; } //Checks if the user (i.e. IP address) is the comment author if ($this->data[$id]['ip_address'] == $_SERVER['REMOTE_ADDR']) { $userIsAuthor = true; } else { $userIsAuthor = false; } //Checks if more than five minutes have past since comment added if((time() - strtotime($this->data[$id]['timestamp'])) <= (5 * 60)) { $stillEditTime = true; } else { $stillEditTime = false; } //Removes null key/value pair if set above** if ($this->data[$id] == null) { unset($this->data[$id]); } //Returns true if editing allowed if (($userIsAuthor AND $stillEditTime) OR $moderator) { return true; } else { return false; } } public function checkAddPermission() { $query = 'SELECT timestamp FROM comments WHERE ip_address = ? ORDER BY timestamp DESC'; $statement = $this->connection->prepare($query); $statement->bind_param('s', $this->ip); $statement->bind_result($timestamp); $statement->execute(); $statement->fetch(); $statement->close(); if (empty($timestamp)) { $timestamp = null; } else { $timestamp = strtotime($timestamp); } //New comments limited to every sixty seconds if((time() - $timestamp) > (60)) { return true; } else { return false; } } //Adds comment data to the database public function addComment() { $query = 'INSERT INTO comments (name, email, url, comment, for_what, ip_address) VALUES (?, ?, ?, ?, ?, ?)'; $statement = $this->connection->prepare($query); $statement->bind_param('ssssss', $this->name, $this->email, $this->url, $this->comment, $this->forWhat, $this->ip); $statement->execute(); $statement->close(); $this->notifyModerators(); } //Updates existing comment data public function updateComment() { $query = 'UPDATE comments SET name = ?, email = ?, url = ?, comment = ? WHERE id = ?'; $statement = $this->connection->prepare($query); $statement->bind_param('sssss', $this->name, $this->email, $this->url, $this->comment, $this->commentId); $statement->execute(); $statement->close(); } //Creates a two dimensional array in which comment data is stored public function findComments() { $query = 'SELECT id, name, email, url, comment, for_what, ip_address, timestamp FROM comments WHERE for_what = ? ORDER BY id ASC'; $statement = $this->connection->prepare($query); $statement->bind_param('s', $this->forWhat); $statement->bind_result($id, $name, $email, $url, $comment, $for_what, $ip_address, $timestamp); $statement->execute(); while($statement->fetch()) { $this->data[$id] = array( 'id' => $id, 'name' => $name, 'email' => $email, 'url' => $url, 'comment' => $comment, 'for_what' => $for_what, 'ip_address' => $ip_address, 'timestamp' => $timestamp ); } $statement->close(); //Returns true if comments are present, false if not if (!empty($this->data)) { return true; } else { return false; } } //Returns the data array public function getData() { if (!empty($this->data)){ return $this->data; } else { return false; } } //Returns the number of items in the data array public function getDataCount() { if (!empty($this->data)){ return count($this->data); } else { return 0; } } //Converts BBC tags to HTML tags public function bbcToHtml($txt) { //Replacing bold, italic , and underline tags $txt = nl2br($txt); $txt = str_replace('[b]', '<span style="font-weight:800;">', $txt); $txt = str_replace('[/b]', '</span>', $txt); $txt = str_replace('[i]', '<span style="font-style:italic;">', $txt); $txt = str_replace('[/i]', '</span>', $txt); $txt = str_replace('[u]', '<span style="text-decoration:underline;">', $txt); $txt = str_replace('[/u]', '</span>', $txt); //Replacing URL string $urlSearchString = " a-zA-Z0-9\:\/\-\?\&\.\=\_\~\#\'"; $txt = preg_replace("/\[url\]([$urlSearchString]*)\[\/url\]/", '<a href="$1" target="_blank">$1</a>', $txt); $txt = preg_replace("(\[url\=([$urlSearchString]*)\](.+?)\[/url\])", '<a href="$1" target="_blank">$2</a>', $txt); //Replacing mail string $mailSearchString = $urlSearchString . " a-zA-Z0-9\.@"; $txt = preg_replace("(\[mail\]([$mailSearchString]*)\[/mail\])", '<a href="mailto:$1">$1</a>', $txt); $txt = preg_replace("/\[mail\=([$mailSearchString]*)\](.+?)\[\/mail\]/", '<a href="mailto:$1">$2</a>', $txt); return $txt; } public function moderatorViewing($value) { $this->moderatorViewing = $value; } //Sets displayError variable public function setDisplayError($value) { $this->displayError = $value; } //Returns the displayError variable public function displayError() { return $this->displayError; } } ?> Example of implementation (see output here):: <?php //Sets note object from the registry and gets data array containing note data $note = $this->registry->get('noteObject'); $noteData = $note->getData(); //Outputs opening container div and HTML header echo "\t\t<div class=\"notesContainer\">\n"; echo "\t\t\t<h2 class=\"notes\">Notes:: More from me..</h2>\n"; echo "\t\t\tI use this area of my site to share random thoughts, experiences, etc.\n\n"; //Ouputs navigation div with link to notes home page echo "\t\t\t<div class=\"notesNavigationContainer\">\n"; echo "\t\t\t\t<div class=\"notesNavigationRight\">\n"; echo "\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/\">Home »</a>\n"; echo "\t\t\t\t</div>\n"; echo "\t\t\t\t<div class=\"notesNavigationLeft\">\n"; echo "\t\t\t\t\t<span style=\"color:#ffffff;\">_</span>\n"; echo "\t\t\t\t</div>\n"; echo "\t\t\t\t<div class=\"notesNavigationCenter\">\n"; echo "\t\t\t\t</div>\n"; echo "\t\t\t</div>\n\n"; //Outputs entry data echo "\t\t\t<div class=\"noteContainer\">\n"; echo "\t\t\t\t<h2 class=\"notes\">" . $noteData['title'] . "</h2>\n"; echo "\t\t\t\t" . $noteData['date'] . "\n"; echo "\t\t\t\t<p class=\"noteBody\">\n\t\t\t\t\t" . stripslashes(str_replace("\n" , "\t\t\t\t\t<br />\n\t\t\t\t\t", $noteData['note'])) . "\n\t\t\t\t</p>"; echo "\n\t\t\t</div>\n\n"; //Sets comments object from the registry $comments = $this->registry->get('commentsObject'); $comments->findCommentsFor('noteEntry' . $noteData['id']); //Outputs opening comments container echo "\t\t\t<div class=\"noteCommentContainer\">\n"; //Finds comments, and, if present, outputs them from oldest to newest if ($comments->findComments()) { //Outputs comments header echo "\t\t\t\t<h2 class=\"notes\">Comments:</h2>\n"; //Ouputs comments foreach($comments->getData() as $comment) { //Ouputs opening comment div's echo "\t\t\t\t<div class=\"noteComment\">\n"; echo "\t\t\t\t\t<div class=\"noteCommentLeft\">\n"; //Adds link to user provided URL if present if (isset($comment['url'])) { echo "\t\t\t\t\t\t<a href=\"" . $comment['url'] . '">' . $comment['name'] . "</a>\n"; } else { echo "\t\t\t\t\t\t" . $comment['name'] . "\n"; } echo "\t\t\t\t\t\t<br />\n"; //Ouputs comment date echo "\t\t\t\t\t\t<span class=\"noteCommentDate\">" . date('F j, Y, g:i a', strtotime($comment['timestamp'])) . "</span>\n"; // Provides an edit link if user has permission to edit if ($comments->checkUpdatePermission($comment['id'], $this->registry->get('notesModerator'))) { echo "\t\t\t\t\t\t<br />\n"; echo "\t\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . '/notes/entry/' . $noteData['id'] . '/' . $comment['id'] . "#comment\">Edit</a>\n"; } echo "\t\t\t\t\t</div>\n"; //Ouputs comment and closing tags echo "\t\t\t\t\t<div class=\"noteCommentRight\">\n"; echo "\t\t\t\t\t\t" . $comments->bbcToHtml($comment['comment']) . "\n"; echo "\t\t\t\t\t</div>\n"; echo "\t\t\t\t</div>\n"; } } /* Outputs comment form, and includes comment data within the for if the user * is editing an exisitng comment */ //Sets commentID, gets comment data, and checks for editing permission $commentId = $comments->getCommentId(); $commentsData = $comments->getData(); $permission = !empty($commentId) ? $comments->checkUpdatePermission($commentId, $this->registry->get('notesModerator')) : false; //Sets form variables $commentId = $permission ? $commentId : null; $name = $permission ? $commentsData[$commentId]['name'] : null; $email = $permission ? $commentsData[$commentId]['email'] : null; $url = $permission ? $commentsData[$commentId]['url'] : null; $comment = $permission ? $commentsData[$commentId]['comment'] : null; //Outputs anchor to comment form echo "\n\t\t\t\t<a name=\"comment\"></a>\n"; //Ouputs comment form header echo "\n\t\t\t\t<h2 class=\"noteCommentForm\">Post a comment...</h2>\n\n"; //Ouputs comment form echo "\t\t\t\t<form class=\"noteComment\" action=\"http://" . $_SERVER['SERVER_NAME'] . '/notes/entry/' . $noteData['id'] . "#comment\" method=\"post\">\n"; //Displays an error if triggered by checks in comments class if ($comments->displayError()) { echo "\n\t\t\t\t\t<p class=\"noteCommentError\">\n\t\t\t\t\t\t*Name, valid e-mail address, and comment fields required.<br />\n"; echo "\t\t\t\t\t\t*Only one new comment is allowed per minute!<br />\n"; echo "\t\t\t\t\t\t*Comments cannot be edited after five minutes!\n\t\t\t\t\t</p>\n\n"; } //Outputs fieldset with labels and inputs echo "\t\t\t\t\t<fieldset class=\"noteComment\">\n"; echo "\t\t\t\t\t\t<label for=\"name\">Name:</label><br />\n"; echo "\t\t\t\t\t\t<span><input type=\"text\" name=\"name\" id=\"name\" size=\"30\" value=\"" . $name . "\" maxlength=\"22\" class=\"noteComment\" /></span><br />\n\n"; echo "\t\t\t\t\t\t<label for=\"email\">E-mail: <span class=\"noteCommentParenthetical\">(will NOT be displayed)</span></label><br />\n"; echo "\t\t\t\t\t\t<span><input type=\"text\" name=\"email\" id=\"email\" size=\"30\" value=\"$email\" maxlength=\"250\" class=\"noteComment\" /></span><br />\n\n"; echo "\t\t\t\t\t\t<label for=\"url\">URL:</label><br />\n"; echo "\t\t\t\t\t\t<span><input type=\"text\" name=\"url\" id=\"url\" size=\"30\" value=\"$url\" maxlength=\"250\" class=\"noteComment\" /></span><br />\n\n"; echo "\t\t\t\t\t\t<label for=\"comment\">Comment: <span class=\"noteCommentParenthetical\">(you may use [b][i][u][url] and [mail] <a href=\"http://en.wikipedia.org/wiki/BBCode\">BBC</a> tags)</span></label><br />\n"; echo "\t\t\t\t\t\t<span><textarea name=\"comment\" id=\"comment\" rows=\"10\" cols=\"72\">$comment</textarea></span>\n\n"; echo "\t\t\t\t\t\t<span><input type=\"hidden\" name=\"commentId\" id=\"commentId\" value=\"$commentId\" /></span>\n\n"; echo "\t\t\t\t\t\t<span><input type=\"submit\" name=\"submit\" value=\"Submit\" class=\"submitNoteComment\" /></span>\n"; echo "\t\t\t\t\t</fieldset>\n"; //Ouputs closing form and container tags echo "\t\t\t\t</form>\n"; echo "\t\t\t</div>\n"; echo "\t\t</div>"; ?> Thanks again for your help! Link to comment Share on other sites More sharing options...
cgm225 Posted September 16, 2008 Author Share Posted September 16, 2008 Bump... please test out my comment class here Link to comment Share on other sites More sharing options...
Dead6re Posted September 18, 2008 Share Posted September 18, 2008 Just a few things, I notice you use overflow:hidden to prevent the page width increasing if you have a long line of characters. Break this up! e.g. function SingleWordwrap($String, $Width){ $String = explode(' ', $String); $NewString = ''; foreach ($String as $Value) { $Len = strlen($Value); if($Len > $Width) $Value = wordwrap($Value, $Width, '–<br/>', true); $NewString .= $Value.' '; } return $NewString; } $Comment = SingleWordwrap($Comment, 20); You strip the tags for posts, what happens if you have a technical post and people want to respond with code? Instead of stripping the comments you can use htmlentities: $Value = htmlentities($Value, ENT_QUOTES, 'UTF-8', false); Then use the MySQL escapestring function. Finally, if you post an incorrect form, you lose all the stuff you had previously written. Use $_POST to retrieve what you had before and also display the error message. Link to comment Share on other sites More sharing options...
cgm225 Posted September 18, 2008 Author Share Posted September 18, 2008 Thank you very much for your input!! I will work on making those changes then repost here. Anything else? How does my class look? Link to comment Share on other sites More sharing options...
Maq Posted September 24, 2008 Share Posted September 24, 2008 I know this is 6 days old but... Looks good keep it simple but not too simple. I would make borders darker, widen it, and maybe some colors, it's kind of boring... Link to comment Share on other sites More sharing options...
darkfreaks Posted September 26, 2008 Share Posted September 26, 2008 Cross Site Scripting in URI How to Fix: strip_tags(), htmlspecialchars() Link to comment Share on other sites More sharing options...
Recommended Posts