0

This CF Function is getting a timeout error. It's executed in a page that has more stuff going on. Is there a more efficient way to rewrite this function?

<cffunction name="GetTheSerialNumber" access="remote" returnformat="plain" returntype="string" output="no">
    <cfargument name="term" required="yes" />

    <cfsetting requesttimeout="20"/>
    <cfif NOT IsDefined("session.LoggedIn")>
        <cfabort />
    </cfif>

    <cfquery name="GetSerialNumber" datasource="#application.datasource#">
        SELECT [serialNum]
            ,MissionAsset.MissionAssetID
            ,MissionAssetDescription
            ,ESN.EfracusSerialNumberID
        FROM MissionAsset        
            LEFT JOIN EfracusSerialNumber ESN ON MissionAsset.MissionAssetID = ESN.MissionAssetID
        WHERE MissionAsset.isActive = <cfqueryparam value="1" cfsqltype="cf_sql_bit"> 
            AND serialNum LIKE <cfqueryparam value="%#arguments.term#%" cfsqltype="cf_sql_varchar"> 
            AND MissionAssetStatusID IN (2,3,4,5,6)
        ORDER BY serialNum
    </cfquery>

    <cfset stcReturn = '['>

    <cfloop query="GetSerialNumber">
        <cfset stcReturn &= '{"label":"#GetSerialNumber.serialNum#", "value":"#GetSerialNumber.serialNum#", "missionAssetID": "#GetSerialNumber.MissionAssetID#", "missionAssetDescription": "#GetSerialNumber.MissionAssetDescription#", "ID" : "#GetSerialNumber.EfracusSerialNumberID#"}'>
        <cfif GetSerialNumber.CurrentRow NEQ GetSerialNumber.RecordCount>
            <cfset stcReturn &= ",">
        <cfelse>
            <cfset stcReturn &= "]">
        </cfif>
    </cfloop>

    <cfreturn stcReturn>
</cffunction> 

Update: I took some of the suggestions below and optimized the function without having to change any of the front end. This is what we have now. Thanks!

<cffunction name="GetTheSerialNumber" access="remote" returnformat="plain" returntype="string" output="no">
    <cfargument name="term" required="yes">

    <cfsetting requesttimeout="20"/>
    <cfif NOT IsDefined("session.LoggedIn")>
        <cfabort />
    </cfif>
    
    <cfquery name="GetSerialNumber" datasource="#application.datasource#">
        SELECT [serialNum] as LabelValue, MissionAsset.MissionAssetID as MAID, MissionAssetDescription as MAD, ESN.EfracusSerialNumberID as ESNID
        FROM MissionAsset        
            LEFT JOIN EfracusSerialNumber ESN ON MissionAsset.MissionAssetID = ESN.MissionAssetID
        WHERE MissionAsset.isActive = <cfqueryparam cfsqltype="cf_sql_bit" value="1"> 
            AND serialNum LIKE <cfqueryparam cfsqltype="cf_sql_varchar" value="%#arguments.term#%"> 
            AND MissionAssetStatusID IN (2,3,4,5,6)
        ORDER BY LabelValue
    </cfquery>

    <cfset stcReturn = '['>
    <cfloop query="GetSerialNumber" >
        <cfset stcReturn &= '{"label":"#LabelValue#", "value":"#LabelValue#", "missionAssetID": "#MAID#", "missionAssetDescription": "#MAD#", "ID" : "#ESNID#"}'>
        <cfif GetSerialNumber.CurrentRow NEQ GetSerialNumber.RecordCount>
            <cfset stcReturn &= ",">
        </cfif>
    </cfloop>
    <cfset stcReturn &= ']'>

    <cfreturn stcReturn>
</cffunction>    
FSUKXAZ
  • 95
  • 7
  • How much time does the query take? This is most likely a performance issue with the query. – rrk Aug 14 '23 at 15:28
  • Take the if/else logic out of your loop. Since you appear to want to return an array of structures, do that inside your loop with array and struct functions. Or, if you want to build and return a string, build the string inside the loop and replace the final character afterwards. – Dan Bracuk Aug 15 '23 at 03:12
  • So something like this then? – FSUKXAZ Aug 16 '23 at 14:21

2 Answers2

1

The query is most likely the culprit; you'll have to check how it performs on the database independent of the application. Check indexes and the execution plan to see where bottlenecks may lie.

As for your code, first I'd make sure to scope the query to the function using the local scope. Then you can just serialize the query data to JSON using a built-in function, but how depends on what version of ColdFusion you're using.

<cfscript>
news = queryNew("id,title", "integer,varchar");
queryAddRow(news);
querySetCell(news, "id", "1");
querySetCell(news, "title", "Dewey Defeats Truman");
queryAddRow(news);
querySetCell(news, "id", "2");
querySetCell(news, "title", "Men walk on Moon");

writeDump(serializeJSON(news, "struct"));
</cfscript>

This returns an JSON array of objects using name/value pairs.

[
    {
        "ID": 1,
        "TITLE": "Dewey Defeats Truman"
    },
    {
        "ID": 2,
        "TITLE": "Men walk on Moon"
    }
]

If you alias the database column names, you can easily create the keys indicated in your JSON output. You may have to adjust the code that reads the key names to match the case returned by the function.

<cffunction name="GeTheSerialNumber" access="remote" returnformat="plain" returntype="string" output="no">
    <cfargument name="term" required="yes" />

    <cfsetting requesttimeout="20"/>
    <cfif NOT IsDefined("session.LoggedIn")>
        <cfabort />
    </cfif>

    <cfquery name="local.GetSerialNumber" datasource="#application.datasource#">
        SELECT 
            [serialNum] as label
            , [serialNum] as [value]
            , MissionAsset.MissionAssetID as missionAssetID
            , MissionAssetDescription as missionAssetDescription
            , ESN.EfracusSerialNumberID as ID 
        FROM MissionAsset        
            LEFT JOIN EfracusSerialNumber ESN ON MissionAsset.MissionAssetID = ESN.MissionAssetID
        WHERE MissionAsset.isActive = <cfqueryparam value="1" cfsqltype="cf_sql_bit"> 
            AND serialNum LIKE <cfqueryparam value="%#arguments.term#%" cfsqltype="cf_sql_varchar"> 
            AND MissionAssetStatusID IN (2,3,4,5,6)
        ORDER BY serialNum
    </cfquery>

    <cfreturn serializeJSON(local.GetSerialNumber, "struct")>
</cffunction> 

If you want more fine control, you can try the ArrayCollection.cfc I put together for older versions of ACF.

Adrian J. Moreno
  • 14,350
  • 1
  • 37
  • 44
  • We're on CF11. However, in this case it's bringing back a search to a text box as someone types, so regardless of version I'd prefer to do something different. It's older code we inherited. – FSUKXAZ Aug 14 '23 at 20:05
  • FWIW, I've gone through multiple projects to stop using built-in CF UI controls and replace them with more modern solutions. The core of that change is still the same: Find a more optimal data call and have it populate the UI control of your choice. I'd optimize this data call and serialization bottleneck before replacing the UI control. You can always change the JSON format to work with a new front-end control. – Adrian J. Moreno Aug 15 '23 at 22:47
  • Yeah, we won't be redesigning anything on the front end. Too much going on on that page and it's already more modern. That's why I'm looking for suggestions with this function. – FSUKXAZ Aug 16 '23 at 14:07
0

The question has an accepted answer, which is better than this one. This one will show how to make the code more efficient while maintaining a return type of string in the function.

<cfset var stcReturn = '['>
<cfset var records = GetSerialNumber.recordcount>

<cfloop 
     query="GetSerialNumber"
     StartRow = 1
     EndRow = records - 1>

    <cfset stcReturn &= 
          '{"label":"#serialNum#"
          ,"value":"#serialNum#"
          , "missionAssetID": "#MissionAssetID#"
          , "missionAssetDescription":#MissionAssetDescription#"
          , "ID" : "#EfracusSerialNumberID#"}
          , '>  <!--- note the comma here --->
   
</cfloop>

    <cfset stcReturn &= 
          '{"label":"#GetSerialNumber.serialNum[records]#"
          ,"value":"#GetSerialNumber.serialNum[records]#"
          , "missionAssetID": "#GetSerialNumber.MissionAssetID[records]#"
          , "missionAssetDescription":#GetSerialNumber.MissionAssetDescription[records]#"
          , "ID" : "#GetSerialNumber.EfracusSerialNumberID[records]#"}
          ] '>  <!--- note the square bracket here --->
    
Dan Bracuk
  • 20,699
  • 4
  • 26
  • 43