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'm using an API which returns text in the following format:

#start
#p 09060 20131010
#p 09180 AK
#p 01001 19110212982
#end
#start
#p 09060 20131110
#p 09180 AB
#p 01001 12110212982
#end

I'm converting this to a list of objects:

var result = data.match(/#start[\s\S]+?#end/ig).map(function(v){

    var lines = v.split('\n'),
        ret = {};

    $.each(lines, function(_, v2){
        var split = v2.split(' ');
        if(split[1] && split[2]) 
            ret[split[1]] = split[2];
    });

    return ret;
});

My concern is that the API returns quite a lot of data, therefore I would like some feedback regarding on how to improve the performance.

For instance, is there any way to reduce the mapping complexity from O(N2) to O(N)?

Also, please suggest regex improvements :)

share|improve this question
add comment

2 Answers 2

up vote 3 down vote accepted

If you use regular expressions for parsing, then I would recommend using them for everything. Here's a solution that proceeds line by line, using capturing parentheses to see what the line contained.

function parse(data) {
    var re = /(#start)|(#end)|#p\s+(\S+)\s+(\S+)/ig;
    var results = [], match, obj;
    while (match = re.exec(data)) {
        if (match[1]) {           // #start
            obj = {};

        } else if (match[2]) {    // #end
            results.push(obj);
            obj = null;           // ← Prevent accidental reuse if input is malformed

        } else {                  // #p something something
            obj[match[3]] = match[4];
        }
    }
    return results;
}
share|improve this answer
    
Thanks, but I think that you've missed something. All I get is an array of empty objects. If I inspect each match, it'll look like either ["#start", "#start", undefined, undefined, undefined] or ["#end", undefined, "#end", undefined, undefined]. –  Johan Apr 4 at 12:10
    
Sorry, I miswrote var result = [] instead of var results = []. It doesn't make sense that you would get empty objects, though. Works for me. –  200_success Apr 4 at 12:26
    
Nope, you're right. I just made a simplified example above, my code looks slightly different. But for my example above, this works just fine, some my problem lies elsewhere. I'll look in to it, thanks a lot for your snippet! –  Johan Apr 4 at 12:32
1  
Changing the regular expression to /(#start)|(#end)|#p\s+(\S+)\s*(\S*)/ig would let the third field be optionally empty. –  200_success Apr 9 at 16:29
1  
Changing the regular expression to /(#start)|(#end)|#p\s+(\S+)\s*(.*)/ig would let the third field contain anything, including an empty string or a string containing spaces. –  200_success Apr 9 at 17:27
show 4 more comments

I dislike regexes with a passion ;) Especially because sometimes they beat solutions that ought to be faster.

I would counter propose a solution where you keep using indexOf while keeping track where you are in the data. This way you only go thru the data once. I would also name your constants 0 and 1 so that the reader instinctively knows what you are doing. Furthermore, given that your script is horizontally quite short, I would spell out your variables. I am not a big fan of v, v2 , _ etc. Finally, if speed is important, then good old loops will always beat forEach.

function parseResults( data )
{
  var index = -1,
      lastIndex = -1,
      objects = [],
      object,
      line,
      parts,
      KEY = 0,
      VALUE = 1;
  //~ is a short circuit for comparing to -1
  while( ~ (index = data.indexOf('\n',index) ) )
  {
    line = data.substring( lastIndex , index );
    if( line == '#start')
      object = {};
    else if( line == '#end' )
      objects.push( object );
    else 
    {
      parts = line.split(' ');
      if( parts[KEY] && parts[VALUE] )
        object[ parts[KEY] ] = parts[VALUE];
    }
    //+1 because I dont want to do ++ in the while, another +1 to make substring work
    //admittedly not very elegant looking :\
    lastIndex = index + 2;
  }
  return objects;
}

I would be most curious if you run this version and you run the 200_success version which one would be more performing with large sets of data.

share|improve this answer
    
Thank you for an alternative solution. Yes, regex is always my very last resort. I think this will be slower than @200's version due to the use of indexOf. Regarding index++ + 1; why not do index += 2? I only have dummy data to test with at the moment, but I'll try to remember to post some performance results here once I get some real data to play around with. –  Johan Apr 4 at 15:03
    
for ++ +1, because I am an idiot ;) Also, for indexOf because of the startPosition, I am not convinced yet that it will be slower –  konijn Apr 4 at 15:50
    
Hehe, well I'll try to remember to post the results ;) Thanks again –  Johan Apr 4 at 15:52
1  
Hi! Just FYI; I would say that the performance difference was neglectable (+- a few ms) –  Johan Apr 9 at 18:16
    
Thanks for following up –  konijn Apr 9 at 18:33
add comment

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.