Adding new or existing tags to a blog post

Question

I'm working on a simple blogsystem (for learning purposes, not to reinvent the wheel). In this system the user can add existing or new tags to a blogpost. I wrote a method to achieve this, but it's ugly, inconvenient and probably of bad performance.

Some facts:

• I make use of Propel as mapper to a PostgreSQL database
• relevant tables in this part are tag and post_tag

CREATE-Statements:

CREATE TABLE tag (
    id integer NOT NULL,
    name character varying(100),
    slug character varying(100),
    modified timestamp(0) without time zone
);

CREATE SEQUENCE tag_id_seq
    START WITH 1
    INCREMENT BY 1
    NO MINVALUE
    NO MAXVALUE
    CACHE 1;


ALTER TABLE ONLY tag
    ADD CONSTRAINT tag_pkey PRIMARY KEY (id);

ALTER TABLE ONLY tag ALTER COLUMN id SET DEFAULT nextval('tag_id_seq'::regclass);

CREATE TABLE post_tag (
    post_id integer NOT NULL,
    tag_id integer NOT NULL
);

ALTER TABLE ONLY post_tag
    ADD CONSTRAINT post_tag_pkey PRIMARY KEY (post_id, tag_id);


ALTER TABLE ONLY post_tag
    ADD CONSTRAINT post_tag_post FOREIGN KEY (post_id) REFERENCES post(id) ON UPDATE CASCADE ON DELETE CASCADE;


ALTER TABLE ONLY post_tag
    ADD CONSTRAINT post_tag_tag FOREIGN KEY (tag_id) REFERENCES tag(id) ON UPDATE CASCADE ON DELETE CASCADE;

• The method is called after submitting the post. It gets the postID and an array containing all tags assigned to this post. It adds new tags in the table tag and makes sure that in table post_tag all new, all previously existing and all previously existing but not assigned tags, which the user chose, are connected to the post.

This is the method:

      public function addTagByPost($posttags, $postid){

        //collect all existing tags
        $all = $this->getAllTags();

        $existing = array();
        foreach($all AS $tags){
            $existing[$tags->getName()] = $tags->getId();
        }

        //delete all tags linked to the post
        $links = PostTagQuery::create()
            ->filterByPostId($postid);

        $links->delete();

        foreach($posttags AS $tag){
            if(!isset($existing[$tag])){
                //add new tags from post
                $newtag = new Tag();
                $newtag->setName($tag);
                $newtag->setSlug($this->generateSlug($tag));
                $newtag->save();
                $tagid = $newtag->getId();
            }else{
                $tagid = $existing[$tag];
            }

            //link all tags to the post
            $link = new PostTag();
            $link->setPostId($postid);
            $link->setTagId($tagid);
            $link->save();

        }
    }

Imagine in table tag are three rows, PHP, MySQL and PostgreSQL. Imagine there is a blogpost tagged with PHP and PostgreSQL. The user edits the post, removing the tag PHP and adding the tag Python. When he submits, the current tags are PostgreSQL and Python.

At first, all existing tags are fetched from db (PHP, MySQL and PostgreSQL). Then all links between the current post and its tags are deleted from table post_tag (PHP and PostgreSQL).

Now the tags assigned by the user (PostgreSQL and Python) are matched against the existing ones (PHP, MySQL and PostgreSQL). As Python does not already exist, it is added to table tag.

Now, all userassigned tags are accessible via id. These IDs are then inserted into table post_tag (PostgreSQL and Python), so the post is connected to them.

Please feel free to make suggestions.

Answer 1

Take a step back and look at your code. Do the different classes/objects ha ve Single responsibility? Or is the same class responsible for adding posts, managing tags and coupling a tag to a post?

I think it is time to go back to the drawing board and see what you actually want/need.

Then every class you write should only have a single responsibility. e.g. representing a comment or a TaggableComment class that extends the Comment class and adds the ability to add and remove tags.

Also make sure that a Class shouldn't have to know how to handle the creation of other objecs. Using the 'new' operator inside a class methods is bad practice. If you would like to change the way a Tag is constructed, you will now have to change the Comment class aswell, not useful.

Look into some programming patterns, A really good read is: http://addyosmani.com/resources/essentialjsdesignpatterns/book/ I know, it's not php but javascript. But moest of the patterns there also apply to php, it's the idea behind the patterns that will help you