HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Optimizing name highlighting

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
namehighlightingoptimizing

Problem

I have created a set of functions, that when given a struct of structs full of names, it will highlight those names that appear in a body of HTML based text. Is this the best method of doing this?

```





variables.stPartners = arguments.stPartners;
variables.sStopString = '';
variables.sLink = arguments.sLink;

return this;





local.sHighlightedText = arguments.sSearchText;
local.lstPunct = '';
local.lstPunct = listAppend(local.lstPunct, ",", ',');
local.lstPunct = listAppend(local.lstPunct, ".", ',');
local.lstPunct = listAppend(local.lstPunct, " ", ',');
local.lstPunct = listAppend(local.lstPunct, "'", ',');
local.lstPunct = listAppend(local.lstPunct, '"', ',');
local.lstPunct = listAppend(local.lstPunct, '’', ',');
local.lstPunct = listAppend(local.lstPunct, '', ',');
local.lstPunct = listAppend(local.lstPunct, '&', ',');
local.lstPunct = listAppend(local.lstPunct, '?', ',');
local.lstPunct = listAppend(local.lstPunct, '!', ',');
local.lstReplaced = "";

for(local.stPartner in variables.stPartners){
local.stArguments.stPartner = variables.stPartners[local.stPartner];
local.stArguments.sHighlightType = arguments.sHighlightType;
local.nNameLength = len(local.stArguments.stPartner.sFullName);

local.nExists = findnocase(local.stArguments.stPartner.sFullname, local.sHighlightedText);
local.aBytes = local.sHighlightedText.GetBytes();

if(local.nExists gt 0){
local.sSupposedName = mid(local.sHighlightedText, local.nExists, local.nNameLength);

local.nNextByte = (local.nExists-1)+(lo

Solution


  local.sHighlightedText              = arguments.sSearchText;
  local.lstPunct                      = '';
  local.lstPunct                      = listAppend(local.lstPunct, ",", ',');
  local.lstPunct                      = listAppend(local.lstPunct, ".", ',');
  local.lstPunct                      = listAppend(local.lstPunct, " ", ',');
  local.lstPunct                      = listAppend(local.lstPunct, "'", ',');
  local.lstPunct                      = listAppend(local.lstPunct, '"', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '’', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '&', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '?', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '!', ',');
  local.lstReplaced                   = "";

  for(local.stPartner in variables.stPartners){
        local.stArguments.stPartner   = variables.stPartners[local.stPartner];
        local.stArguments.sHighlightType          = arguments.sHighlightType;
        local.nNameLength             = len(local.stArguments.stPartner.sFullName);

        local.nExists                 = findnocase(local.stArguments.stPartner.sFullname, local.sHighlightedText);
        local.aBytes                  = local.sHighlightedText.GetBytes(); 

        if(local.nExists gt 0){
            local.sSupposedName       = mid(local.sHighlightedText, local.nExists, local.nNameLength);

            local.nNextByte           = (local.nExists-1)+(local.nNameLength+1);

            if(listFindNoCase(local.lstPunct, Chr(local.aBytes[local.nNextByte]), ',') gt 0){
                local.nStop           = findNoCase(variables.sStopString,local.sHighlightedText,local.nExists-22);
                if(listFindNoCase(local.lstReplaced, local.stArguments.stPartner.sFullName, "|") eq 0 AND local.nStop eq 0){
                    local.sHighlighted          = getHighlight(argumentCollection=local.stArguments);
                    local.sHighlightedText      = rereplacenocase(local.sHighlightedText, local.stArguments.stPartner.sFullname, local.sHighlighted, "all");
                    local.lstReplaced           = listAppend(local.lstReplaced, local.stArguments.stPartner.sFullName, "|");
                }
            }
         }
     }
     return local.sHighlightedText;
  


Don't use a List (which, by the way, is just a string, so I wonder whether having , in , delimited string will go well) for anything but printing output. There are just so many pitfalls that you can fall in that it's not funny anymore. In this case, use a String instead:

local.punctation = ",. '\"’<>&?!";


Then, when you need to check if the string contains any of these, instead of

if(listFindNoCase(local.lstPunct, Chr(local.aBytes[local.nNextByte]), ',') gt 0)


use java.lang.String.contains instead.

if(local.punctation.contains(Chr(local.aBytes[local.nNextByte])){


ColdFusion Strings are Java Strings. ColdFusion Lists are Java Strings. Get to know the underlying classes, because Coldfusion's functions come with many gotchas, and I personally do not view the language as safe anymore.

See this StackOverflow answer why you can't put comma's in comma delimited lists. Basically, your comma is interpreted as a delimiter.

Code Snippets

<cffunction name="highlightPartners" returntype="string" access="public" hint="this searches through the partner struct and highlights the partner">
<cfargument name="sSearchText" type="string" required="true" hint="the text to search">
<cfargument name="sHighlightType" type="string" required="false" default="link" hint="can add others if needed later">
<cfscript>
  local.sHighlightedText              = arguments.sSearchText;
  local.lstPunct                      = '';
  local.lstPunct                      = listAppend(local.lstPunct, ",", ',');
  local.lstPunct                      = listAppend(local.lstPunct, ".", ',');
  local.lstPunct                      = listAppend(local.lstPunct, " ", ',');
  local.lstPunct                      = listAppend(local.lstPunct, "'", ',');
  local.lstPunct                      = listAppend(local.lstPunct, '"', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '’', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '<', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '>', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '&', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '?', ',');
  local.lstPunct                      = listAppend(local.lstPunct, '!', ',');
  local.lstReplaced                   = "";

  for(local.stPartner in variables.stPartners){
        local.stArguments.stPartner   = variables.stPartners[local.stPartner];
        local.stArguments.sHighlightType          = arguments.sHighlightType;
        local.nNameLength             = len(local.stArguments.stPartner.sFullName);

        local.nExists                 = findnocase(local.stArguments.stPartner.sFullname, local.sHighlightedText);
        local.aBytes                  = local.sHighlightedText.GetBytes(); 

        if(local.nExists gt 0){
            local.sSupposedName       = mid(local.sHighlightedText, local.nExists, local.nNameLength);

            local.nNextByte           = (local.nExists-1)+(local.nNameLength+1);

            if(listFindNoCase(local.lstPunct, Chr(local.aBytes[local.nNextByte]), ',') gt 0){
                local.nStop           = findNoCase(variables.sStopString,local.sHighlightedText,local.nExists-22);
                if(listFindNoCase(local.lstReplaced, local.stArguments.stPartner.sFullName, "|") eq 0 AND local.nStop eq 0){
                    local.sHighlighted          = getHighlight(argumentCollection=local.stArguments);
                    local.sHighlightedText      = rereplacenocase(local.sHighlightedText, local.stArguments.stPartner.sFullname, local.sHighlighted, "all");
                    local.lstReplaced           = listAppend(local.lstReplaced, local.stArguments.stPartner.sFullName, "|");
                }
            }
         }
     }
     return local.sHighlightedText;
  </cfscript>
</cffunction>
local.punctation = ",. '\"’<>&?!";
if(listFindNoCase(local.lstPunct, Chr(local.aBytes[local.nNextByte]), ',') gt 0)
if(local.punctation.contains(Chr(local.aBytes[local.nNextByte])){

Context

StackExchange Code Review Q#68537, answer score: 2

Revisions (0)

No revisions yet.