OOP in PHP. Exercising on creating a blog

Question

I am attempting to do object oriented programming in right way, and appreciate any review to see if my approach is correct.

class_blogpost.php as below:

<?php
class BlogPosts {

function __construct() {
    # code...
}

public function ListAllBlogPosts() {
    $DB = DatabaseFactory::getFactory()->Connect();
    $query = $DB->prepare("SELECT * FROM posts ORDER BY date DESC");
    $query->execute();
    $list_all = $query->fetchAll(PDO::FETCH_ASSOC);
    return $list_all;
}

public function ShowABlogPost($post_id) {

    if (isset((int)$_GET["id"];)) {
        $post_id = (int)$_GET["id"];
        $DB = DatabaseFactory::getFactory()->Connect();
        $query = $DB->prepare("SELECT * FROM posts WHERE id = :post_id");
        $query->bindParam(':post_id', $post_id, PDO::PARAM_INT);
        $query->execute();
        $show_post = $query->fetchAll(PDO::FETCH_ASSOC);
        return $show_post[0];
    } else {
        echo "Post not found.";
    }

}

}

$blogposts = new BlogPosts;

Below is code for home.php which uses ListAllBlogPosts() function:

<?php
include_once 'connect.php';
$title = "Home";
include_once 'header.php';
$list_all = $blogposts->ListAllBlogPosts();
?>
      <!-- col starts -->
      <div>
<?php
foreach ($list_all as $r) {
  $id = $r["id"];
  $title = $r["title"];
  $post = $r["post"];
  $date = $r["date"];
?>

        <h1><?php echo $title; ?></h1>
        <p><?php echo $post; ?></p>
        <a href="post.php?id=<?php echo $id; ?>">more</a>

<?php
} #ends foreach loop
?>
      </div>

And finally post.php below, user reaches as post.php?id=1 like that:

<?php
include_once 'connect.php';
$id = $blogposts->ShowABlogPost($_GET["id"])["id"];
$title = $blogposts->ShowABlogPost($id)["title"];
$date = $blogposts->ShowABlogPost($id)["date"];
$post = $blogposts->ShowABlogPost($id)["post"];
include_once 'header.php';
?>
      <div>
        <h1><?php echo $title; ?></h1>
        <p><?php echo $post; ?></p>
      </div>

Given your code is not OOP, would you still do things like $blogposts->ShowABlogPost($id)["title"];,$date = $blogposts->ShowABlogPost($id)["date"];? Or you would just fetch a single row and use its contents? What makes you think that fetching the same record from database four times in a row is a good OOP? — Your Common Sense
– Your Common Sense, Commented Jun 13, 2019 at 11:04
Thank you @YourCommonSense, I haven't realized I was fetching 4 times, thanks for pointing out this murder. :) Surely, I would just fetch a single row and use its contents. — LetsSeo
– LetsSeo, Commented Jun 13, 2019 at 11:13
Why would it be important to cast (int)$_GET["id"] before checking if it isset()? — mickmackusa
– mickmackusa, Commented Jun 13, 2019 at 11:20

Your Common Sense · Accepted Answer · 2019-06-13 11:22:06Z

That's a good start but the structure is overall inefficient, that leads to code duplication.

the main problem is the violation of the Single responsibility principle. The ShowABlogPost does four things at a time:

connects to the database
checks the HTTP data
fetches a certain record from the database
displays some error

it should be doing only one thing, what it's intended for - fetching a record from a database.

Another issue is multiple calls to ShowABlogPost(). It would be, like, if you have to pay a cab driver 44.50 and you got this exact amount in your pocket, but would fetch the notes and coins one by one instead of taking them all at once.

You already have an array that contains all the data from the first call. So just use this array.

Other issues are:

$DB = DatabaseFactory::getFactory()->Connect(); is duplicated in every method. Why not to make a database connection a class variable? So it will be available in all class methods by default.
PDO::FETCH_ASSOC could be made the default fetch option so you won't have to call it every time explicitly
when no variables are going to be used in the query, there is no use for the prepared statements.
when you need to fetch only one row, then you have to use fetch() method, not fetchAll()

So let's rewrite your class

<?php
class BlogPosts {

    protected $db;

    function __construct($db) {
        $this->db = $db;
    }

    public function ListAllBlogPosts() {
        $sql = "SELECT * FROM posts ORDER BY date DESC";
        $return $this->db->query($sql)->fetchAll();
    }

    public function ShowABlogPost($post_id) {
        $query = $this->db->prepare("SELECT * FROM posts WHERE id = :post_id");
        $query->bindParam(':post_id', $post_id, PDO::PARAM_INT);
        $query->execute();
        return $query->fetch();
    }
}

And now we can rewrite post.php

<?php
include_once 'connect.php';
include_once 'class_blogpost.php';

$db = DatabaseFactory::getFactory()->Connect();
$blogposts = new BlogPosts($db);

if (!isset($_GET["id"]) {
    die( "Empty request");
} else {
    $post = $blogposts->ShowABlogPost($_GET["id"]);
}
?>

<?php if ($post) : ?>
      <div>
        <h1><?= $post['title']; ?></h1>
        <p><?= $post['post']; ?></p>
      </div>
<?php else : ?>
    Post not found.
<?php endif ?>

Stack Exchange Network

OOP in PHP. Exercising on creating a blog

1 Answer 1

You must log in to answer this question.

Hot Network Questions

OOP in PHP. Exercising on creating a blog

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions