Reputation: 33
This is my first post to SO, a resource that is incredibly valuable!
I am trying to determine if a value (state code) exists in a list of codes and if so, set a new variable that represents the division (ie. sales territory, no match = 15). The code below works, but I want to improve my skills and do this as efficiently as possible. So my question is there a "better" way to achieve the same result?
<cfset state = "LA">
<cfset division = 15>
<cfset position = 0>
<cfset aStates = ArrayNew(1)>
<cfset aStates[1] = "MA,ME,NH,RI,VT">
<cfset aStates[2] = "CT,DE,NJ,NY,DE">
<cfset aStates[3] = "DC,MD,VA,WV">
<cfset aStates[4] = "TN">
<cfset aStates[5] = "NC,SC">
<cfset aStates[6] = "GA">
<cfset aStates[7] = "FL">
<cfset aStates[8] = "AL,KY,LA,MS">
<cfset aStates[9] = "IL,WI">
<cfset aStates[10] = "CO,MN,ND,SD,WY">
<cfset aStates[11] = "IN,OH,MI">
<cfset aStates[12] = "ID,OR,UT,WA">
<cfset aStates[13] = "AZ,HI,NV">
<cfset aStates[14] = "CA">
<cfset position = 0>
<cfloop array="#aStates#" index="lStates">
<cfset position = position+1>
<cfif ListFindNoCase(lStates,variables.state) NEQ 0>
<cfset division = position>
</cfif>
</cfloop>
<cfdump var="#aStates#" label="states array">
<cfoutput>State: #state#</cfoutput>
<cfoutput>Division: #division#</cfoutput>
Upvotes: 2
Views: 1584
Reputation: 33
Thank you all for your input. I am answering this question myself with the code I decided to use since it is based on piece supplied by multiple answers/comments. I hope this is the correct way to do this, if not please advise.
The divisions were not stored in the db as they are a set it and forget it mapping that has not changed in years and I thought I'd save a call to the DB. If they were subject to change I would have taken a table approach suggested by @leigh
Thanks to @matt-busche for the improved loop, ucase() and break recommendations, and @cfqueryparam for the short hand array suggestion.
Here's what I went with. The code is actually a function in a cfc that processes a form submission but I have pasted only the relevant part.
<cfset form.division = 15>
<cfset aDivisions = ["MA,ME,NH,RI,VT",
"CT,DE,NJ,NY,DE",
"DC,MD,VA,WV",
"TN",
"NC,SC",
"GA",
"FL",
"AL,KY,LA,MS",
"IL,WI",
"CO,MN,ND,SD,WY",
"IN,OH,MI",
"ID,OR,UT,WA",
"AZ,HI,NV",
"CA"]>
<cfloop from="1" to="#ArrayLen(aDivisions)#" index="i">
<cfif ListFind(aDivisions[i],Ucase(arguments.sForm.state)) NEQ 0>
<cfset form.division = i>
<cfbreak>
</cfif>
</cfloop>
Upvotes: 0
Reputation: 28873
With the current structure, you are relying on string comparisons, so there is not too much room for improvement.
Unless there is a specific reason you must hard code the values, a more flexible approach is to store the information the database. Create tables to store the "states" and "divisions", and another table to store the relationships:
States
StateID | Code | Name
1 | ME | Maine
2 | GA | Georgia
3 | CA | California
4 | NH | New Hampshire
...
Divisions
DivisionID | Name
1 | Sales Territory
...
DivisionStates
DivisionID | StateID
1 | 1
1 | 4
6 | 2
14 | 3
...
Then use an OUTER JOIN to retrieve the selected state and division. Assuming #state#
will always contain a valid entry, something along these lines (not tested):
SELECT s.Name AS StateName
, CASE WHEN d.Name IS NULL THEN 'No Match' ELSE d.Name END AS DivisionName
FROM States s
LEFT JOIN DivisionStates ds ON ds.StateID = s.StateID
LEFT JOIN Divisions d ON d.DivisionID = ds.DivisionID
WHERE s.Code = <cfqueryparam value="#state#" cfsqltype="cf_sql_varchar">
I do not know the source of the #state#
variable, but I am guessing it may be passed via a form <select>
. If it is currently hard coded, you could simplify things further by using querying the "states" table, and using it to populate the list. Also, consider using the ID as the list value, rather than the state "code".
Upvotes: 4
Reputation: 5510
ListContains()
is like ListFind()
, but it searches for a list element that contains the the text you're looking for anywhere in it. (ListFind()
looks for a list element that entirely matches the string. Generally ListFind()
is the better tool but not in this case.
(CF does have an ArrayContains, but it doesn't work for this task, it merely tells you if the array contains an exactly matched element.)
Because you're searching for unique two-char state-codes, this is a perfect use of the function.
<cfset aStates = ArrayNew(1)>
<cfset aStates[1] = "MA,ME,NH,RI,VT">
<cfset aStates[2] = "CT,DE,NJ,NY,DE">
<cfset aStates[3] = "DC,MD,VA,WV">
<cfset aStates[4] = "TN">
<cfset aStates[5] = "NC,SC">
<cfset aStates[6] = "GA">
<cfset aStates[7] = "FL">
<cfset aStates[8] = "AL,KY,LA,MS">
<cfset aStates[9] = "IL,WI">
<cfset aStates[10] = "CO,MN,ND,SD,WY">
<cfset aStates[11] = "IN,OH,MI">
<cfset aStates[12] = "ID,OR,UT,WA">
<cfset aStates[13] = "AZ,HI,NV">
<cfset aStates[14] = "CA">
<cfset lstates = arraytolist(astates,"|")>
<cfset division = listcontainsnocase(lstates,"CO","|")>
<cfoutput>Division: #division#<br>
State: #state#</cfoutput>
We set the delimiter to | (pipe), but you can use anything, other than the default (comma), because you're using that for you're sub-delimiter.
For future reference, if you're using CF 9 (I believe) or after, you can use array shorthand for quickly building arrays.
<cfset aStates=["MA,ME,NH,RI,VT","CT,DE,NJ,NV,DE","DC,MD,VA,WV"]> ...
While it may seem merely like a stylistic difference, it saves a lot of time.
Structures can be created similarly.
<cfset MyDogIs = {Size = "medium", Color = "Black", HasPaws = ["FL","FR","BL","BR"]}>
(And you can nest implicit array and structs within another!)
Upvotes: 1
Reputation: 1866
I thought of an approach in this by placing the states into a Struct; with the state codes as keys.
<cfscript>
state = "LA";
division = 15;
states_divisions_map = {
MA: 1, ME: 1, NH: 1, RI: 1, VT: 1,
CT: 2, NJ: 2, NY: 2, DE: 2,
DC: 3, MD: 3, VA: 3, WV: 3,
TN: 4,
NC: 5, SC: 5,
GA: 6,
FL: 7,
AL: 8, KY: 8, LA: 8, MS: 8,
IL: 9, WI: 9,
CO: 10, MN: 10, ND: 10, SD: 10, WY: 10,
IN: 11, OH: 11, MI: 11,
ID: 12, OR: 12, UT: 12, WA: 12,
AZ: 13, HI: 13, NV: 13,
CA: 14
};
if(StructKeyExists (states_divisions_map, state) ){
division = states_divisions_map[state];
}
writeDump(var: states_divisions_map, label: "States");
writeOutput("State: #state#");
writeOutput("Division: #division#");
</cfscript>
This way you can quickly check for the state without all the loops.
Observation: In your original code, DE
is present twice.
Upvotes: 1