I have a test that I've already turned in but would like some outside review on where I could improve. Spent most of the day today trying my best to keep this O(n) complexity. Any help is very much appreciated.

If it's a bit difficult to read here feel free to check out the repository

Example of how it should be formatted

{

    travelers: [

      {

        id: 1,

        name: 'Neo',

        flights: [

          {

            legs: [

              {

                airlineCode: 'AA',

                airlineName: 'American',

                flightNumber: 'AA456',

                frequentFlyerNumber: ''

              }

            ]

          },

          {

            legs: [

              {

                airlineCode: 'VA',

                airlineName: 'Virgin',

                flightNumber: 'VA789',

                frequentFlyerNumber: 'NVA123'

              },

              {

                airlineCode: 'AK',

                airlineName: 'Alaskan',

                flightNumber: 'AK789',

                frequentFlyerNumber: 'NAK123'

              }

            ]

          }

        ]

      }

    ]

  }

index.js where the magic happens

'use strict';



const profilesAPI = require('./api/profiles.service')

const tripsAPI = require('./api/trip.service')

const airlinesAPI = require('./api/airlines.service')



const setTraveler = (profile, iter) => ({

    id: profile[ iter ].personId,

    name: profile[ iter ].name,

    flights: 

})



const setAirlineHash = airlines => {

  const hash = {}

  airlines.forEach(airline => {

    hash[ airline.code ] = airline.name

  })



  return hash

}



const setLegs = ({ airlineCode, flightNumber, airlineName, air }) => ({

  airlineCode,

  flightNumber,

  airlineName,

  frequentFlyerNumber: air[ airlineCode ] ? air[ airlineCode ] : ''

})



async function getTravelersFlightInfo() {

  try {

    const { profiles } = await profilesAPI.get()

    const { trip: { flights } } = await tripsAPI.get()

    const { airlines } = await airlinesAPI.get()

    const travelers = { travelers:  }



    const airlinesHash = setAirlineHash(airlines)



    let currentTraveler = {}

    let legs = 

    let iP = 0

    let iF = 0

    let iTID = 0 

    let iL = 0

    while (iP < profiles.length) {

      if (!currentTraveler.id) {

        currentTraveler = setTraveler(profiles, iP)

      }



      if (flights[ iF ].travelerIds[ iTID ] === currentTraveler.id) {

        const { airlineCode, flightNumber, } = flights[ iF ].legs[ iL ]

        const { air } = profiles[ iP ].rewardPrograms

        const airlineName = airlinesHash[ airlineCode ]

        const data = setLegs({ airlineCode, flightNumber, airlineName, air })



        legs.push(data)



        // increase iF and reset IL if at the end of legs

        if (++iL >= flights[ iF ].legs.length) {

          currentTraveler.flights.push({ legs })

          legs = 



          // move to the next profile when we hit the end of flights

          if (++iF >= flights.length) {

            travelers.travelers.push(currentTraveler)

            currentTraveler = {}

            iF = 0

            iP++

            iTID++

          }



          iL = 0

        }

      }

      else {

        if (++iF >= flights.length) {

          travelers.travelers.push(currentTraveler)

          currentTraveler = {}

          iF = 0

          iP++

        }

      }

    }



    console.log(JSON.stringify(travelers, null, 2))

  }

  catch (err) {

    console.log(err)

  }

}



console.log(getTravelersFlightInfo())



module.exports = getTravelersFlightInfo;

profiles.js

const get = () => {

  return Promise.resolve({

    profiles: [

      {

        personId: 1,

        name: 'Neo',

        rewardPrograms: {

          air: {

            'AK': 'NAK123',

            'VA': 'NVA123'

          }

        }

      },

      {

        personId: 2,

        name: 'Morpheus',

        rewardPrograms: {

          air: {

            'AA': 'MAA123'

          }

        }

      },

      {

        personId: 3,

        name: 'Trinity',

        rewardPrograms: {

          air: {

            'VA': 'TVA123'

          }

        }

      },

      {

        personId: 4,

        name: 'Mr. Anderson',

        rewardPrograms: {

          air: {

            'AA': 'AAA444',

            'AK': 'AAK444',

            'VA': 'AVA444'



          }

        }

      }

    ]

  });

}



module.exports = {

  get

};

trips.js

const get = () => {

  return Promise.resolve({

    trip: {

      flights: [

        {

          travelerIds: [1, 2, 3],

          legs: [

            {

              airlineCode: 'AA',

              flightNumber: 'AA456'

            }

          ]

        },

        {

          travelerIds: [1, 2],

          legs: [

            {

              airlineCode: 'VA',

              flightNumber: 'VA789'

            },

            {

              airlineCode: 'AK',

              flightNumber: 'AK789'

            }

          ]

        }

      ]

    }

  })

}



module.exports = {

  get

};

airlines.js

const get = () => {

  return Promise.resolve({

    airlines: [

      { code: 'AK', name: 'Alaskan' },

      { code: 'AA', name: 'American' },

      { code: 'BA', name: 'British' },

      { code: 'DT', name: 'Delta' },

      { code: 'UA', name: 'United' },

      { code: 'VA', name: 'Virgin' },

    ]

  })

}



module.exports = {

  get

};

Whaa??? Unreadable and bloated.

Harsh title, but don't take it to heart. It takes a long time to learn to program effectively and the learning curve is steep. Your code shows all the hallmarks of inexperience, but it also shows you have what it takes to become a good coder.

The review

At my first look at this code, I had a hard time working out what you were doing. The main reason was that you are using a very unusual style of iteration.

You are using a single while loop to iterate over four independent arrays profiles, flights, travelerIds and legs.

Using very badly named counter variables, whose meanings only become apparent after I mentally stepped through the code (their purpose should be immediately apparent).

To manage each phase of the iteration you use 5 if statements to check data, increment counters, and build the required information object. The result is lots of duplication, AKA bloat.

You need to improve your naming.

Naming problems

• 
  setTraveler You are not setting an existing state, rather you create a new object. Maybe createTraveler, or even traveler
  


• 
  setLegs. Same as above.


• 
  setAirlineHash Again you create, and you use the singular Hash, but you create a hash map, or commonly just map. Maybe mapArilines
  



• getTravelersFlightInfo You are creating, specifically merging existing data, the context implies travelers, flights, and info, so maybe something like mergeTravelers, or mergeFlightInfo;




• iP,iF,iTID,iL Once I stepped over the code I understood them, i for i index, and P,F,L, for profile, flight and leg. With TID for travelers Id (should have been iTId) Aliased or abbreviated variable are not bad (they can save a lot of characters that make code line long and hard to read), but they have to be immediately clear (I should not have to read the whole source, and data structure) in the code, via comments or assignment from named alternatives. It turns out you don't need them so I will not give a suggested alternatives.

Code Structure

Maintain clear function roles

Rather than make the function getTravelersFlightInfo, async, I would move the loading out of the function and pass the required data as arguments. This moves the role of acquiring data out as a separate role

The role includes what is returned, and the function returns nothing, just outputing to the log. It should return the expected travelers object. If you need to output as JSON to the log do that outside the function.

Reduce noise and bloat

The functions setTraveler, and setLegs are on the most part just bloat, forcing you to pass named arguments to the functions that can be done inline with less code.

The function mapAirlines (you called setAirlineHash) can be done using array.reduce and would be better done outside the main function.

Excessive use of destructuring. The destructuring syntax was introduce to reduce code complexity, and you should only use it when the resulting code is simpler. eg you have const { profiles } = await profilesAPI.get() the destructuring is redundant and const profiles = await profilesAPI.get(); is better. BTW no spaces after { and before } in object literals and destructure assignments . { profiles } as {profiles}

Learn the language. Much of the bloat comes from poor use, or missing use of many Javascript's built in language features.

Only known errors should be caught

The try catch seems to have no actual use (your code does not create any errors). What you have done is silence errors by diverting them to the log. (BTW if you must log errors use console.error(error)). ONLY use try catch if you are generating known errors, or you expect known error types to be generated while the function does its thing. If you do catch errors, only handle the errors you expect and rethrow errors you don't expect so that higher level error management can take care of them.

Example

The following code is meant as an example of how to improve the code using more language features and a more idiomatic approach.

It is however not the only solution and there is room to reduce the complexity by doing a little data processing before calling mergeTravelers

You could reverse the construction process, stepping over the flights first and access profiles via a map using personId (constructed before calling mergeTravelers), which would be the least complex solution

Anyways the example has a minor mod to get at the data, not using async and promises, (see comments) but I did include the async code.

There was limited information so I went with the example solution you gave to verify it works. Profiles that have no matching trips will have empty flights array (Not sure if this is correct)

'use strict';

function showMergedTravelinfo(travelers) {

    console.log(JSON.stringify(travelers, null, 2));

}

async function acquireTravelFlightData() { 

    const mapAirlines = (map, airlines) => (map[airlines.code] = airlines.name, map);

    return { 

        profiles: await profilesAPI.get(),

        flightList: await tripsAPI.get().trip.flights,  

        airlineMap: await airlinesAPI.get().airlines.reduce(mapAirlines, {}),

    };

}

function mergeTravelers({profiles, flightList, airlineMap}) {    

    const createReward = (program, code) => program[code] ? program[code] : "";

    return {

        travelers: profiles.map(profile => {   

            const createLeg = leg => ({ ...leg,

                airlineName: airlineMap[leg.airlineCode],

                frequentFlyerNumber: createReward(

                    profile.rewardPrograms.air, 

                    leg.airlineCode

                )

            });

            const flights = ;

            for (const flight of flightList) {

                if (flight.travelerIds.includes(profile.personId)) {

                    flights.push({legs: flight.legs.map(createLeg)}); 

                }

            }

            return {id: profile.personId, name: profile.name, flights};

        })

    };

}



/* How to use if data was via API's 

showMergedTravelinfo( mergeTravelers( aquierTravelFlightData())); 

*/



























setTimeout(() => {  // Just for the snippet this ensures code is parse before run

      // Using the local data src

      const travelers = mergeTravelers(localSrcTravelFlightData(data));

      showMergedTravelinfo(travelers);



    }, 0

);



function localSrcTravelFlightData(data) { 

    const mapAirlines = (map, airlines) => (map[airlines.code] = airlines.name, map);

    return { 

        profiles: data.profiles,

        flightList: data.trip.flights,  

        airlineMap: data.airlines.reduce(mapAirlines,{}),

    };

}



/* Change the data source for the snippet and because I am lazy */

const data = {

    profiles: [{

            personId: 1,

            name: 'Neo',

            rewardPrograms: {air: {'AK': 'NAK123','VA': 'NVA123'}}

        },{

            personId: 2,

            name: 'Morpheus',

            rewardPrograms: {air: {'AA': 'MAA123'}}

        },{

            personId: 3,

            name: 'Trinity',

            rewardPrograms: {air: {'VA': 'TVA123'}}

        },{

            personId: 4,

            name: 'Mr. Anderson',

            rewardPrograms: {air: {'AA': 'AAA444','AK': 'AAK444','VA': 'AVA444'}}

        }

    ],

    trip: {

        flights: [{

                travelerIds: [1, 2, 3],

                legs: [{airlineCode: 'AA', flightNumber: 'AA456'}]

            },{

                travelerIds: [1, 2],

                legs: [{airlineCode: 'VA', flightNumber: 'VA789'}, {airlineCode: 'AK', flightNumber: 'AK789'}]

            }

        ]

    },

    airlines: [

        { code: 'AK', name: 'Alaskan' },

        { code: 'AA', name: 'American' },

        { code: 'BA', name: 'British' },

        { code: 'DT', name: 'Delta' },

        { code: 'UA', name: 'United' },

        { code: 'VA', name: 'Virgin' },

    ]

};

Update.

I will remind you that code review is about reviewing existing code and not "how do I?" which your comments imply.

The reset is part of the update.

O(n) must define n

You original code which you quoted to be

"...trying my best to keep this O(n) complexity."

Could be anything depending on what you define n to be. The complexity is better expressed in three terms n = flights, m = profiles, l = mean profiles per flight. The legs can be ignored as they would likely have a mean near 1.

Your solution and the example I gave both have O(nml)

You can improve by mapping the profiles (as pointed out in the original answer) which would bring the solution down to O(2m+nl) as shown in the changes to acquireTravelFlightData and mergeTravelers below.

async function acquireTravelFlightData() { 

    const mapAirlines = (map, airline) => (map[airline.code] = airline.name, map);

    const mapProfiles = (map, profile) => {

        map[profile.personId] = {

            id : profile.personId,

            name : profile.name,

            rewards : profile.rewards,

            flights : ,

        }

        return map;

    };

    var profiles;

    return { 

        profileMap: (profiles = await profilesAPI.get()).reduce(mapProfiles,{}),

        profiles,

        flightList: await tripsAPI.get().trip.flights,  

        airlineMap: await airlinesAPI.get().airlines.reduce(mapAirlines,{}),

    };

}    

function mergeTravelers({profileMap, profiles, flightList, airlineMap}) {    

    const createReward = (program, code) => program[code] ? program[code] : "";

    const travelers = { travelers :  };

    flightList.forEach(flight => {

        flight.travelerIds.forEach(id => {

            profile = profileMap[id];

            profile.flights = flight.legs.reduce((flights, leg) = (flights.push({   

                    ...leg,

                    airlineName: airlineMap[leg.airlineCode],

                    frequentFlyerNumber: createReward(profile.rewards, leg.airlineCode)

                }), flights),

                profile.flights

            );

        });

    });

    return {travelers : profiles.map(profile => {

            delete profileMap[profile.personId].rewards;

            return profileMap[profile.personId];

        }

    )};

}

_{Note: the above example is untested and may contain typos}

Thus we can reduce the complexity to O(nl) as 2m is trivial compared to nl. Or you can express the complexity in terms of total number of passenger flights n₁ = nl or O(n₁)

Now the solution is O(n) by defining n as the total number of passenger flights.