• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Forum prototype - Constructive criticism requested.

Hi everyone. I've spent weeks on this, and I'm still not convinced I have it right. I've practically designed a whole forum already, and then get pissed off and end up doing a rewrite. I'd be interested to see how everyone else would do this. I've opted to use objects rather than a model. I'd appreciate any pointers - something I could change, a method I should add, a different structure etc...

Here's the prototype, I think it's fairly self-explanatory, and obviously there will be more methods added eventually.


class Forum {
    var $id,
    function Forum() {}
    function set_id() {} # Setting the id will reinitialize the object.
    function get_id() {}
    function set_title() {}
    function get_title() {}
    function set_description() {}
    function get_description() {}
    function set_lft() {} # Setting the lft will reinitialize the object.
    function get_lft() {}
    function set_rgt() {} # Setting the rgt will reinitialize the object.
    function get_rgt() {}
    function delete($forum_id=NULL) {} # Deletes this forum, and any forums/posts under it.
    function save() {} # Inserts/updates the database appropriately
    function _reset() {} # Resets the object.
    function move_up() {} # Moves the forum up one place within the confines of the parent forum.
    function move_down() {} # Moves the forum down one place within the confines of the parent forum.
    function get_posts($page_num) {} # retreives the posts from the forum
    function get_post($post_id) {} # Get's a single post from the database
    function add_post($post_obj) {} # Adds a post
    function mark_post_read($post_id, $user_id) {}
    function mark_forum_read($forum_id, $user_id) {}
    function delete_post($post_id) {} # Deletes a post
    function delete_all_posts($forum_id) {} # Deletes all posts under the specified forum
    function move_post($post_id, $forum_id) {} # Moves a post

class Forum_Post {
    var $id,
    function Forum_post() {}
    function set_id($id) {} # Reinitialises the object.
    function get_id() {}
    function set_user_id($user_id) {}
    function get_user_id() {}
    function set_subject($subject) {}
    function get_subject() {}
    function set_body($body) {}
    function get_body() {}
    function set_date($date) {}
    function get_date() {}
    function _reset() {} # Resets the object

Although I use objects all the time, I don't think I use them the way I should, but I'd like to, which is why I want to try to start off on the right foot. When I say "how I should", I mean that I only ever seem to need a single object of any one type instantiated at anyone time (like a model, or library).

[eluser]The Wizard[/eluser]
hello, why not simple divide it into a model?
it would be pretty cool i think.

i did a forum once without CI and it had issues but
yours looks pretty good though.

why not take a look under the hood of EE ?

Sadly, that's an endeavour which will cost me cash I can't afford right now. Whilst EE is free, the forum module is not. Also, I don't really understand EEs modular design. It just confuses the heck out of me.

Thanks for your comments.

I would have the following main objects
- forum
- thread
- post
- user

With the following relationships
- Has many forums
- Has many threads
- Belongs to only one forum

- Has many posts
- Belongs to only one forum

- Has one author
- Belongs to one thread

- Creates many posts

What does what
Pretty much the same as you've got, put also would need parent, so you can nest forums

Gets list of posts, add post to the end of the thread, get post count etc

Again, much the same as yours

Get/set user details, post history, unread posts, settings etc

One thing I'll say about yours is, not sure if some of your set methods are really needed (you may have just added them when you put the get?)
Such as:

The reason I say you don't need these, is cause they would be set in the contructor when you create that forum/post, and they don't really change, the only exception being post.date as it can be edited, but that'll be taken care of internally

You could also just use __get and __set if you want

If you want me to explain my points in more detail, then just ask, or contact me via PM


I've never used ORM before, but it sounds simple. I can't help feeling it's too restrictive when it comes to structure, but that's just speculation on my part - I've yet to look into it in detail.

Thanks for your suggestions. I find it helps to have a set of fresh eyes, and the opinions of people who have more common sense than I do.

As for those setters.

Forum.set_id - This was going to be used to initialise the object using the forum ID, as was set_lft and set_rgt.
Post.set_id - Again, this was going to initialise the post object.
Post.set_user_id - As this is something that shouldn't be changed, I was going to use the setter to prevent it being set. Probably not smart? I dunno, it sounded good to me at the time, and I thought it would help prevent out programming errors.
Post.set_date - Again, same as above. I just wanted to put the date into a format that MySQL would accept. I think I might just use a timestamp instead.

Thanks again for your comments.

I'm still not sure if I prefer the use of set_id to instantiate the object, or just a constructor which takes the ID and creates the object.
With the Post, you would need to have multiple Post objects, so using the set_id, you would be overwritng the current one, instead, you need a set of them, same for forum (or do you have some eveil genius up your sleeve to get around that?)

As for the setters which don't change, just make the variable private, and you don't have to worry about it (you are using PHP5, right?)

Also, how comes you decided to go the pure OOP route instead of MVC? I know you initialy lowered your demands from MVC as you couldn;t find something to get ideas from, but looks like you're doing it from scratch anyway

The idea of not using the constructor for the forum object, was so you could initialise the object with either the ID, the lft, or rgt value of the forum. With the post object, I did the same for consistency. It just made sense to me that if the ID is set, then we should be loading an existing entry for editing.

I thought I'd give OOP a try as it's not something I've really done before. I just wanted something that made sense, was easy to maintain and scalable. I am self-taught, so I tend to learn by example, but it's a real painus in the anus then you can't find any examples...

Another reason I thought I should go with objects, is that some methods call others several times, and each time that method makes a call to the database. I wanted to cache certain nodes within the object, to save unnecessary database calls, for example "get_root", "next_sibling" and so on, but I think this is complicating things.

What ORM library do you use? I was checking out IgnitedRecord, and it seems to be incomplete(?).

Thanks again for your comments.

Grrr, the forum just lost my reply (I hope yours won't do that Tongue)

If you're using set_id, set_lft, set_rgt to init the Forum object, then you might want to also add one of those as a parameter to your constructor, to save having to create it, and then init.

I'm also self taught, so i know what you mean, but its also part of the fun. And you'll learn so much more by doing it yourself from scratch then basing your design on someone elses. It'll make you ask why a lot more, you'll make more mistakes, and you'll (hopefully) learn from those mistakes.

I don;t think caching database data in the objects would work, as wouldn;t they be re-instantiated after every page request? The only stateful parts of a web app is the database, files, sessions and cookies

And I don;t use ORM, and no idea why you seem to be under the impression that I do :S
You've been the only one to mention it in this thread Tongue

Sometimes the forum does that when you've had the reply page open too long. I've learnt that hitting the back button, and clicking Fast Reply (assuming that's what you did), should show your post again. Then you need to copy it, and either refresh the page to use Fast Reply again, or just go through to the Post Reply page, and paste it in there.

I agree, caching objects only lasts for that request, but let's say I called get_root_node several times (once from my app, and several times from within the object itself), that's what the caching is for. Instead of making a call to the database several times, only one call is made, and then the cached data is passed back for subsequent requests. It sounded OK to me, but I couldn't find any examples to back it up, which is why I have been left so confused. I want to create the model part so it has the smarts to cut down on database calls, but I can't decide if that's good, or just adding unnecessary complications. After all, less calls to the database is certainly better.

It was your saying "has many", "has one", "belongs to one" and so on. As that's how you describe database objects in ORM, I assumed you used it.

[quote author="TheFuzzy0ne" date="1241197783"]Sometimes the forum does that when you've had the reply page open too long. I've learnt that hitting the back button, and clicking Fast Reply (assuming that's what you did), should show your post again. Then you need to copy it, and either refresh the page to use Fast Reply again, or just go through to the Post Reply page, and paste it in there.[/quote]
I know it sometimes does that, it happens to me on average 5 times a day, but normally my reply is still in the form, it wasn't there this time Sad

I think a database model would be a good idea, as it means all you Database logic is in a single class, which maikes it easier to maintain

And I just used “has many”, “has one”, “belongs to one” etc as it was a simple way of explaing the relationships... I didn;t know it was the ORM way lol

Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  

  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2021 MyBB Group.