2

I'm always confused as to which way is the best to do the following. For example, say we have two classes BlogPost and Tag, which correspond to the database tables Posts and Tags. Is it bad design to instantiate a class inside another class or should they be independent?

 class BlogPost { private $db; private $title; private $description; private $tagsArray; public function __construct($db){$this->db = $db;} public function getTitle(){/*...*/} public function getDescription(){/*...*/} public function getTags(){/*...*/} public function setTitle($title){/*...*/} public function setDescription($desc){/*...*/} public function setTags($tags){/*...*/} public function addPost(){ //insert into database query //bindParams //execute $this->addTags($this->db->last_insert_id()); } public function addTags($lastInsertId){ require_once('class.Tag.php'); foreach($tagsArray as $tagItem){ $tagClass = new Tag($this->db, $lastInsertId); $tagClass->setTagName($tagItem["title"]); $tagClass->addTag(); } } }

index.php

 require_once('class.BlogPost.php'); $blogPost = new BlogPost($db); $blogPost->setTitle("title"); $blogPost->setDescription("description"); $blogPost->setTags($tagsArray); $blogPost->addPost();

Or is it better to keep classes independent? Like so:

 class BlogPost { private $db; private $title; private $description; public function __construct($db){$this->db = $db;} public function getTitle(){/*...*/} public function getDescription(){/*...*/} public function getId(){/*...*/} public function setTitle($title){/*...*/} public function setDescription($desc){/*...*/} public function setId($id){/*...*/} public function addPost(){ //insert into database query //bindParams //execute $this->setId($this->db->last_insert_id()); } }

index.php

require_once('class.BlogPost.php'); $blogPost = new BlogPost($db); $blogPost->setTitle("title"); $blogPost->setDescription("description"); $blogPost->addPost(); $lastInsertId = $blogPost->getId(); require_once('class.Tag.php'); $tagClass = new Tag($db, $lastInsertId); $tag->setTags($tagsArray); $tag->addTag();

Thanks for any info!

Improve this question

4 Answers 4

Reset to default
2

Dependency injection is gaining a lot of steam in the PHP world right now, which would have you instantiate the dependent object outside and pass in the constructor or a setter. This is very useful for unit testing, and allows flexibility if your dependent object were to change. Even better would be to pass an interface, so if you changed something, say your database, no application code would need to change, just the base DB class or factory.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I just read Patrick's comment and I totally agree. Passing a class that implements "ITag" would be even more flexible, giving you the option to have both "Simple" and "Advanced" tags (just a theoretical example).
This is the obvious answer.
1

It's certainly not bad design. There's nothing in OO that says "you should not do this". Reducing dependencies is a goal, of course, but there can be no unnecessary dependency on the Tag class when you are executing an operation with the goal of creating Tags in the first place.

Since the Tag objects are dependent on the BlogPost due to database relations, it makes total sense to have a BlogPost method create them. By taking the code out you only "gain" the privilege of having to write the code manually on each occasion (and possibly getting it wrong).

Improve this answer

5 Comments

@Jimbo: The instances created inside addTags do not leak outside and even more, if you read the code carefully you will see that addTags itself should not even be public in the first place. What exactly do you believe is bad design and why?
Maybe we should edit your answer to clarify? I believed that your response to OP's question: Is it bad design to instantiate a class inside another class? was: "It's certainly not bad design". Lol!
@Jimbo: It depends on the context, don't you think?
I will never be one of those people that goes on about how "in very unique and obscure cases, this can actually be a good thing", personally :-)
@Jimbo: That's one way to see it.
0

In my opinion, I would create a "Tag" Factory class that generates "Tags", either one at a time or en masse, and then once you have all your "Tags" that you want to attach to a "BlogPost", pass that array to the "BlogPost" class.

It just makes for a cleaner design, IMO.

Improve this answer

Comments

-1

You should avoid using require/include for classes. Use a Class Loader

Improve this answer

1 Comment

This is not an answer to this question, instead, it should be a comment.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.