Generating DOM from a complex strings via arrays

By Posted on

Problem

This is a revised and expanded version of Transforming complex strings into simple arrays

My function grabPerson turns strings of user information from multiple users to “business cards” on my webpage, via 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,#gfring@LPH.com,#gfring@lph.com,#Fring,, Gustavo (US - New Mexico);#11903;#Rodarte-Quaylet,, Lydia (US - Houston),#i:0#.w|atremalrquaylet,#lrquaylet@madrigal.com,#,#,, Rodarte-Quaylet,, Lydia (US - Houston)

The 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 am using the function (selfToArray) to separate the user ID from the rest of the data.

function selfToArray(input, inputArray, splitter) { 
    var splitby = splitter || ';'               
    if (input.indexOf( splitby ) === -1 ) {
    }
    else {
        var input_split_array = input.split( splitby )
        if (input_split_array.length < 1 ) {
        }
        else if ( input_split_array[1].charAt(0) == '#') {
            for (var i = 0; i < input_split_array.length ; i++) {
                if (i % 2 !== 0) { //check if odd (even is the ID, no need for this now.)
                    inputArray.push( input_split_array[i].replace('#', '') )
                }
            }
        }
    }
}       

function getAuthor(input, inputArray ) { 
    var author = {};
    if (input.indexOf( ',,' ) >= 0) input = input.replace(/,,/g, 'dummyvalue'); //used for identifying the name later on
    var arr = input.split(',#')
    for(var i=0; i < arr.length; i++) {
        if (arr[i].indexOf("dummyvalue") >= 0) { //find section containing the name
            arr[i] = arr[i].replace('dummyvalue',',')                       
            if (arr[i].indexOf(" (") >= 0) { //if author name contains location, such as: Fring,, Gustavo (US - New Mexico)
                author.name = arr[i].split(' (')[0]
                author.location = arr[i].split(' (')[1]
            }
            else {
                author.name = arr[i][0]                                 
            }
        }
        else if (arr[i].indexOf("@") >= 0) { //find section with email
            author.email = arr[i]
            author.sip = arr[i]
            author.username = arr[i].split('@')[0].toLowerCase()
        }
        else if (arr[i].indexOf("adfs") >= 0) {
            author.login = arr[i]
        }
    }
    inputArray.push( author );          
}   

function grabPerson( author_raw ) { 
    var result_container = '' 
    var filterArray = []; //for storing the data after the ID separation
    var userinfoArray = []; //for collecting the results of processing this data
    selfToArray( author_raw, filterArray ) //separate ID, keep the rest in the new array
    if (filterArray.length) {
        for (var i = 0; i < (filterArray.length); i++) {
            getAuthor( filterArray[i], userinfoArray ) //transform userinfo into array
        }
    }
    if (userinfoArray.length) {
        result_container += '<ul class="persons_container">'
        for (var i = 0; i < (userinfoArray.length); i++) {
            var profilesizing = 0 //could utilize 'i' for gradual size increase/decrease
            var author_location = userinfoArray[i].location || ""
            if (author_location) author_location = " (" + author_location
            var author_name = userinfoArray[i].name + author_location
            if (author_name) {
                var result_person = userTile( userinfoArray[i].email, author_name, "inline", profilesizing, author_location)
                result_container += result_person   
            }
        }
        result_container += '<div style="display: block; clear: both;"></div>'
        result_container += '</ul>'
        return result_container //the results are collected and only later added to the DOM, in a single operation.
    }
}

Thanks to the feedback given to me by @konijn, I’ve improved the code somewhat, but I suspect that there are still optimizations that could be done in order to make this as fast as possible, since the hundreds (and potentially thousands) of rows are processed.

Solution

General notes

  • Consistency, please. JavaScript is typically camelCase (and PascalCase for constructors). You do use that, but you also use underscored_names here and there. Please pick one style – preferably the JS standard one – and stick to it.

  • You’re missing a lot of semicolons. I already mentioned it in a previous review, but I’d really just add those semis even if the code can run without them. Again, the automatic semicolon insertion is more of a crutch for lazy code than syntactic sugar.

About selfToArray()

  • The name is sort of strange. What’s self? stringToArray would be more straight-forward although it’s pretty generic and doesn’t describe what kind of string it is. Still, better than selfToArray.

  • Your if..elsees are kinda funny, since you’re not actually doing anything in the if blocks. Just flip the comparison around, and there’s no need for an else block:

     if (input.indexOf( splitby ) !== -1 ) { <your code> }

  • Or, keep the current comparison, and just return right away. Again, no need for an else block

     if (input.indexOf( splitby ) === -1 ) { return; }
 <your code>

  • The typical way to set a default value for an argument is to reuse the argument’s name, rather than make a new variable. It helps readability. So:

     splitter = splitter || ';';
 // or, alternative syntax
 splitter || (splitter = ';');

    By the way, I’d probably call it delimiter or separator instead (more conventional names).

  • Then again, I doubt you’ll ever need to pass a custom delimiter. The rest of the code is tailored to some very specific data. It looks for # characters, and it discards every other part of the split string. It seems to me that you won’t need a function that does all those very specific things – but with something other than a semicolon as the delimiter. (And indeed you don’t use the function with a custom delimiter argument.)

  • Overall, I’m wondering why you need to pass an array to this function. It can just return one, all on its own. This isn’t C where you need to pass pointers to pointers and stuff like that. To have any function modify its input is pretty iffy unless there’s a really, really good reason. And I can’t think of any since you can just handle the array it returns however you want (e.g. concat it to another array or something). String in, array out, period.

About getAuthor()

You’re using some fragile magic here, it seems. You assume the name part will always contain ",," (I’d assume that that’s there in case of a middle name), and that the login is always "adfs" (huh?). Oh, and that the word “dummyvalue” doesn’t by chance appear anywhere else. But it might, who knows? Sure, it’s very unlikely, but the code is still kludgy.

If there’s any sort of logic to the data you’re dealing with, the “rows” will always contain the same sort of information in the same order. In this case the order appears to be:

  1. Name and location
    1. Last name
    2. Middle name (presumably)
    3. First name
    4. Location in parantheses
  2. Login (I assume)
  3. Email
  4. Secondary email? Not sure, and either way it appears optional
  5. Name again, apparently.

If indeed that’s the pattern (and I’ll assume here that it is), you could just convert an input string to an array of “author objects” wholesale, and skip the selfToArray. A tiny internal function can handle the name/location parsing.

function getAuthors(input) { // note: plural, since it returns an array
  // internal function to parse the name/location
  function getNameAndLocation(string) {
    var parts = string.match(/([^,]*),s*([^,]*),s*(.*?)s+(([^)]*)/);
    if(!parts) {
      return null; // oops, couldn't match name+location
    }
    return {
      name: {
        first: parts[3],
        middle: parts[2],
        last: parts[1]
      },
      location: parts[4]
    };
  }

  // this replaces selfToArray()
  var rows = input.split(/;#?/).filter(function (_, i) { return i % 2; });

  return rows.map(function (row) {
    var author = {},
        parts = row.split(/,#/),
        nameAndLocation = getNameAndLocation(parts[0]);

    author.name = nameAndLocation.name;
    author.location = nameAndLocation.location;
    author.login = parts[1];
    author.email = author.sip = parts[2]; // p.s. what is "sip"? Not clear.
    author.username = parts[2].split('@')[0];

    return author;
  });
}

For your example, it returns:

[
  {
    name: { first: 'Gustavo', middle: '', last: 'Fring' },
    location: 'US - New Mexico',
    login: 'i:0ǵ.t|adfs|gfring',
    sip: 'gfring@LPH.com',
    email: 'gfring@LPH.com',
    username: 'gfring'
  },
  {
    name: { first: 'Lydia', middle: '', last: 'Rodarte-Quaylet' },
    location: 'US - Houston',
    login: 'i:0#.w|atremalrquaylet',
    sip: 'lrquaylet@madrigal.com',
    email: 'lrquaylet@madrigal.com',
    username: 'lrquaylet'
  }
]

(p.s. there’s a backslash in Lydia’s login-info, which I think is supposed to be a pipe character |, right?)

As mentioned, the sip property isn’t really explained, and since it’s set to just be the email address, I don’t see the need for it. It’s just duplication.

But anyway, now you’ve got structured data to work with.

About grabPerson()

  • Strange name. It sounds like a whacky synonym for getAuthor. But it isn’t. It generates HTML, so perhaps its name should reflect that?

  • Doing HTML strings is easy – but also error-prone. If the text you’re concatenating contains characters like < or >, you’re creating invalid HTML. It can get weird in a hurry (besides, all those strings aren’t terribly readable). Better to create actual DOM objects, since that’s what the DOM’s for: An object-oriented interface for HTML. The elements don’t need to be inserted anywhere; you can just create them in memory for later insertion.

  • You’re inserting a div into a ul element, which is invalid HTML. I know its purpose is to clear floats, but at least use a special li for that. Or, better yet, use the CSS “clearfix” trick.

I won’t do a full rewrite, but something like this should get you started:

function buildAuthorList(authors) {
  var list = document.createElement("ul");
  list.className = "persons_container";

  for(var i = 0, l = authors.length ; i < l ; i++) {
    var person = document.createElement("li");
    person.textContent = authors[i].name.first + " " + authors.name.last;
    list.appendChild(person);
  }

  return list; // return an in-memory DOM element, ready for insertion
}

However, I’d highly recommend using a library for cross-browser compatibility when it comes to (large-scale, repeated) DOM manipulation. These days, that usually means “just use jQuery”. There are a lot of spiky pitfalls when it comes to the DOM and how different browsers implement it (textContent, for instance, didn’t appear in IE until IE9). In other words: There’s a reason jQuery is as huge as it is.

Leave a Reply

Your email address will not be published. Required fields are marked *