Alright, I'll start off by answering the questions you've asked, and depending if it all get's covered, I might append more at the end. We'll see, I have yet to actually read your questions!
So, let's grab the first one...
Will this design be handy when code get more and more complicated? More POST requests etc.
No. Right now, I see no way of expanding this. As soon as it does "get more and more complicated", you will run into lots of problems, especially with the website architecture.
Part of the Object Oriented paradigm is a principle called SOLID. If we start off with the S, which stands for Single Responsibility, we immediately notice multiple flaws in your code.
For instance, your
cms class is handling database connections, database querying, and session handling. So really, that tells us right off the bat that
cms isn't even necessary! It tells you that you really need a database connection handler, a querying utility, and a separate session handling class. But even those could be broken down further to prevent having all-powerful "Manager" classes.
The second principle in SOLID is code be open and closed. That means open for extension and closed to modification. I see neither in your code right now. We cannot build onto
cms or the index file or the demo page. And it's certainly going to be a pain to alter these classes, and since we even need to alter them, your code is not closed.
One possible solution would be follow KISS. Because your classes are responsible for so much, it's hard to keep things separated. This makes it difficult to change page-unique information. Give the functionality room to breathe, don't expect things to always fit in the same class, because that's not OOP.
Study the rest of SOLID, it's a large topic and I would be able to point out other mistakes, but it's best we move on!
could there be any security vulnerabilities?
Well, what are you afraid of? And under what circumstances are the code and the developer being placed in?
I say this because the threats will be different if you're the only one seeing this site, as opposed to the threats where the attacker knows your exact database scheme.
I'll point out some issues I see, but I cannot give a full security audit as I'm not a professional.
.inc.php is a convention, but can be considered a bad one. There are threats associated to using it if you leave off the appended
.php. Luckily, you look good! Be careful though.
- As of the PHP docs, using magic quote functions is deprecated. Avoid using any of these as support may fault and open you up to different types of attacks.
- Following that, you strip slash the page. However, that's only if the magic quotes are working. If they're not, the attacker can
include any page he wants (given the right permissions). I'd avoid giving the user so much power.
stripslashes($username) doesn't make sense. I'm not sure why you'd do this if you're using prepared statements. On top of this, I don't even know where you're getting
$username from. It's dead code, which is also a security risk.
This is what I could spot immediately, I'm sure I missed something though.
Any performance issues?
That's hard to say. Why don't you profile your code and see if it bottlenecks at any points? Then examine those parts and see about refactoring them.
From what I can tell, the database looks inefficient and you make a lot of unnecessary calls. Improper connection storage and caching also looks prevalent here. Every time you load a page, you start a new database connection, and then perform a whole new set of queries. Will the theme be changing on every single page load, or is that something you can store?
Do I utilize code properly as this is my real PHP project?
What does "properly" mean? I don't even know how to answer this! Are you asking if this is production worthy? If so, I'd automatically say no for all of the points I have and will point out.
I would love some intensive criticism about this code.
Well perfect, because I still have lots! :)
- First off, follow a PHP coding style standard. To start you off, I'll point you in the almost official PHP-FIG standard. It's important that every bit of code is consistent and readable. Follow the styles for naming things especially.
- Avoid shorthanding names such as "database" in names like
$dbhost. It doesn't provide any benefit to shorten. Same with "password" in
$dbdb is ridiculous. This obviously needs a new name.
$dbc has no meaning or clarity what-so-ever. Give your variable context!
- Things like
public $website should not be public. Encapsulate and give them getters and setters.
- Your constructor should not be creating those types of variables. The whole class needs to be rethought, and then apply dependency injection principles, learn more from the D in SOLID.
- Give your methods a visibility. Leaving the default is considered bad practice.
connect() is bad name. Connect to what? What is
cms connecting to? Oh! Maybe it needs to connect to the Google Patents API!
session() really doesn't belong here, we talked about this.
getWebsite() is extremely vague. This needs a lot of work.
$moduleArray has Hungarian notation and it seems confused about it's responsibility.
- You really need to lay off the single character variables (
- If your code needs comments such as
// end (GET || POST) && GET, then your conditional is too long. I shouldn't have to need a comment to know the different stages of the code.
Index.php has a lot of responsibility which could be out sourced to different files/classes.
$dbc = $cms->connect(); where is
$cms defined; this is dead code.
- I see you have 4 levels of nesting, you need to work on breaking this down. Either by simple refactoring, or if you can, guard clauses.
echo "error"; looks terrifying. Add in user-friendly and proper error handling. The Stack Exchange network is perfect for finding questions about how to do that.
- Having literals such as
<a href=\"index.php?page=page&title=$url\">$title</a></br /> are very hard to read. Either eleminate the HTML from the PHP (and do it dome where else), or use a function like
Well, that's what I could squeeze out! I hope you know what needs to be done :) I'd be happy to answer any questions.