Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a function that loops through large objects of raw data, translating coded strings into readable words. The function is very basic, and I keep wondering if I should use an object instead containing the words to be detected and translated. This would be cleaner-looking and more flexible, but it would not like it to harm the performance of the script, since it needs to run through potentially thousands of items.

function formatTitle(input) {
    var input_lowercase = input.toLowerCase() 
    if (input == "total") {
        return "Title"
    } else if ( input_lowercase == "type" || input_lowercase == "dokumenttype" ||  input_lowercase == "km_type"  ) {
        return label_documenttype
    } else if ( input_lowercase.indexOf("tema_") >= 0 || input_lowercase == "spesfikasjon" ) {
        return label_theme
    } else if ( input_lowercase.indexOf("prosess_") >= 0) {
        return label_process   
    } else if (input.indexOf("SM_") >= 0) {
        return input.substr(3);           
    } 
    else {
        return input.charAt(0).toUpperCase() + input.slice(1)
    }
}

There are more exceptions in the list, I've shortened it a bit down for practical reasons. I use the function in the following context (when t_formatheader is switched on):

    if ( skipping_mechanism == "lookup" ) { //42% faster than switch block in IE
        for (var key in arr_firstnode) {                
            if (key in keysToIgnore) continue;  
            if ( t_formatheader == "1" ) {
                arrayPushUnknown( columns, formatTitle( key ), "", true, key )
            }
            else {
                arrayPushUnknown( columns, key, "", true, key )     
            }
        }
    }
share|improve this question
    
Question, does the string in reality start with tema_ and prosess_ and SML_ or could it really be anywhere ? –  konijn Aug 21 '14 at 17:45
    
@konijn Good question! Yes, in this case the strings start with this, so I could do a check from the beginning of the string instead of the anywhere-searching "indexOf", I assume that this might increase performance a slight bit. –  Skarven Aug 22 '14 at 6:53

1 Answer 1

up vote 3 down vote accepted

I think you need a mix of lookup object(s) and regular if statements; there are clearly direct matches, matches on the prefix and the rest. Also, to write else if is pointless since you return anyway in previous if checks, so you can simply use

if(condition){
  return value
}
if(condition2){ // <- no else here
  return value2
}

All that together you could consider something like this:

var formatTitleDirectMapping = {
  'total' : 'title',
  'type' : label_documenttype,
  'dokumenttype' : label_documenttype,
  'km_type' : label_documenttype,
  'spesfikasjon' : label_theme,
}

var formatTitlePrefixMapping = {
  'tema_' : label_theme,
  'prosess_' : label_process,
  'dokumenttype' : label_documenttype,
  'km_type' : label_documenttype,
}    

function formatTitle(input) {
  var lower = input.toLowerCase();
  //Is there a straight forward conversion? Then get out
  if( formatTitleDirectMapping[lower] )
    return formatTitleDirectMapping[lower];
  //Is there a straight forward conversion for the prefix? Then get out
  var prefix = lower.split("_")[0];
  if( formatTitlePrefixMapping[prefix] )
    return formatTitlePrefixMapping[prefix];
  //Put the conversions that dont lend themselves to lookup structures here
  if( prefix == "sm") {
    return input.substr(3);           
  } 
  //Catch all
  return input.charAt(0).toUpperCase() + input.slice(1)
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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