I am a programming novice. I've written this simple PHP script to run a very basic comment form and would appreciate any feedback, especially on these three topics:
- Efficiency (not sure the right word for this): basically, am I doing anything in a clearly awkward/inferior/inelegant way, in terms of using significantly more lines of code or blocks to do what I could have done in less, poorly organizing the code, or bypassing functions or techniques that would provide equivalent functionality with simpler code?
- Security: this script (and the page that calls it) are currently protected by
htpasswd
and will only be accessed by trusted individuals. In light of that, does my script have any security weaknesses? - Readability + conventionality: are there any awkward practices that would look strange or confusing to an experienced PHP programmer?
To be clear, the code is working exactly as I intend now, which is to display existing comments and to provide two fields (name and comment) for submission, which submit successfully only if both are nonempty and otherwise indicate with CSS which is blank and preserve the POST data. (I do not receive any errors when using ini_set('display_errors', 'On'); error_reporting(E_ALL | E_STRICT);
.)
My code, which follows, is included via include
from another PHP page that has the surrounding HTML and CSS. (I should also note that I realize that this script does not follow the PRG model.)
<?php
/** Function read_comments() takes no arguments. It reads from what it presumes is a correctly formatted (in HTML) comments.txt file. It then prints based on whether the comments file is empty—a default message if so, or otherwise the preformmated contents. */
function read_comments() {
$comments = file_get_contents('comments.txt');
if(empty($comments)) {
echo '<p><i>There are no comments at this time.</i></p>';
} else {
echo $comments;
}
}
/** Function print_form() prints the user submission form, including inline CSS and POST data if a field is left blank. It takes one integer argument that does the following:
* 0 = print blank form
* 1 = mark the textarea border red (to indicate invalid information, defined later as null); also prints POST data
* 2 = mark the text input field border red (to indicate invalid information, defined later as null); also prints POST data
* 3 = mark both fields red */
function print_form($val) {
/* This line is purely stylistic. */
echo '<p><i>Leave a question/comment:</i></p>';
$form = '<form method="post" action="#"><textarea name="comment" id="comment" style="';
/* toggle CSS on textarea based on validation */
if (($val == 1)||($val == 3)) {
$form .= 'border: 1px solid #912; ';
}
$form .= 'width: 80%; height: 10em;">';
/* display post data if any part of the form is invalid */
if ($val != 0) {
$form .= strip_tags($_POST['comment']);
}
$form .= '</textarea><p><i>Your name:</i><br><input type="text" name="name" id="name"';
/* display post data if any part of the form is invalid */
if ($val != 0) {
$form .= 'value="' . strip_tags($_POST['name']) . '"';
}
/* toggle CSS on input block based on validation */
if (($val == 2)||($val == 3)) {
$form .= ' style="border: 1px solid #912;"';
}
$form .= '></p><p><input type="Submit" value="Post comment"></p></form>';
echo $form;
}
/** The role of process_form() is to evaluate whether there is any information to be written; if so, validate it; then call read_comments() and print_form() appropriately */
function process_form() {
$err = 0;
if ($_POST) {
/* validate the comment box */
if (empty($_POST['comment'])) {
$err++;
}
/* validate the name box */
if (empty($_POST['name'])) {
$err = $err + 2;
}
/* if valid, process the form */
if ($err == 0) {
/* create full HTML comment string */
$comment = '<p>“' . strip_tags($_POST['comment']) . '”<br><span style="text-align: right; font-size: 0.75em;">—' . strip_tags($_POST['name']) . ', ' . date('F j\, g\:i A') . '</span></p>';
/* write file to comments page */
file_put_contents('comments.txt', $comment, FILE_APPEND | LOCK_EX);
}
}
/* Run read_comments() to retrieve the comments, including anything new, then print the form with the appropriate validation value */
read_comments();
print_form($err);
}
process_form();
?>