2
\$\begingroup\$

I have an app that does a fair bit of text-parsing based on free-form user inputs. In this, I run searches on the whole string, and then on the last value of the string separate from the rest of the string. Here is a pair of functions I wrote to getLast and getSubString.

<cffunction name="getLast" output="no" returnType="string" access="public" hint="Takes a string and returns the last value by either comma or space, or returns nothing if the string is only one value long">
    <cfargument name="str" required="yes" type="string">

    <cfset var returnVar = "">

    <cfif listLen(arguments.str) GT 1>
        <cfoutput>listLen(#arguments.str#)</cfoutput>
        <cfset returnVar = trim(listLast(arguments.str))>
    <cfelseif listLen(arguments.str, " ") GT 1>
        <cfoutput>listLen(#arguments.str#, " ")</cfoutput>
        <cfset returnVar = trim(listLast(arguments.str, " "))>
    </cfif>

    <cfreturn returnVar>
</cffunction>

<cffunction name="getSubString" output="no" returnType="string" access="public" hint="Takes a string and returns the beginning of the string minus the last value">
    <cfargument name="str" required="yes" type="string">

    <cfset var last = getLast(arguments.str)>
    <cfset var lastLen = len(last) + 1>
    <cfset var thisLen = len(str) - lastLen>
    <cfset var returnVar = str>

    <cfif thisLen GTE 1>
        <cfset returnVar = trim(mid(str, 1, thisLen))>
    </cfif>

    <!--- remove trailing commas --->
    <cfif right(trim(returnVar), 1) IS ",">
        <cfset returnVar = mid(returnVar, 1, len(trim(returnVar)) - 1)>
    </cfif>

    <cfreturn returnVar>
</cffunction>

And just for fun, some code that was commented out of the second function:

<!---
<cfset var listDelim = ",">
<cfset var i = 0>
<cfset var returnVar = "">

<!--- if the string is longer than one value --->
<cfif len(last)>
    <cfif NOT findNoCase(arguments.str, ",")>
        <cfset listDelim = " ">
    </cfif>
    <cfloop from="1" to="#evaluate(listLen(arguments.str, listDelim) - 1)#" index="i">
        <cfset returnVar = "#returnVar# #listGetAt(arguments.str, i, listDelim)#">
    </cfloop>
    <cfset returnVar = trim(returnVar)>
<cfelse>
    <cfset returnVar = arguments.str>
</cfif>
--->

Next time I think I'm going to avoid parsing all my own text and just dumping it all into SOLR.

\$\endgroup\$
  • \$\begingroup\$ I didn't look here to see if there are UDFs exactly for this, but before I write UDFs, I always check here: cflib.org \$\endgroup\$ – user24876 May 8 '13 at 21:29
3
\$\begingroup\$

If you are using ColdFusion 9+, you should abandon your vars within functions and move toward using the LOCAL scope. LOCAL vars do not escape the function.

This:

<cfset var last = getLast(arguments.str)>
<cfset var lastLen = len(last) + 1>
<cfset var thisLen = len(str) - lastLen>
<cfset var returnVar = str>

Should be:

<cfset LOCAL.last = getLast(arguments.str)>
<cfset LOCAL.lastLen = len(LOCAL.last) + 1>
<cfset LOCAL.thisLen = len(LOCAL.str) - LOCAL.lastLen>
<cfset LOCAL.returnVar = str>
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Disagree (and it's subjective either way). Actively scoping local variables makes the code more verbose and clunky to read. But mileage varies. I'd not - however - advocate changing existing code from one to the other approach. \$\endgroup\$ – Adam Cameron May 4 '13 at 7:22
2
\$\begingroup\$

Observations:

  • getLast() is very vaguely named. It looks like you're trying to get the last "word" in a string, so unless getLast() is in a CFC called Sentence.cfc or WordList.cfc, I think its name could be improved, eg: getLastWord().
  • if you are after getting the last "word" when there's regex support for that, which makes for a much smaller function, and is also more robust in that it uses an accepted definition for what constitutes a "word":

(the editor is ignoring my formatting if I position the code here, for some reason. I've put it at the bottom...)

Or just use listLast() with multiple delimiters:

listLast(str, ", .");

But the regex solution is probably more robust.

I would use a variation of the regex approach above, or just listDeleteAt() for the second function. And, again, improve its name.

Code from above:

public string function getLastWord(required string str){
    var result = reReplace(str, ".*\W(.+)$", "\1", "ONE");
    result = reReplace(result, "\W", "", "ALL"); // any non-word characters 
    return result;
}
writeOutput(getLastWord("the quick brown fox jumps over the lazy dog."));
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

I know this is very old, but just wanted to give some thoughts on your getSubString() function:

  1. getLast() and lastLen use:

    <cfset var last = getLast(arguments.str)>
    <cfset var lastLen = len(last) + 1>
    <cfset var thisLen = len(str) - lastLen>
    <cfset var returnVar = str>
    
    • If str is just a single word and not a comma- or space-delimited list, your lastLen will be 1, and your thisLen will now be 1 less than the length of str's value.

      Subsequently, your returnVar will just return your str without its last character. This may be what you intended, or it may be not...

    • Be consistent in scoping your vars - use arguments.str everywhere.
    • I would trim(arguments.str) in the 3 lines above where it is used, and drop all the trim()s in subsequent lines.
  2. Instead of <cfif thisLen GTE 1>, which is same as GT 0 in this case, just test for boolean: <cfif thisLen>.

  3. <cfset returnVar = trim(mid(str, 1, thisLen))> - why not just use left() function instead: <cfset returnVar = left(returnVar, thisLen)>? + use returnVar instead of str and drop trim() if you used it in var setting code.

  4. This code block:

    <cfif right(trim(returnVar), 1) IS ",">
     <cfset returnVar = mid(returnVar, 1, len(trim(returnVar)) - 1)>
    </cfif>
    

    should be inside the <cfif> block above it - it is only relevant when thisLen > 0. And again, just use left() function instead of mid() (and drop all trim() calls if you changed the var setting lines to use it).

  5. To simplify things even more, your two <cfif> blocks can be compressed into these 3 lines:

    <cfif thisLen>
     <cfset returnVar = rereplace(left(returnVar, thisLen), "[\s,]+$", "")>
    </cfif>
    
| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

I believe ColdFusion already has similar function to your first: ListLast() - Gets the last item in a list http://livedocs.adobe.com/coldfusion/8/htmldocs/help.html?content=functions_l_14.html

I would suggest in getSubString that you trim the input when you assign it to 'last', then you can remove the trim()s used later on.

<cfset var last = trim(getLast(arguments.str))>
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ If you look at her getLast(); function you can see that she's making use of ListLast(); her function basically just performs ListLast( x ); and ListLast( x , " " ); \$\endgroup\$ – Busches May 15 '12 at 20:58
  • \$\begingroup\$ Yeah, my problem with "listLast" is that I'm not sure which delimiter might be required, depending on what the user types in. This is all part of a text parsing routine where a user could type in an address, including spaces and/or commas. \$\endgroup\$ – Sharondio Jun 19 '12 at 14:46
  • \$\begingroup\$ In fact, now that I think about it, I believe my original solution to this did a loop on characters starting at the right and stopping when it hit a space or a comma. \$\endgroup\$ – Sharondio Jun 19 '12 at 14:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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