Suggestions for OOP CMS

[deft_]

So, I'm creating a CMS for fun and trying to learn OOP in the process. I've written out a couple of files that successfully pull content from a database and display it.

 

Right now, I have a CMS class that handles loading everything. It uses a Query class to query the database to get the data that should be displayed, and then a Page class to load the page file and display the data.

 

I'm just wondering if the code is ideal or if there would be a better way to do it. Should I divide it up more? I'm just not sure if I'm using objects correctly or not. My goal is to make it as flexible as possible like other CMS'. I'd appreciate any advice anyone has. I'll add some of the code below, and then attach all the files to the post in case anyone is willing to look at it all. 

 

CMS class:

<?php

class CMS {

	private $db, $query;
	public $data;

	public function __construct() {
		$this->db = new Database('mysql:host=' . DB_HOST . ';dbname=' . DB_NAME, DB_USER, DB_PASS);

		// Setup main query
		$this->query = new Query($this->db);
	}

	public function bootstrap() {

		// Get page and load page data
		$this->load();

		// Then render the page files
		$this->render();

	}

	public function load() {

		// Get page and set query
		$this->getPage();

		// Query page data
		$this->data = $this->query->run($this->query->query_args);

	}

	public function render() {
		$page = new Page($this->data);
		$page->render();
	}

	public function getPage() {

		if(!empty($_SERVER['QUERY_STRING'])) {

			$page_id = isset($_GET['p']) ? $_GET['p'] : '';

			if(is_numeric($page_id)) {
				$args = [
					'type' => 'page',
					'values' => [
						'page_id' => $page_id
					]
				];

				// Save page id to query page
				$this->query->setArgs($args);
			}

		}

	}

}

Query Class:

<?php

class Query {

	private $db;

	private $query;

	public $query_args = [];
	
	private $types = [
		'page',
		'post'
	];

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

	public function setArgs($args) {
		foreach($args as $key => $value) {
			$this->query_args[$key] = $value;
		}
	}

	public function getArgs() {
		return $this->query_args;
	}

	public function extractArgs() {
		$type = $this->query_args['type'];

		$values = [];
		$values_array = $this->query_args['values'];

		foreach($values_array as $key => $value) {
			$values[] = $value;
		}

		return ['type' => $type, 'values' => $values];
	}

	public function run() {
		if(!empty($this->query_args)) {

			$args = $this->extractArgs();
			$type = $args['type'];
			$values = $args['values'];

			if(in_array($type, $this->types)) {
				$query = $this->db->query("SELECT * FROM {$type}s WHERE id = ?", $values);
				return $query->fetchObject();
			}

		}

	}

}

Page class:

<?php

class Page {

	public $pageData;

	public function __construct($data) {
		$this->pageData = $data;
	}

	public function render() {

		// Setup page variable with all page data
		$page = $this->pageData;

		// Load theme functions file if exists
		if(file_exists('themes/default/functions.php')) {
			require 'themes/default/functions.php';
		}

		if(!$page) {
			$error_404 = 'themes/default/404.php';

			if(file_exists($error_404)) {
				include $error_404;
			} else {
				echo '404 Error: Page not found.';
			}
		} else {
			include 'themes/default/index.php';
		}
	}

}

Index template file:

<?php include 'header.php'; ?>

	<div class="container">
		<h1><?php echo $page->title; ?></h1>

		<?php echo $page->content; ?>
	</div>

<?php include 'footer.php'; ?>

[Jacques1]

The main problem is that you aren't really using OOP.

 

If you had written this as plain old procedural code, it would be ~20 simple lines. Your classes and objects pump that up to almost 200 lines, but they aren't doing anything more. It's the same thing. There's no additional flexibility, no way to extend existing features, no possibility for code reuse.

 

Before you jump to implementation details, you should understand why you're using OOP. The goal of object orientation is not to put code into objects. It's about taking advantage of specific principles like generalization and encapsulation. The objects are just the tools.

 

When you understand that, you should start with a simple, clearly defined task rather than a gigantic open-end project like a CMS. It's also a good idea to learn from others instead of trying to reinvent the wheel. There are many, many open-source implementations of all kinds of tasks from all kinds of clever programmers.

Edited June 16, 2017 by Jacques1

[deft_]

Do you have any resources you can recommend where I can learn more about this with code examples? I've followed a few MVC tutorials on YouTube which use OOP just like this but they don't really explain what you mentioned or how it all works.

[Jacques1]

Like I said, there are plenty of open-source projects on GitHub and other platforms. Choose a topic which you find interesting and which isn't too broad, then look for real projects which implement with this, ideally from people who know what they're doing.

 

I don't think you can learn OOP from tutorials. From my experience, the code quality is usually very poor, the authors aren't competent, and the implementations don't have much to do with reality. There may be good tutorials for very specific topics (like a particular design pattern), but overall, they won't teach you proper OOP.

 

You should focus on two things:

• Get a feeling for usefulness. Does it really make sense to write a 60-line class which cannot do anything but execute a one-line query? I doubt it.
• Try to be more straightforward. For example, the workflow in your CMS class is very odd, forcing the user to call particular methods in a particular order, some of which have weird side effects (like getPage() which also sets query parameters).
• (Re)learn some PHP basics. For example, you spend an awful lot of time copying items from one array to another, apparently not realizing that a simple assignment does the same thing and is both much shorter and more efficient.

[deft_]

Okay thanks, I'll try looking into some other projects.

 

Also, bare in mind I've just thrown something together quickly to get something working. The method names aren't exactly going to stay the same, like getPage() and the code in getPage() might be moved elsewhere or whatever. I'm not exactly sure how to structure it which is one of the things I'm trying to learn. I imagine users would not be editing this part of the code, so they would not have to call any of these methods. And if there is no particular order in which methods should be called, I don't understand how it would work. Obviously the right pieces of code have to be in the right places. I'm assuming I'm misunderstanding what you mean there.

 

Also, you said:

 

• (Re)learn some PHP basics. For example, you spend an awful lot of time copying items from one array to another, apparently not realizing that a simple assignment does the same thing and is both much shorter and more efficient.

 

I'm not sure what you mean by a simple assignment? Could you elaborate on that? I'm assuming you're talking about the query args array. 

 

And thank you for your help so far, I appreciate it.

Edited June 16, 2017 by deft_

[Jacques1]

And if there is no particular order in which methods should be called, I don't understand how it would work.

 

My point is that your classes expect the class user to go through an exact sequence of steps which is neither documented nor obvious. If they don't do that, then the methods blow up or misbehave in unexpected ways.

 

Take your CMS class, for example: I create an instance, so now I should be ready to call a method like render(), right? Wait, the whole application crashes with a cryptic error message telling me that some query parameters are null. What the hell? As it turns out, I'm not supposed to call any methods except for bootstrap() which ... renders the page. Or it doesn't. If there doesn't happen to be a URL parameter named “p”, then the whole method just quietly ignores my request.

 

This makes absolutely no sense. The only way to understand the class is to go through the entire implementation, which is pretty much the worst that can happen.

 

When you say that nobody except you is going to touch the code, you're really missing the point of OOP. The whole reason for organizing the code with objects it to make it easier to understand. Again, you don't seem to know why you're even using OOP.

 

Classes need to make sense. A good approach is to take the role of somebody who doesn't know the code and has to use the class for the first time. Can they do it? This naturally leads to several rules:

• Objects should be fully usable after instantiation. If that's not possible, you need to document the exact steps and throw an exception when the user doesn't follow this protocol.
• Methods which aren't supposed to be called directly shouldn't be public at all. If they have to be public for some reason, they need to be clearly marked as internal.
• Give feedback. When something is wrong (e. g. parameters are missing or invalid), throw an exception which explains what happened. Never just stay quiet.

 

I'm not sure what you mean by a simple assignment?

 

This:

$arr_1 = [11, 12, 13];
$arr_2 = $arr_1;

creates a copy of $arr_1 and puts it into $arr_2. There's no need to manually copy the items from $arr_1 to $arr_2. In fact, you shouldn't do that, because the assignment approach is much smarter. It uses a copy-on-write mechanism where the items aren't actually copied until you start modifying the new array.

 

[deft_]

I see what you're saying. Like I said, I did throw this code together just to see if I could get something rendering to the page how I would want.. ie. with the $post variable so I can render out content like the title using $post->title. Therefore, you're right, the class has a lot of parts missing like not throwing exceptions or like you said if $_GET['p'] isn't set it ignores the request (actually it doesn't, it just redirects to 404 page which I would make redirect to homepage instead). I just haven't gotten to those parts yet. I guess maybe it would be better to just finish those parts, though, before moving on to other things, I just wanted to hurry and get something rendering to the page, then go back and refactor it.

 

As for others using the code, in this case people would not have access to the code, just the back-end and the option to create themes. However, there is a chance that another developer could work with the code to help extend it, in which case I agree it's definitely important to make it easy easy to understand and work with, but like I said I just haven't gotten to those parts yet.

 

Take your CMS class, for example: I create an instance, so now I should be ready to call a method like render(), right?

 

So you're saying the render method should be marked as internal? I haven't heard of this but I'll look into it.

 

Also, are you saying that all classes should be able to be used more than once? Are there any times that an object shouldn't be instantiated more than once? For instance, I don't see why anyone would create another instance of the CMS class as only one is needed and it's already been created.

 

As for why I'm using OOP, I'm pretty much using it because from what I understand, it's ideal if there's a chance of scaling your projects to something much larger in the future and also for modularity, although I'm not sure if I'm effectively doing that, hence this whole thread.

 

And last but not least, for the arrays, I'm assuming you're meaning the setArgs() method:

public function setArgs($args) {
    foreach($args as $key => $value) {
        $this->query_args[$key] = $value;
    }
}

I'm doing this so I can add to the query args array by calling setArgs() more than once to build up the query, this way it doesn't overwrite the query_args variable when setArgs() is ran twice. However I see now that the extractArgs() method is completely unnecessary, not sure why I had that in there haha. And also in the extractArgs() method I do see now that looping through the results to create a separate $values array is unnecessary and could be completed with a normal assignment.

Edited June 16, 2017 by deft_

[gizmola]

Just based on your responses, it seems you aren't clear on all the basic principles and syntax of php OOP and what they are used for generally.

 

In terms of studying CMS projects, my recommendation would be to look at one of the several cms projects built on top of symfony.  The reason I recommend symfony is that it has well designed component libraries and is widely used in the professional php world.  There are both successful commercial cms products and other projects that have less legacy code and will probably be easier to understand.

 

Just off the top of my head, there is EzPublish, Drupal (latest version was built using symfony components) and Bolt.  There's even a new one that I've not looked at previously named "Sulu" that looks interesting.  I would suggest getting one of these from github (probably Sulu or Bolt) and playing with it/debugging and looking at code.  Whenever you see something in the structure or classes that doesn't make sense to you, do some investigation into the syntax and try and surmise why it was used.

 

Typically you will learn a number of new tricks in a short amount of time, that you can apply.  

 

Unfortunately, OOP syntax is just a start, as most projects worth their while also utilize object oriented design patterns.  There are many books on the subject.  Many popular MVC frameworks utilize a pattern/philosophy called Dependency Injection.  The originator of the symfony project wrote these articles many years ago, and I think they are still a great introduction to the concept.  http://fabien.potencier.org/what-is-dependency-injection.html

 

For general design pattern study, there are now a lot of resources you can find if you google.  For one example:  http://www.fluffycat.com/PHP-Design-Patterns/

[Jacques1]

I would actually stay away from the CMS and any advanced OOP topic until you have a solid understanding of the basics. While you're able to write code which looks like OOP, you've adopted a very strange programming style which really doesn't make any sense to a reader other than you.

 

I understand this is a work in progress. My point is that the classes have fundamental design problems which I don't think can be fixed. I would start from scratch – with a simpler project.

 

When I talk about class users, I obviously mean programmers, not website visitors. It doesn't matter if the only programmer is you. Classes that don't make sense and cannot be extended are inherently bad and decrease the code quality of the project, regardless of whether there's actually somebody to complain about it.

 

 

 

So you're saying the render method should be marked as internal?

 

No. I'm saying that methods which aren't supposed to be public should in fact not be public. That's the whole point of distinguishing between public, protected and private.

 

The @internal tag is only for the special case when you need the method to be public (because it's called by some other object) but at the same time don't want it to be called like a regular method. This is not the case here and rare. I mentioned it for the sake of completeness.

 

 

 

Also, are you saying that all classes should be able to be used more than once? Are there any times that an object shouldn't be instantiated more than once? For instance, I don't see why anyone would create another instance of the CMS class as only one is needed and it's already been created.

 

Where did I say that there should be multiple CMS instances?

 

I'm saying that your CMS object should be workable and sensible and not make the programmer (whoever that is) jump through hoops. Not sure why you're having so much trouble with this idea.

 

Let's say you had to design a physical product like a smartphone. How would you do that? You would probably make sure it behaves in a sane way according to basic expectations: Pressing the power button turns it on, pressing the call button starts a new call etc. If the only way to use your phone is to tilt it at 45°, move the fingers in a zigzag pattern over the screen while reciting bible verses, or else it will blow up in the user's face, you would probably agree there's something wrong with the engineering process.

 

It's the same thing with objects. When I instantiate an object, I expect to have a fully working entity which behaves according to basic expectations (public methods may be called directly etc.).

 

 

 

I'm doing this so I can add to the query args array by calling setArgs() more than once to build up the query, this way it doesn't overwrite the query_args variable when setArgs() is ran twice.

 

This doesn't make sense. A setter by definition sets a new value. It doesn't aggregate values.

 

 

 

 

[deft_]

Lol @Jacques1, you're really condescending aren't you. Is it that hard to imagine someone starting with a concept that's completely new to them and as confusing as OOP actually is to actually have some trouble? I appreciate your help but damn, not everyone is going to understand everything right off the bat. I think your expectations are a bit high, it seems like you think I should know everything about OOP which I clearly don't and is why I'm creating a post asking for help. Chill a little bit lol.

[deft_]

Thanks @gizmola. I'll go ahead and look into Bolt and see if I can learn anything and I guess I'll try to go back over the basics of OOP because apparently I've missed some things.

Edited June 17, 2017 by deft_

[Jacques1]

I think your expectations are a bit high ...

 

 

That's a rather strange complaint when I've told you several times to start with a smaller, simpler project and not try to learn everything at once. The only high expectations came from yourself.

 

Telling people to "chill" does sound idiotic, though. I'm not your stoner friend.

[deft_]

I'm referring to your comment about not being sure why I'm having a hard time with this, and a few other things you said. When you started learning PHP did it all make sense to you right away? I assume not. And when I say chill, I just mean try not to be so harsh. I'm new to this and trying to learn, there's no need to be so condescending.

Edited June 17, 2017 by deft_

[ginerjm]

If one has "started learning PHP" does it make sense to jump into a project as complex as a CMS?  The point that was being made is learn PHP to the point that you clearly understand it and are fluent enough to see how and why a CMS may be something you want to create (although there are plenty of them out there already).  You are attempting to create a tool that is to be used to make PHP coding easier yet you yourself may not be ready to do that.

 

And yes, Jacques is tough on everyone - not just noobs.  His lack of forbearance is wholly compensated for by his extreme knowledge though.

[deft_]

Yeah I figured he was. And I'm not new to PHP, I know it well enough to create a CMS using procedural code. I was just giving it as an example that when you start to learn something new it takes time to grasp things fully. Anyway this thread is going in a direction I didn't want it to, so we'll leave it at that.

Edited June 17, 2017 by deft_

[Strider64]

I use forums like this to avoid writing obsolete  PHP code and there are a lot of obsolete tutorials out on the web. Jacques1 I find will help a noob from wasting his/her time by writing bad PHP code. I find going to the latest edition of a PHP Book is the way to go when learning PHP and PHP OOP. I have found Larry Ullman to be a good author to bridge the gap between procedural and OOP. I also find having a strong grasp of HTML/CSS helps out tremendously before even attempting PHP (procedural or OOP), for if you don't then your will struggle with PHP. It also helps to have basic knowledge of a programming language even if it is basic. I think my very first programming language was Fortran in college in the early 80s. For all programming language have the basic programming structure (if statements, loops, arrays, etc...). I'm finally getting the handle on PHP OOP myself, but I don't even consider myself an expert. I would probably go as far to I'm at advanced level, but even then there are times I have my doubts that I'm even that.

My point is the goal of writing OOP in my opinion is to have classes that are very portable from one web project to another without have to change anything in the classes or very minimal changes. I have this one class Calendar that I have develop that I simply move over to the current project and the only thing I have to change is the CSS. The classes will also save you a lot of unnecessary code in your projects and save you time in the development stage. A lot of people just learn the minimal amount of PHP OOP and use a framework to do the heavy lifting. There's nothing wrong with that for you're not reinventing the wheel and there are even times employers demand that you use a framework. I personally like writing my own PHP classes and I work for myself so I have that luxury.

 

I also found after obtaining a Graphics Designer degree in computer technology that people critiquing you or telling you the truth is way better than someone sugar coating it because they are afraid of hurting your feelings. I found asking family members or friends is the worst thing you can do when it comes to this. Just my .02 cents. 

Edited June 18, 2017 by Strider64

[deft_]

My point is the goal of writing OOP in my opinion is to have classes that are very portable from one web project to another without have to change anything in the classes or very minimal changes.

 

This is what I've been realizing after looking into it more. None of the tutorials I've followed so far have really gotten this point across. I guess the appeal of OOP for me has been that it's a good way to write code that looks cleaner and is more organized, unlike procedural which in my opinion can become ugly and bloated. I think that's a good enough reason to use OOP code right there, even if you're not really using it "efficiently". 

 

And I agree completely that critiquing is necessary, but there is a difference between critiquing and being condescending, and there's no reason to be so condescending, all I'm trying to say. I'm not saying his advice is bad.

Edited June 18, 2017 by deft_

[Moorcam]

Not sticking my nose in or anything but I would seriously take on board what Jacques1 is telling you. He/she is not trying to be nasty, but stern and there is a reason for it. Trust me, I thought Jacques1 was rude when I first started posting here but now I find their input very valuable and they have helped me greatly in realising my mistakes. One being that I do not use oop in my queries when I know now I should. But, just like you, I am learning.