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've put together this simple fetcher for text data (which I just copy and pasted from a flight info website) - it takes in text data, and spits out an array of objects containing values for each property of each flight it finds. I thought it would be a good accompaniment exercise to "Eloquent Javascript: Chapter 3, Data Structures: Objects and Arrays."

  • I'm looking for ways in which I should condense the functions, or possibly merge their tasks. I don't think there's a whole lot that can be shared amongst each fetch function, however.
  • I think there may be a more simple way of noting the index of each item (found from the specific fetcher function), and then passing that on to the next fetcher function, but I'm not sure if that applies for each property of the flight, and I'm not sure what pattern I would use for that.
  • General code critique or advice?

*Note: I held off on adding fetchers for timeSched, status, and onSched until I get some feedback here, thanks! *

var flightObject = {

    /*
    We assume our text value comes in this form: airlineCode airlineName destAbbrev dest timeSched status onSched
    */

    text: "9E 3801 Pinnacle Airlines (MSP) Minneapolis 3:38 PM Landed On-time\nDL 3801 Delta Air Lines (MSP) Minneapolis 3:38 PM Landed On-time\n1I 131 Netjets Aviation (MEM) Memphis 3:06 PM Scheduled On-time\n1I 880 Netjets Aviation (HPN) Westchester County 3:06 PM En Route On-time\nRAX 308 Royal Air Freight, Inc. (FDY) Findlay 3:06 PM Landed On-time\nWN 627 Southwest Airlines (FLL) Fort Lauderdale 3:16 PM En Route On-time\nWN 2541 Southwest Airlines (SAT) San Antonio 3:35 PM En Route Delayed\nWN 1939 Southwest Airlines (LAS) Las Vegas 3:35 PM En Route On-time\nFIV 540 Citationshares (PWK) Chicago 3:10 PM Scheduled On-time",

    /*
    Here is our function that grabs the Flight Number (airlineCode), Airline (airlineName),
    Destination Abbreviation (destAbbrev), and Destination Long Title (dest), 
    Scheduled Time (timeSched), Status of Flight (stat), and whether flight is on time (onSched)
    */

    extractFlight: function() {

        /*
        Split paragraphs into lines
        */

        var paragraphs = this.text.split("\n");

        /*
        Get index for parenthesis which will help in finding destAbbrev
        note: start at i = 3 because we know it can't occur earlier
        due to data's nature.
        */


        function getParenthIndex(){

            for( var i = 3 ; i < words.length ; i++ ) {
                var word = words[i];
                if (word.charAt(0) === "(") {
                    return i;
                }
            }

        }

        function getAirlineName(){
            var parenthIndex = getParenthIndex();
            var airlineName = "";

            /*
            note: we start for loop at i = 2 because we know it can't 
            occur earlier due to data's nature.
            */

            for( var i = 2; i < parenthIndex; i++ ) {
                var word = words[i];
                if (i === parenthIndex - 1) {
                    return airlineName += word;
                }
                else {
                    airlineName += word + " ";
                }
            }
        }

        /*
        Grab destination abbreviation using index of 
        word that starts with parenthesis as guide
        */

        function getDestAbbrev(){
            var parenthIndex = getParenthIndex();
            return words[parenthIndex];
        }

        /*
        Grab destination using index of 
        word that starts with parenthesis as guide,
        while searching for number to know when to stop.
        */

        function getDest(){
            var parenthIndex = getParenthIndex();
            var dest = "";

            for( var i = parenthIndex + 1 ; i < words.length ; i++ ) {
                var word = words[i];
                var re = /\d/;

                if (!re.test(word)){
                    if (i === parenthIndex + 1) {
                        dest+= word;
                    }
                    else {
                        dest += " " + word;
                    }
                }
                else {
                    return dest;
                }
            }
        }

        /*
        Take array and add flight objects by looping through
        each paragraph and grabbing each property value we're interested in.
        */

        var flights = [];

        for( var i = 0 ; i < paragraphs.length ; i++ ) {

            var paragraph = paragraphs[i];

            var words = paragraph.split(" ");

            /* Now we find the flight number which is the 1st and 2nd word */

            var flightCode = words[0] + " " + words[1];
            var flightCodeConden = words[0] + words[1];

            var airlineName = getAirlineName();

            var destAbbrev = getDestAbbrev();

            var dest = getDest();

            flights[flightCodeConden] = {
                "flightCode" : flightCode,
                "airlineName" : airlineName,
                "destAbbrev" : destAbbrev,
                "dest" : dest
            };

        }

        console.log(flights);
    }
};
share|improve this question
    
maybe you're doing it like this for a particular reason? You could simplify a lot using slice and join methods on words. –  Stuart Dec 22 '12 at 2:19

1 Answer 1

up vote 0 down vote accepted

The individual flights here are good candidates for being individual objects, along the following lines. I take a slightly different approach of first identifying the index of the flight code and the index of the date, and then using that to parse all the other information using words.slice. That simplifies things so much you don't have to worry about passing indices around between different functions (though if you did need to do that as your parser gets more complicated, you could do so by making the relevant indices properties of the Flight object).

function Flight(text) {
    var ch, i, words = text.split(' '), parenthIndex, dateIndex;
    for (i = 3; i < words.length; i++) {
        ch = words[i].charAt(0);
        if (ch === "(") {
            parenthIndex = i;
        }
        else if (ch >= '0' && ch <= '9') {
            dateIndex = i;
            break;
        }
    }
    this.airlineName = words.slice(2, parenthIndex).join(' ');
    this.destAbbrev = words[parenthIndex];
    this.dest = words.slice(parenthIndex + 1, dateIndex).join(' ');
    this.code = words[0] + ' ' + words[1];
    this.id = words[0] + words[1];
}

Then in your extractFlights function you only need the following, to break the text down into individual lines and send them to new Flight objects to be parsed.

extractFlights: function() {
    var paragraphs = this.text.split("\n"), flights = {}, f, p;
    while (p = paragraphs.pop()) {
        f = new Flight(p);
        flights[f.id] = f;
    }
    console.log(flights);
}

(fiddle)

share|improve this answer
    
Can you explain your while loop in extractFlights? Did you mean to check for equality instead of assigning p to paragraphs.pop()? –  mcabrams Dec 27 '12 at 0:03
    
I just ended up using a for loop: for (var i = 0; i < paragraphs.length; i++) { p = paragraphs.pop(); f = new Flight(p); flights.push(f); } –  mcabrams Dec 27 '12 at 0:05
    
Your for loop is a more transparent way of doing the same thing. The while statement assigns the last value in paragraphs to p and removes it from paragraphs; when paragraphs is empty it returns undefined and so the loop stops. It's just a nice way of looping though all the values of an array when you don't care if the array gets destroyed in the process. –  Stuart Dec 27 '12 at 0:14

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.