patternphpMinor
Blog/Forum implementation
Viewed 0 times
blogimplementationforum
Problem
I've started learning PHP for university-related projects today. I tried implementing a basic blog/forum. Here's the full source code.
It has the following features/design-decisions:
I'm looking for a full code review: I've only read the PHP manual today and while I understand most of its features I have no idea what the best practices are and how things should be implemented.
I will list below some of my main concerns with the code/design.
I have a
It links the stylesheet at the beginning (and it could also import JS files):
Is this the correct way of adding the same `
function doBetweenStrong($lambda)
{
print("");
It has the following features/design-decisions:
- There are posts, which have the following fields: title, author, date and contents.
- Posts have comments, which have the following fields: author, date, contents.
- There is a basic login system: users can register/login with an username and password.
- JSON files are used for data storage instead of a MySQL database.
- There is a JSON file for posts,
posts.json, which contains posts and comments.
- There is a JSON file for users, which contains usernames and password hashes.
I'm looking for a full code review: I've only read the PHP manual today and while I understand most of its features I have no idea what the best practices are and how things should be implemented.
I will list below some of my main concerns with the code/design.
I have a
core.php file which is included with require in every other PHP file.It links the stylesheet at the beginning (and it could also import JS files):
Is this the correct way of adding the same `
to every file?
Every time I want to access the user database or the posts database, I call these functions:
function getUsersDB() { return new UsersDB(); }
function getPostsDB() { return new PostsDB(); }
However, creating a new UsersDB or PostsDB instance forces a read from the .json file. I need to access these databases in multiple PHP files.
Is there a way to "cache" these UsersDB and PostsDB instances? Something similar to C++'s static, that can be shared between all PHP scripts?
When I have to generate something between HTML tags, I just call print like this:
print("");
print($mPost["author"]);
print("");
Is there an easier/better way of generating open/close tags? Should I create a function that takes a lambda like:
``function doBetweenStrong($lambda)
{
print("");
Solution
First of all, it's good that you try to implement something like a blog/forum on your own. It will teach you a lot of good/bad practices. In one comment you said, that you are trying to understand how these frameworks could be impemented, so it would be very good, if you take a look at them. Check out their APIs and see what they did.
Im used to Symfony (http://symfony.com/) so my advice will mostly use their architecture as an example. However, be sure to remember that there are many ways to success and the things I describe are my current point of view.
Your major problems (maybe not now, but in a few days/weeks/months) are the structure of your project, the structure of your source code and the mixing of php/html.
Project Structure
Currently, you have a few php files in your root directory and a few php files in your PHP directory. On a first glance, it seems, that the PHP directory contains your framework and the root folder contains the views, which use this framework. If that's the case, make it obvious.
Advice #1: Give your framework a name and rename the folder accordingly.
That way, you will have an easier time to find the place you are looking for. Which leads to the next problem: Your core.php contains multiple class definitions. If you continue doing this, your file will grow exponentially and it will take you a long time to find the line you want to modify.
Advice #2: Put each php class in a separate file.
The next step is to use namespaces to further structure your classes and files. It might be more difficult to begin with, but will help you in the long run. Additionally you might want to look into autoload. Used correctly, this should solve your troubles with
Advice #3: Use Namespaces and autoload
Another possible improvement are the files in your root directory. Currently you probably access each file directly via www.whatev.er/viewPost.php or www.whatev.er/createPost.php. I highly recommend to take a look at something like a controller/routing mechanism and only use ONE file (index.php) to access your website. Take a look at .htaccess and URL Rewriting. This is actually a pretty powerful and extensive technique. So just a short explanation: Every (nearly every) call to your website is redirected to your index.php. So www.whatev.er/blub/bla/1234 or www.whatev.er/post/create all call your index.php. In the index file, you can parse the requested uri and call the appropriate actions.
Advice #4: Use .htaccess and URL Rewriting and only one file as an entry point to your website.
Advice #5: Use some kind of controller/routing mechanism to handle actions depending on your route.
Source Code Structure
As already mentioned, only define one class per file. I like it, that you encapsulate the fields of your classes. However, you have many static functions, which basically degrades your classes to some kind of namespaces. There is nothing wrong with static functions, but be careful not to overuse them.
Advice #6: Don't overuse static functions (and statics in general)
As you mentioned,
The use of global objects is generally discouraged, but migh be okay given your current experience. Another (slightly) better option is to instantiate the database once and pass it to the method that needs the database. The imho best option is to use dependency injection. However, this is pretty advanced stuff and something you want to look into in the future.
Advice #7: Use dependency injection if possible. Globals might be okay for now, but lead to trouble in the future.
You say, that you have posts and comments. However, in your code you only deal with assoc arrays. Why don't you create classes for Post and Comment? This is the perfect place to use a class.
Advice #8: Use classes for entites (Post/Comment) and not assoc arrays.
If you use classes for Post/Comment, you might wonder how to write them to json. PHPs jsonSerialize will help you there. However, you probably have to convert json -> Post/Comment yourself.
Advice #9: Use jsonSerialize or something similar to serialize objects to json.
Mixing of PHP/HTML
This is probably the problem, that will annoy you the most in the future. As it will make dealing with outputting stuff way more complicated than it needs to be. I highly encourage you to use some kind of templating system to render templates and display html. Don't ever output or print html code in a php file. Take a look at twig (used in eg. Symfony). Something like that will extremely improve your codebase. Your problem with printing will vanish and you can use common parts of html code without any problems.
Advice #10: Use some kind of templating mechanism to output HTML.
Im used to Symfony (http://symfony.com/) so my advice will mostly use their architecture as an example. However, be sure to remember that there are many ways to success and the things I describe are my current point of view.
Your major problems (maybe not now, but in a few days/weeks/months) are the structure of your project, the structure of your source code and the mixing of php/html.
Project Structure
Currently, you have a few php files in your root directory and a few php files in your PHP directory. On a first glance, it seems, that the PHP directory contains your framework and the root folder contains the views, which use this framework. If that's the case, make it obvious.
Advice #1: Give your framework a name and rename the folder accordingly.
That way, you will have an easier time to find the place you are looking for. Which leads to the next problem: Your core.php contains multiple class definitions. If you continue doing this, your file will grow exponentially and it will take you a long time to find the line you want to modify.
Advice #2: Put each php class in a separate file.
The next step is to use namespaces to further structure your classes and files. It might be more difficult to begin with, but will help you in the long run. Additionally you might want to look into autoload. Used correctly, this should solve your troubles with
include and require.Advice #3: Use Namespaces and autoload
Another possible improvement are the files in your root directory. Currently you probably access each file directly via www.whatev.er/viewPost.php or www.whatev.er/createPost.php. I highly recommend to take a look at something like a controller/routing mechanism and only use ONE file (index.php) to access your website. Take a look at .htaccess and URL Rewriting. This is actually a pretty powerful and extensive technique. So just a short explanation: Every (nearly every) call to your website is redirected to your index.php. So www.whatev.er/blub/bla/1234 or www.whatev.er/post/create all call your index.php. In the index file, you can parse the requested uri and call the appropriate actions.
Advice #4: Use .htaccess and URL Rewriting and only one file as an entry point to your website.
Advice #5: Use some kind of controller/routing mechanism to handle actions depending on your route.
Source Code Structure
As already mentioned, only define one class per file. I like it, that you encapsulate the fields of your classes. However, you have many static functions, which basically degrades your classes to some kind of namespaces. There is nothing wrong with static functions, but be careful not to overuse them.
Advice #6: Don't overuse static functions (and statics in general)
As you mentioned,
getUsersDB() and getPostsDB() is suboptimal. You have pretty much three options:- Use global objects
- Pass a reference where it is needed
- Use dependency injection
The use of global objects is generally discouraged, but migh be okay given your current experience. Another (slightly) better option is to instantiate the database once and pass it to the method that needs the database. The imho best option is to use dependency injection. However, this is pretty advanced stuff and something you want to look into in the future.
Advice #7: Use dependency injection if possible. Globals might be okay for now, but lead to trouble in the future.
You say, that you have posts and comments. However, in your code you only deal with assoc arrays. Why don't you create classes for Post and Comment? This is the perfect place to use a class.
Advice #8: Use classes for entites (Post/Comment) and not assoc arrays.
If you use classes for Post/Comment, you might wonder how to write them to json. PHPs jsonSerialize will help you there. However, you probably have to convert json -> Post/Comment yourself.
Advice #9: Use jsonSerialize or something similar to serialize objects to json.
Mixing of PHP/HTML
This is probably the problem, that will annoy you the most in the future. As it will make dealing with outputting stuff way more complicated than it needs to be. I highly encourage you to use some kind of templating system to render templates and display html. Don't ever output or print html code in a php file. Take a look at twig (used in eg. Symfony). Something like that will extremely improve your codebase. Your problem with printing will vanish and you can use common parts of html code without any problems.
Advice #10: Use some kind of templating mechanism to output HTML.
Context
StackExchange Code Review Q#66577, answer score: 4
Revisions (0)
No revisions yet.