CODEWITHSUNDEEP
coldfusion
user1978340
user1978340

Reputation: 101

ColdFusion having multiple cfqueries in cffunction

I am trying to write a function for a survey where it pulls questions from a database. The catch is that there are both active and unactive questions. I need older questions to show up when someone views the results from an old survey.

Here is the code I am trying within a CFC:

<cffunction name="getAllQuestions" access="public" returntype="query">
    <cfargument name="survey" default=0>
    <cfif len(#survey#) gt 0>
    <cfquery name="getsdate" datasource="blah.database">
    select * from past_survey
    where survey_id = #survey#
    </cfquery>
    <cfreturn getsdate>
    </cfif>
    <cfquery name="getquestions" datasource="blah.database">
        select * from pool_questions
        <cfif len(#survey#) eq 0> 
        where active_flag='Y'
        <cfelse>
        where <cfqueryparam value="#dateformat 
                       (getsdate.survey_date, "yyyy/mm/dd")#"> BETWEEN start_date AND  
                       end_date
        </cfif>   
        order by qtn_nb
    </cfquery>
    <cfreturn getquestions>
</cffunction>

#survey# is the survey id which is generated by the form. What I am trying to do is that if survey has a value to run query getsdate. Then the second query would run no matter if survey has a value or not. If there is not value it should pull all active questions. If there is a value then it should check if the survey date is between the start date and end date for past questions.

Any advice on how to make this work would be greatly appreciated. Thank you in advance!

Upvotes: 0

Views: 1348

Answers (2)

Tim Cunningham
Tim Cunningham

Reputation: 329

    <cffunction name="getAllQuestions" access="public" returntype="struct">
        <cfargument name="survey" required="true" default="0" type="numeric">

        <cfset var qryReturn                    = ""> <!---Always var scope your variables to prevent them from leaking to other functions --->
        <cfset var structReturn                 = structNew()>
        <cfset structReturn.pastSurvey          = "">
        <cfset structReturn.surveyQuestions     = "">

        <cfif survey GT 0>
            <cfquery name="qryReturn" datasource="blah.database">
                SELECT * 
                FROM   past_survey
                <!--- Always cfqueryparam to prevent SQL injection attacks & also always reference the scope to prevent confusion --->
                WHERE  survey_id = <cfqueryparam cfsqltype="cf_sql_integer" value="#arguments.survey#">
            </cfquery>
            <cfset structReturn.pastSurvey = qryReturn>
       <cfelse>
            <cfquery name="qryReturn" datasource="blah.database">
                 SELECT *
                 FROM pool_questions
                 <cfif arguments.survey EQ 0>
                     WHERE active_flag = 'Y'
                 <cfelse>
                     WHERE <cfqueryparam value="#dateformat 
                       (getsdate.survey_date, "yyyy/mm/dd")#"> BETWEEN start_date AND  
                       end_date
                 </cfif>
                 ORDER BY qtn_nb
             </cfquery>
             <cfset structReturn.surveyQuestions = qryReturn>
        </cfif>

        <cfreturn structReturn>
    </cffunction>

You probably should be doing this in two separate functions, but I will attempt to answer your question.

My code will return a struct of queries (you can change to an array if you prefer) that returns a past survey and the survey questions

Note: In your example code, you have a few bad practices.

  1. You are checking the length of the survey value rather than checking the value itself.
  2. If you want to ensure that survey always has a value regardless of if it is passed or not, set requried=true and give it a default value.
  3. Use cfqueryparam to prevent sql injection attacks
  4. Any variables created in the function need to be var scoped to prevent them from leaking to other cffunctions in the same cfcomponent. I always do this at the top. Yes, even the name you give a cfquery needs to be var scoped.
  5. Since you are doing a return after your first query, if the survey value is greater than 0 it will never get to the second query where it has the date check.

Upvotes: 4

Dan Bracuk
Dan Bracuk

Reputation: 20804

I see the following problems you need to address.

First, your survey argument has a default value of 0 and you are doing conditional logic on the length of it. The length of "0" is 1, so that condition will always return true.

Next, you state that you want the 2nd query to run whether the first one runs or not, but you refer to a value from the 1st query in the 2nd one. That means if the 1st query does not run, the 2nd one will crash due to an undefined variable.

Next, dateformat returns a string. Applying it the way you do in the 2nd query is at best unnecessary, and at worse, and indication that you are storing the start and end dates in pool_questions as strings. If you are attempting to strip out the time portion of the datefield in the first query, ColdFusion has a cast() function for that.

Also, scope your variables. ie - instead of <cfif len(survey), do this <cfif len(arguments.survey).

Also, var your local variables. In this case, it's the names of your two queries.

That should get you started.

Upvotes: 1

Related Questions