Transforming complex strings into simple arrays

Question

Some time ago I wrote a series of functions for turning a string of user information from multiple users an array containing the information needed: Name, username and location. The raw userinfo string (userinfo_raw) looks like this:

1074;#Fring,, Gustavo (US - New Mexico),#i:0ǵ.t|adfs|gfring,#[email protected],#[email protected],#Fring,, Gustavo (US - New Mexico);#11903;#Rodarte-Quaylet,, Lydia (US - Houston),#i:0#.w|atrema\lrquaylet,#[email protected],#,#,, Rodarte-Quaylet,, Lydia (US - Houston) 

My first problem was that users are separated with "#;", but this separator is also used within each user, separating the user ID from the rest of the information. I do not need the user ID, so I went overboard and made an entire function (selfToArray) to separate the user ID from the rest of the data.

var userinfoArray = new Array(); var filterArray = new Array(); selfToArray( author_raw, filterArray ) //separate ID, keep the rest in the new array if (filterArray.length > 0) { for (i = 0; i < (filterArray.length); i++) { author_data = filterArray[i].name getAuthor(author_data, userinfoArray, tile_AuthorArray_total) } } function selfToArray(input, inputArray, splitter) { if ( input ) { var input_id = '' var input_name = '' var splitby = ';' if (typeof splitter != 'undefined') splitby = splitter if (input.indexOf( splitby ) >= 0) { var input_split_array = input.split( splitby ) var input_split_array_length = input_split_array.length if (input_split_array_length > 1) { var second_value = input_split_array[1] var second_value_first_char = second_value.charAt(0) var second_value_second_char = second_value.charAt(1) if (second_value_first_char == '#') { var number_check = isNaN(second_value_second_char) //it is a hash, and second char is not a number for (var i = 0; i < input_split_array_length; i++) { var this_id = '' var name = '' if (i % 2 == 0) { //check if even this_id = input_split_array[i] this_id = this_id.replace('#', '') input_id = this_id } else { //odd name = input_split_array[i] name = name.replace('#', '') input_name = name inputArray.push({ id: input_id, name: input_name }); } } } } } } } function getAuthor(input, inputArray, totalArray ) { var this_name = '' var this_login = '' var this_email = '' var this_username = '' var this_SIP = '' var this_login = '' var this_location = '' if (input.indexOf( ',,' ) >= 0) { input = input.replace(/,,/g, 'dummyvalue'); } var arr = input.split(',#') for(var i=0; i < arr.length; i++) { if (arr[i].indexOf("dummyvalue") >= 0) { arr[i] = arr[i].replace('dummyvalue',',') if (arr[i].indexOf(" (") >= 0) { this_name = arr[i].split(' (')[0] this_location = arr[i].split(' (')[1] } else { this_name = arr[i][0] } } else if (arr[i].indexOf("@") >= 0) { this_email = arr[i] this_SIP = arr[i] this_username = arr[i].split('@')[0].toLowerCase() } else if (arr[i].indexOf("adfs") >= 0) { this_login = arr[i] } } inputArray.push({ name: this_name, login: this_login, email: this_email, username: this_username, SIP: this_SIP, location: this_location }); totalArray.push( this_name ); } 

I've learned enough to know that this is very, very far from an elegant way of doing this, but I am not certain of the optimal approach. I assume that first selfToArray() function should be easy to replace with either a regex filter or a simple loop, depending on what runs fastest.

Answer 1

From a once over:

• Read this about the arrow pattern code, more specifically, your check on input should read like this:

  if( !input ) { return; //Or, you could even throw an error / exception ? } 

• This can be shortened with the OR operator:

  var splitby = ';' if (typeof splitter != 'undefined') splitby = splitter 

  to

  var splitby = splitter || ';' 

• input_split_array_length has the same length input_split_array.length and you use it only twice. Either give it a shorter name, or don't create that variable at all.

• You could replace all these

  var this_name = '' var this_login = '' var this_email = '' var this_username = '' var this_SIP = '' var this_login = '' var this_location = '' 

  with

  var author = {}; 

  and then simply use author.name instead of this_name. Similarly you can then replace your push statement with inputArray.push( author );

• Semicolons, use them
• var userinfoArray = new Array(); ->var userinfoArray = [] , which is more idiomatic
• this_login is declared twice
• You are declaring i in the global namespace here: for (i = 0; i < (filterArray.length); i++) { , use a tool like jshint.com to detect this and fix this
• You determine number_check but you never use it
• Naming
  • second_value_first_char ->secondValueFirstChar <- Use lowerCamelCase
  • As names go, secondValueFirstChar is kind of terrible, think about shorter more meaningfull names
• Commenting, you dont have a lot of it, one comment is hard to understand and they all deserve their own line I think

Overall, I think you should take the above into account and rewrite large portions. Then, I would resubmit your re-factored code, because I think I only scratched the surface.