PHP Mad Libs with OOP and SQL

Question

I've been learning PHP and I've just gotten into using OOP. Classes/objects/methods etc. My end goal is to take an old Mad libs program I did with PHP, and convert it to OOP. The gist of the project was allowing the user to enter a verb, adverb, adjective, and noun and storing it in the DB, then displaying it on the page as a Mad lib (along with the rest of the DB).

I was hoping someone could review my source and let me know how to properly use the setters and getters, right now I think my code is just using POST to pass the user entered data to the method?? But if I take it out, it doesn't work anymore.

Basically I'm pretty lost right now and would appreciate any and all advice on how I can improve/fix this code. Right now it does work (including insertion and query of DB), but i'm not sure if I did It properly.

General guidelines Im following to covert to OOP (Inside Madlibs Class):

• Create instance vars for holding a noun, verb, adjective, adverb, and story

• Create getters and setters for all instance vars

• Create method for inserting the new instance vars into DB

• Create method for querying the stories that returns a results set newest to oldest

• Create a method that takes the results set (from the query) as an argument, and returns the results onto page

Madlibs Class Code:

<?php
    class MadLibs {
        private $noun; // String
        private $verb; // String
        private $adjective; // String
        private $adverb; // String
        private $story; // String - entire madlib story

        // Getters
        public function getNoun() {
            return $this->noun;
        }

        public function getVerb() {
            return $this->verb;
        }

        public function getAdjective() {
            return $this->adjective;
        }

        public function getAdverb() {
            return $this->adverb;
        }

        public function getStory() {
            return $this->story;
        }


        // Setters
        public function setNoun($noun) {
            $this->noun = $noun;
        }

        public function setVerb($verb) {
            $this->verb = $verb;
        }

        public function setAdjective($adjective) {
            $this->adjective = $adjective;
        }

        public function setAdverb($adverb) {
            $this->adverb = $adverb;
        }

        public function setStory($story) {
            $this->story = $story;
        }


        // method for inserting the new properties into mad libs database table
        public function insertMadLibs($noun, $verb, $adjective, $adverb, $story) {
            $story = "Have you seen the $adjective $noun? They got up and started to $verb $adverb!";

            $dbc = mysqli_connect('localhost', 'root', '', 'project1')
                    or die('Error connecting to DB');

            $query = "INSERT INTO Madlibs (id, noun, verb, adjective, adverb, story, dateAdded)" .
                    "VALUES (0, '$noun', '$verb', '$adjective', '$adverb', '$story', NOW())";

            $result = mysqli_query($dbc, $query)
                     or die('Error querying DB');

            mysqli_close($dbc);

            echo '<span id="success">Success!</span>';
        }


        // method for querying the stories that return results set newest to oldest
        public function fetchStory() {
            $dbc = mysqli_connect('localhost', 'root', '', 'project1')
                    or die('Error connecting to DB.');

            $query = "SELECT * FROM Madlibs ORDER BY dateAdded DESC";

            $result = mysqli_query($dbc, $query)
                    or die('Error querying DB.');

            mysqli_close($dbc);

            return $result;
        }


        // method that takes the results set (from the query) as an argument, and returns the results in a formatted HTML table
        public function displayStory($result) {
            $dbc = mysqli_connect('localhost', 'root', '', 'project1')
                    or die('Error connecting to DB.');

            while ($row = mysqli_fetch_array($result)) {
                echo '<div id="comments"><p>Have you seen the <b>' . $row['adjective'] . ' ' . $row['noun'] . '</b>? <br />';
                echo 'They got up and started to <b>' . $row['verb'] . ' ' . $row['adverb'] . '</b>! </p>';
                echo '<span id="footer">Date: ' . $row['dateAdded'] . '</span></div>';
            }

            mysqli_close($dbc);
        }

    }
?>

index file where calls are made:

<!DOCTYPE html>
<html>

    <head>
        <title>Mad libs</title>
        <link rel="stylesheet" href="styles.css" type="text/css" />
    </head>

    <body>

<?php
    require_once('MadLibs.php');

    $mad_libs = new MadLibs();

    $mad_libs->setNoun($_POST['noun']);
    $mad_libs->setVerb($_POST['verb']);
    $mad_libs->setAdjective($_POST['adjective']);
    $mad_libs->setAdverb($_POST['adverb']);

    // How do I remove these and still have the program function??
    // Get entered info form form
    $noun = $_POST['noun'];
    $verb = $_POST['verb'];
    $adjective = $_POST['adjective'];
    $adverb = $_POST['adverb'];

    if (isset($_POST['submit'])) {
        if ( (!empty($noun)) && (!empty($verb)) && (!empty($adjective)) && (!empty($adverb)) ) {
            $mad_libs->insertMadLibs($noun, $verb, $adjective, $adverb, $story);
        } else {
            echo '<span id="error">Please fill out all fields!</span>';
        }
    }

?>

        <div id="wrapper">

            <h1>Mad Libs</h1>

            <hr>

            <form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
                <label for="noun">Enter a <b>Noun</b>: </label>
                <input type="text" name="noun" id="noun" class="input" value="<?php echo $noun; ?>"/>
                <br />

                <label for="verb">Enter a <b>Verb</b> (Present Tense): </label>
                <input type="text" name="verb" id="verb" class="input" value="<?php echo $verb; ?>"/>
                <br />

                <label for="adjective">Enter an <b>Adjective</b>: </label>
                <input type="text" name="adjective" id="adjective" class="input" value="<?php echo $adjective; ?>"/>
                <br />

                <label for="adverb">Enter an <b>Adverb</b>: </label>
                <input type="text" name="adverb" id="adverb" class="input" value="<?php echo $adverb; ?>"/>
                <br />

                <input name="submit" id="submit" type="submit" value="Submit"/>
            </form>

        </div>

<?php
    $result = $mad_libs->fetchStory();
    $mad_libs->displayStory($result);
?>

    </body>

</html>

Answer 1

You've used a single class to encapsulate virtual all the behavior of Mad Libs. Although this code is syntactically correct, it is clear you started with existing code and just poured most of it into a class. That's not the best approach.

Your code is also extremely inflexible. It cannot be reused for any other Mad Lib story, and that would be the whole point, I think.

So, what to do?

First you need to analyze the domain for which you're going to write OOP code. What are the obvious elements here? There's a story with blanks which need to be substituted by user input. That gives you, right away, three object: The MadLibStory, the MadLibBlank, and MadLibUserInput. Let's not think any futher, that can really hurt, and just create these classes:

class MadLibStory {
}

class MadLibBlank {
}

class MadLibUserInput {
}

Before we go on we need to think about the relations between these classes. To me the story is at the center. First a story needs to be chosen. That story requires certain substitutions. The user will supply the substitutions and the result is a funny story.

We want to be flexible, so there will be different types of substitutions. Therefore we need a way to identify which type we are dealing with. For greatest flexibility I will use simple strings, which can also be used in the story, like this:

'Have you seen the [adjective] [noun]? They got up and started to [verb] [adverb]!'

This means that in a story, anything between '[' and ']' needs to be substituted with the type indicated.

Let's now write the constructors of the classes. The constructor will, in this case, have all the needed parameters to set up the class.

class MadLibStory {

    private $storyText;
    private $blanks = [];

    public function __construct($storyText)
    {
        $this->storyText = $storyText;
        // match all blanks and remember them
        if (preg_match_all('/\[([^\]]*)\]/', $this->storyText, $matches, PREG_OFFSET_CAPTURE)) {
            foreach ($matches[1] as $match) {
                $this->blanks[] = new MadLibBlank($this, $match[0], $match[1]);
            }
        }
    }

    public function getStoryText()
    {
        return $this->storyText;
    }

}

class MadLibBlank {

    private $story;
    private $type;
    private $offset;

    public function __construct($story, $type, $offset)
    {
        $this->story  = $story;
        $this->type   = $type;
        $this->offset = $offset;
    }

    public function getStory()
    {
        return $this->story;
    }

    public function getType()
    {
        return $this->type;
    }

    public function getOffset()
    {
        return $this->offset;
    }

}

At this point I am going to forget about user input. This is going to be simple, and I am putting user input and HTML output at the global level, just like you did. Mind you, for larger programs this is not optimal, but for now it will do. See MVC design pattern.

We have only two classes, one which deals with the overall story, and one that deals with the blanks. These two classes are intimately linked. Inside the story class constructor we use blank objects, and the blank class constructor has a story object as a parameter. These classes know about each other.

I did not use type hinting.

I also defined the getters for the constructor parameters, they can be useful. Alternatively I could have made the fields 'public'. I prefer it this way, because it gives the class more control over the field. This is a personal choice.

The preg_match_all() function, in the MadLibStory constructor, uses a regular expression. Personally I don't like these, because they are difficult to understand and debug. I used it here because it results in the least amount of code. For an explanation of the regular expression see: regex101.com All I can say is: It works. Sorry.

I would now like to change what is stored in the database, to be more flexible. I will make two database tables, one for the stories and one for the user input. Like this:

CREATE TABLE `madlib_story` (
   `id` INT NOT NULL AUTO_INCREMENT,
   `story` TEXT NOT NULL,
   PRIMARY KEY (`id`)
);

CREATE TABLE `madlib_input` (
   `id` INT NOT NULL AUTO_INCREMENT,
   `story_id` INT NOT NULL,
   `type` VARCHAR(50) NOT NULL,
   `offset` INT NOT NULL,
   `substitute` TEXT NOT NULL,
   PRIMARY KEY (`id`),
   INDEX (`story_id`,`type`)
);

I have chosen, in this case, not to put any prepared stories in a database table. The story table here will be used, each time the user submits substitutions for a story, to store that story and give it an unique identifier which can be used to store the substitutions. It is, of course, quite easy to store prepared stories in another database table. Note that there's a reference to the unique story identifier in the input table.

The only thing left now is that we need to make use of these database tables. I will use mysqli, as you did, but with parameter binding. I will also complete these classes.

The code below is placed in a file called 'madlib.inc':

<?php

class MadLibStory {

    private $storyText;
    private $blanks  = [];
    private $storyId = 0; // zero mean it is not known

    public function __construct($storyText)
    {
        $this->storyText = $storyText;
        // match all blanks and remember them
        if (preg_match_all('/\[([^\]]*)\]/', $this->storyText, $matches, PREG_OFFSET_CAPTURE)) {
            foreach ($matches[1] as $match) {
                $this->blanks[] = new MadLibBlank($this, $match[0], $match[1]);
            }
        }
    }

    public function getStoryText()
    {
        return $this->storyText;
    }

    public function storeStory($mysqli)
    {
        $text = $this->getStoryText();
        // store story in database (no error handling!)
        $stmt = $mysqli->prepare("INSERT INTO madlib_story(text) VALUES (?)");
        $stmt->bind_param('s', $text);
        $stmt->execute();
        $stmt->close();
        // and set the story id
        $this->storyId = $mysqli->insert_id;
        // return object for chaining
        return $this;
    }

    public function getStoryId()
    {
        return $this->storyId;
    }

    public function getBlanks()
    {
        return $this->blanks;
    }

    public function getMadness()
    {
        // put substitutions into blanks, correct for length differences
        $storyText  = $this->getStoryText();
        $correction = 0;
        foreach ($this->getBlanks() as $blank) {
            $offset      = $blank->getOffset() + $correction;
            $substitute  = $blank->getSubstitute();
            $typeLength  = strlen($blank->getType());
            $storyText   = substr_replace($storyText, $substitute, $offset, $typeLength);
            $correction += strlen($substitute) - $typeLength;
        }
        return $storyText;
    }

}

class MadLibBlank {

    private $story;
    private $type;
    private $offset;
    private $substitute = '????';

    public function __construct($story, $type, $offset)
    {
        $this->story  = $story;
        $this->type   = $type;
        $this->offset = $offset;
    }

    public function getStory()
    {
        return $this->story;
    }

    public function getType()
    {
        return $this->type;
    }

    public function getOffset()
    {
        return $this->offset;
    }

    public function setSubstitute($substitute)
    {
        $this->substitute = $substitute;
        // return object for chaining
        return $this;
    }

    public function storeSubstitute($mysqli)
    {
        $storyId    = $this->getStory()->getStoryId();
        $type       = $this->getType();
        $offset     = $this->getOffset();
        $substitute = $this->getSubstitute();
        // store substitute in database (no error handling!)
        $stmt = $mysqli->prepare("INSERT INTO madlib_input(story_id, type, offset, text) VALUES (?,?,?,?)");
        $stmt->bind_param('isis', $storyId, $type, $offset, $substitute);
        $stmt->execute();
        $stmt->close();
        // return object for chaining
        return $this;
    }

    public function getSubstitute()
    {
        return $this->substitute;
    }

}

The only thing, apart form the database storage methods, that's new, is the getMadness() method. It uses an existing story, fills in the substitutions, and returns it. This returned story still has the '[' and ']' brackets in it. That's because I want to replace them with HTML tags, and it is bad practice to do that inside this class. The class should be about the Mad Lib story, not about generating HTML.

Normally you would probably place both classes in a seperate file and use autoloading, but there's only so much we can do in this answer.

Finally we have our main script:

<?php

// show errors during development (disable in production)
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

// get classes 
require('madlib.inc');

// compressed page start
echo '<!DOCTYPE html><html><body><h1>Mad Libs</h1><hr><br>';

// this is the story in this example code
$text  = 'Have you seen the [adjective] [noun]? They got up and started to [verb] [adverb]!';
$story = new MadLibStory($text);

if (count($_POST) > 0) {
    // open connection to database
    $mysqli = new mysqli('127.0.0.1', 'user', 'password', 'database');
    if (mysqli_connect_errno()) die('Connect failed: '.mysqli_connect_error().PHP_EOL);

    // store the story
    $story->storeStory($mysqli);

    // get information from story object
    $storyId = $story->getStoryId();
    $blanks  = $story->getBlanks();

    // fill in the substitutions
    foreach ($blanks as $key => $blank) {
        $substitute = filter_input(INPUT_POST, 'blank'.$key, FILTER_SANITIZE_STRING);
        $blank->setSubstitute($substitute)
              ->storeSubstitute($mysqli);
    }

    // show the result
    $madness = $story->getMadness();
    echo str_replace(['[', ']'], ['<b>', '</b>'], $madness);

    // close connection to database
    $mysqli->close();
}
else {
    // use a form to get user input
    echo '<form method="post">';
    $blanks  = $story->getBlanks();
    foreach ($blanks as $key => $blank) {
        $id = 'blank'.$key;
        echo '<label>Enter a <b>'.ucfirst($blank->getType()).'</b>: </label>'.
             '<input name="'.$id.'"><br><br>';
    }
    echo '<input type="submit" value="Submit">'.
         '</form>';
}

// compressed page end
echo '</body></html>';

And that's it, for now. I hope you can see that these two classes can easily be expanded. You might want to restructure everything a bit when you store original stories in a database table, and you want to retrieve any previously created Mad Lib stories. This code is only meant as an example, it is far from perfect. It tries to illustrate two points:

• Each class should represent one thing in your program (a story, a blank, etc) and all the methods should relate to that.

• Classes should be extendible, like when I added database storage. But I didn't put the story text into the story class, so I can reuse that class for another story.

Those are the first two principle of SOLID. The other principles are not really applicable yet to these two simple classes.