Using the request scope to store an object

Question

I'm using CFWheels to develope an application in ColdFusion.

I have a model called Vote.cfc. Before a Vote object can be created, updated, or deleted, I need to fetch a post object from another model: Post.cfc. A Vote belongs to a Post. A post has many votes.

Using data from the post object, I need to validate the vote object across multiple criteria and several functions. The only way I can think of persisting the post object so that it's available to those functions is by storing it in the request scope.

Others have said this is bad practice. But I was not able find out why. I thought the request scope was thread safe, and it would make sense to use it in this situation.

My other alternative is to load a new instance of the post object in every function that requires it. Although Wheels uses caching, doing this caused request times to spike by 250%.

UPDATE

Here's some samples. First, the controller handles looking to see if a vote object already exists. If it does, it deletes it, if it does not, it creates it. The controller function is essentially a toggle function.

Votes.cfc Controller

   private void function toggleVote(required numeric postId, required numeric userId)
    {
    // First, we look for any existing vote
    like = model("voteLike").findOneByPostIdAndUserId(values="#arguments.postId#,#arguments.userId#");

    // If no vote exists we create one
    if (! IsObject(like))
    {
        like = model("voteLike").new(userId=arguments.userId, postId=arguments.postId);
    }
    else
    {
        like.delete()
    }           
}

Model VoteLike.cfc

After that, a callback registered in the model fires before validation. It calls a function that retrieves the post object the vote will belong to. The function getPost() stores the post in the request scope. It is now made available to a bunch of validation functions in the model.

// Load post before Validation via callback in the Constructor
beforeValidation("getPost");

private function getPost()
{
    // this.postId will reference the post that the vote belongs to
    request.post = model("post").findByKey(this.postId);
}

// Validation function example
private void function validatesIsNotOwnVote()
{
    if (this.userId == request.post.userId)
    {
       // addError(message="You can't like your own post.");
    }
}

The alternative to the getPost() function is to use a scoped call "this.post().userId" to get the post object, as such:

private void function validatesIsNotOwnVote()
{
    if (this.userId == this.post().userId)
    {
        addError(message="can't vote on your own post")
    }
}

But I would then have to repeat this scoped call this.post().userId for EVERY function, which is what I think is slowing down the request!

Answer 1

Update with new answer based on comment thread in OP

Since you're extending the base Vote object form VoteLike.cfc, the CFCs share a local thread-safe variables scope (so long as you're not storing this in Application or Server scope where it can be fiddled with by others). This means that you set a value, such as Variables.Post, once, and reference that in any function within the stack of CFCs.

So change your getPost() function to this:

beforeValidation("getPost");

private function getPost(){
    Variables.Post = model("post").findByKey(this.postID);
}

Now, within any function in either VoteLike.cfc, you can reference Variables.Post. This means that you have a single instance of Post in the local CFCs variables scope, and you don't have to pass it around via arguments or other scopes.

Original Answer below

The "most correct" way to handle this would be to pass the object to each individual function as an argument. That, or add the Post as a property of the Vote object, so that the Vote object has access to the full Post object.

<cffunction name="doTheValidation">
    <cfargument name="ThePost" type="Post" required="true" />
    <cfargument name="TheVote" type="Vote" required="true" />
    <!--- do stuff here --->
</cffunction>

The reason this is a bad practice is because you're requiring that your objects have access to an external scope, that may or may not exist, in order to do their work. If you decided that you wanted to deal with multiple votes or posts on a single page, you then have to fiddle with the external scope to get things to work right.

You're far better off passing your object around, or using composition to piece your objects together in such a fashion that they're aware of each other.

Update

Regarding performance issues, if you use composition to tie your objects together, you end up with just two objects in memory, so you don't have to instantiate a bunch of unnecessary Post objects. Also, passing a CFC into a function as an argument will pass the CFC by reference, which means that it's not creating a duplicate of the CFC in memory, which should also take care of your performance concerns.

Code Sample Update

To illustrate the "by reference" comment above, set up the following files and put them in a single directory on their own.

TestObject.cfc:

<cfcomponent output="false">
    <cfproperty name="FirstName" displayname="First Name" type="string" />
    <cfproperty name="LastName" displayname="Last Name" type="string" />

</cfcomponent>

index.cfm:

<!--- Get an instance of the Object --->
<cfset MyObject = CreateObject("component", "TestObject") />

<!--- Set the initial object properties --->
<cfset MyObject.FirstName = "Dan" />
<cfset MyObject.LastName = "Short" />

<!--- Dump out the object properties before we run the function --->
<cfdump var="#MyObject#" label="Object before passing to function" />

<!--- Run a function, sending the object in as an argument, and change the object properties --->
<cfset ChangeName(MyObject) />

<!--- Dump out the object properites again, after we ran the function --->
<cfdump var="#MyObject#" label="Object after passing to function" />

<!--- Take a TestObject, and set the first name to Daniel --->
<cffunction name="ChangeName">
    <cfargument name="TheObject" type="TestObject" required="true" hint="" />
    <cfset Arguments.TheObject.FirstName = "Daniel" />
</cffunction>

You'll notice when you run index.cfm, that the first dump has the FirstName as Dan, while the second has the FirstName as Daniel. That's because CFCs are passed to functions by reference, which means that any changes made within that function are made to the original object in memory. Hence no recreation of objects, so no performance hit.

Dan

Answer 2

request is a global scope, confined to the page request (i.e. thread safe), that lives for the duration of that page request. So it's bad practice in the same way that global variables are a bad practice, but of a short lived duration. I think of it as throwing the data up in the air or over the fence in the situation you described, where any other code can just pluck out of mid air.

So for your situation it's probably just fine - add some useful reference at the point of consumption perhaps about where the data's being placed into the request scope in the first place. That stated, if you are going back to the same source method each time , consider caching inside whatever function is responsible creating that object therein (e.g. getVote()) And, you could use the request scope for that as such:

<cfparam name="request.voteCache" default="#structNew()#"/>
<cffunction name="getVote" output="false" access="public" returntype="any">
    <cfargument name="voteId" type="string" required="true"/>
    <cfif structKeyExists(request.voteCache, arguments.voteId)>
        <cfreturn request.voteCache[arguments.voteId]>
    </cfif>

    <!--- otherwise get the vote object --->
    <cfset request.voteCache[arguments.voteId] = vote>
    <cfreturn vote>
</cffunction>

Downside is if something else changes the data during the request you'll have a stale cache, but it appears you don't expect any changes during the execution.