Is this a good approach to dealing with tagging?

Question

Can this code be optimized or re-factored? Is this an optimal approach to tagging?

The following code is a callback in my posts model. It creates a record that associates a tag with a post in a QuestionsTags joiner table. When necessary, if a given tag does not already exist in the tags table, the function creates it, then uses its id to create the new record in the QuestionsTags table.

The difficulty with this approach is the QuestionsTags table depends on data in the tags table which may or may not exist.

The function assumes the following tables:

tags(id, tagName),
posts(tags) // Comma delimited list
questionsTags(postId, tagId)

The idea is to loop over a delimited list of tags submitted with a post and check to see if each tag already exists in the tags table

If tag exists:

1. Check to see if there's already a QuestionTag record for this post and this tag in the QuestionTags table.
2. If yes, do nothing (the association already exists)
3. If no, create a new QuestionTag record using the id of the existing tag and the postId

If tag does not already exist:

1. Create the new tag in the tags table
2. Use its id to create a new QuestionsTags record

Code

/**
* @hint Sets tags for a given question.
**/
private function setTags()
{
    // Loop over the comma and space delmited list of tags
    for (local.i = 1; local.i LTE ListLen(this.tags, ", "); local.i = (local.i + 1))
    {
        // Check if the tag already exists in the database
        local.tag = model("tag").findOneByTagName(local.i);
        // If the tag exists, look for an existing association between the tag and the question in the QuestionTag table
        if (IsObject(local.tag))
        {
            local.questionTag = model("questionTag").findOneByPostIdAndTagId(values="#this.postId#,#local.tag.id#");
            // If no assciatione exists, create a new QuestionTag record using the tagId and the postId
            if (! IsObject(local.questionTag))
            {
                local.newQuestionTag = model("questionTag").new(postId = this.id, tagId = local.tag.id);
                // Abort if the saving the new QuestionTag is unsuccesful
                if (! local.newQuestionTag.save())
                {
                    return false;
                }
            }
        }
        // If the tag does not exist create it
        else
        {
            local.newTag = model("tag").new(tagName = local.i, userId = this.ownerUserId);
            // Abort if the the new tag is not saved successfully
            if (! local.newTag.save())
            {
                return false;
            }

            // Otherwise create a new association in the QuestionTags table using the id of the newly created tag and the postId
            local.newQuestionTag = model("questionTag").new(postId = this.id, tagId = local.newTag.id);
            // Abort if the new QuestionTag does not save correctly
            if (! local.newQuestionTag.save())
            {
                return false;
            }
        }
    }
}

FYI: I'm using CFWheels in my application, which explains the ORM functions used.

Answer 1

That's pretty much how I would approach it. Curious why you're using ", " as the delimiter? What if the "user,didn't,leave,a,space"? I'd just use comma and trim() the list element.