Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

    <cfargument name="Key" required="true" type="string" />
    <cfargument name="Value" required="true" type="any" />

    <cfif NOT StructKeyExists( Session, this.LOCAL ) >
        <cfset Session[this.LOCAL] = StructNew() />
    </cfif>

    <cfif NOT StructKeyExists( Session[this.LOCAL], "Data" ) >
        <cfset Session[this.LOCAL]["Data"] = StructNew() />
    </cfif>

    <cfif NOT StructKeyExists( Session[this.LOCAL]["Data"], "AvailableOptions" ) >
        <cfset Session[this.LOCAL]["Data"]["AvailableOptions"]  = StructNew() />
    </cfif>

    <cfreturn Set( Session[this.LOCAL]["Data"]["AvailableOptions"], Arguments.Key, Arguments.Value ) />

</cffunction>

<!--- /// Return Data value for the supplied key --->
<cffunction name="GetAvailableOption"   output="false"  access="public">
    <cfargument name="Key" required="true" type="string" />

    <cfif StructKeyExists( Session, this.LOCAL ) AND 
          StructKeyExists( Session[this.LOCAL], "Data" ) AND
          StructKeyExists( Session[this.LOCAL]["Data"], "AvailableOptions") >
        <cfreturn Get(Session[this.LOCAL]["Data"]["AvailableOptions"], Arguments.Key) />
    <cfelse>
        <cfreturn "" />
    </cfif>
</cffunction>

<!--- /// Returns parameter value of supplied key --->
<cffunction name="GetParam" output="false"  access="public">
    <cfargument name="Key" required="true" type="string" />
    <cfargument name="Default" required="false" type="string" default="" />

    <cfif StructKeyExists( Session, this.LOCAL ) AND StructKeyExists( Session[this.LOCAL], "Params" ) >
        <cfreturn Get(Session[this.LOCAL]["Params"], Arguments.Key, Arguments.Default) />
    <cfelse>
        <cfreturn Arguments.Default />
    </cfif>
</cffunction>

<!--- /// Sets paramter --->
<cffunction name="SetParam" output="false"  access="public">

    <cfargument name="Key" required="true" type="string" />
    <cfargument name="Value" required="true" type="any" />

    <cfif NOT StructKeyExists( Session, this.LOCAL ) >
        <cfset Session[this.LOCAL] = StructNew() />
    </cfif>

    <cfif NOT StructKeyExists( Session[this.LOCAL], "Params" ) >
        <cfset Session[this.LOCAL]["Params"] = StructNew() />
    </cfif>

    <cfreturn Set(Session[this.LOCAL]["Params"], Arguments.Key, Arguments.Value) />

</cffunction>
share|improve this question
1  
Fortunately Mark A Kruger is nice enough to read your code and help you out, but please use more descriptive titles and provide some context. Title should indicate what you're trying to do (imagine if you were searching for this, what would you type?) and you should add a description of what you believe/know the code does, and mention anything you'd particularly like to call out for review. Just posting a block of code isn't very helpful, and (generally) will not net good results. –  dimo414 Jun 18 '12 at 14:49
add comment

2 Answers 2

I really hate the idea of reaching out to the Session from within the function to access variables that you have stored in the session. It ruins the whole encapsulation model that is one of the main points of a component and function to begin with. Doing it this way you might as well just access and fix session vars in-line in a procedural manner. you are not saving anything but you ARE really complicating the session.

The session already has keys and structures - why are you trying to reinvent the wheel?

<!--- /// Return Data value for the supplied key --->
<cffunction name="GetAvailableOption"   output="false"  access="public">
    <cfargument name="Key" required="true" type="string" />

    <cfif StructKeyExists( Session, this.LOCAL ) AND 
          StructKeyExists( Session[this.LOCAL], "Data" ) AND
          StructKeyExists( Session[this.LOCAL]["Data"], "AvailableOptions") >
        <cfreturn Get(Session[this.LOCAL]["Data"]["AvailableOptions"], Arguments.Key) />
    <cfelse>
        <cfreturn "" />
    </cfif>
</cffunction>

This is little different than simply doing:

<cfparam name="session.something.#key#" value=""/>

<cfoutput>#sessions.something[key]#</cfoutput>

Where "key" is the value you are trying to "get".

If you really "must" do it this way then pass a reference to the session in as an argument along with anything else you need.

It also bears in mind that somethings are passed by value and some by reference. When you cross scopes and destroy encapsulation you open up referential problems where a reference is both in the session and somewhere else. If this happens across say - the application scope - you could end up with a code related memory problems that are devilishly difficult to find and fix.

share|improve this answer
    
Thanks a lot for your suggestions. Can you please suggest how should I save search form values. Is there any better approch to do this in the component? –  winlite Jun 18 '12 at 14:04
1  
Winlite - you can certainly save them in the session - just make sure you are restricting growth appropriately. REmember the session is appropriate for saving things from one request to the next - so if that is what you are trying to do then it's ok - but like everything it has benefits and costs. –  Mark A Kruger Jun 18 '12 at 14:16
    
Thanks a lot once again. Would you suggest to access and fix session vars in an in-line style and should not use the compoent atleast for session handling? –  winlite Jun 18 '12 at 14:19
    
Winlite - again, the question greatly depends on your architecture. I have put beans in the session before and it makes sense to do so sometimes. But if you are say... storing the last 10 searchs someone made then maybe not. Maybe you just create session.searchArray and populate it with search strings and use them directly. Nothing prevents you from doing so and the choice is one that is influenced by dozens of variables beyond the scope of this little review tool I would think. –  Mark A Kruger Jun 18 '12 at 14:23
add comment

I would take Mark A Kruger's answer and rewrite in in cfscript. I would also get rid of the else as it adds not meaning to the overall function

void function getAvailableOption(required string key) output="false" access="public" {

if (StructKeyExists( Session, this.LOCAL ) AND 
      StructKeyExists( Session[this.LOCAL], "Data" ) AND
      StructKeyExists( Session[this.LOCAL]["Data"], "AvailableOptions")) {
  return Get(Session[this.LOCAL]["Data"]["AvailableOptions"], Arguments.Key);
  }  

return "";
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.