Encoding strings using periodic table abbreviations

Question

I took a JavaScript challenge to finish a task related to logo of "Breaking Bad" where letters in your first name and last name are spotted with elements of periodic table and its respective atomic number. I wrote the below code, any suggestions to improve performance or any best coding practices

 function Process() {
 var ellist = {
 "h": "1",
 "he": "2",
 "li": "3",
 "be": "4",
 "b": "5",
 "c": "6",
 .
 .
 .
 "Lv":"116",
 "Uus":"117",
 "Uuo":"118"
 };
 var fname = document.getElementById("firstname");
 var lname = document.getElementById("lastname");
 var splits = fname.split("");
 var value;
 for (var i = 0; i < splits.length; i++) {
 var onevalue = fname.indexOf(splits[i]);
 var singlev = fname.substring(onevalue, onevalue + 1);
 var doublev = fname.substring(onevalue, onevalue + 2);
 var triplev = fname.substring(onevalue, onevalue + 3);
 if (ellist[splits[i]] || ellist[doublev] || ellist[triplev]) {
 value = splits[i];
 if (ellist[doublev] || ellist[triplev]) {
 value = ellist[doublev];
 if (ellist[triplev]) {
 value = ellist[triplev];
 // some code here
 }
 // some code here
 }
 // some code here
 }
 }

Using the Process() function which contains the logic. The object ellist contains the list of elements of periodic table with its atomic number. First name is taken from textbox on webpage and stored in fname and similarly the last name in lname and in the for loop it contains the code which checks whether the firstname contains the string which matches the elemetns of periodic table. Any suggestions?

• Virus721: for (var i = 0; i < splits.length; i++) -> for (var i = 0, maxI = splits.length; i < maxI; ++i)
• zch: Do you have any problems with current performance?
• Bergi: @Virus721: No, that's irrelevant premature optimization.

Answer 1

Any suggestions?

Yes, a few.

First off, split your function into parts (SRP), to separate the view (DOM elements and their values) from the logic (finding element names in strings).

var splits = fname.split("");
for (var i = 0; i < splits.length; i++) {
 var onevalue = fname.indexOf(splits[i]);

That doesn't make much sense to me. Don't you expect onevalue == i? If not, you might annotate this explicitly and/or make the comparison. Maybe it's inside the "some code"?

var doublev = fname.substring(onevalue, onevalue + 2);
var triplev = fname.substring(onevalue, onevalue + 3);

Notice that these will have the same value as singlev in the last [two] iterations of your loop, where the end is outside of the string.

if (ellist[splits[i]] || ellist[doublev] || ellist[triplev]) {
 value = splits[i];
 if (ellist[doublev] || ellist[triplev]) {
 value = ellist[doublev];
 if (ellist[triplev]) {
 value = ellist[triplev];

Ouch. Simplify this to

if (triplev in ellist) {
 value = ellist[triplev];
} else if (doublev in ellist) {
 value = ellist[doublev];
} else if (splits[i] in ellist) { // are you sure you don't want `singlev`?
 value = splits[i];
}

Answer 2

The function should take firstname and lastname arguments as strings. If you need to retrieve the strings from DOM elements, make a wrapper function. (You actually screwed this up in the code, treating DOM elements as if they were strings.)

This code is unnecessarily verbose:

var singlev = fname.substring(onevalue, onevalue + 1);
var doublev = fname.substring(onevalue, onevalue + 2);
var triplev = fname.substring(onevalue, onevalue + 3);
if (ellist[splits[i]] || ellist[doublev] || ellist[triplev]) {
 value = splits[i];
 if (ellist[doublev] || ellist[triplev]) {
 value = ellist[doublev];
 if (ellist[triplev]) {
 value = ellist[triplev];
 // some code here
 }
 // some code here
 }
 // some code here
}

You could just say

var singlev = fname.charAt(i);
var doublev = fname.substring(onevalue, onevalue + 2);
var triplev = fname.substring(onevalue, onevalue + 3);
value = ellist[triplev] || ellist[doublev] || ellist[singlev];