trjhtr

I did an exercise of a train travel planner. But it seems like a lot of code to do a simple thing? Is there a better way of writing the JavaScript?

Overview:
The planner needs to be able to find connecting station between origin and destination, then print out the traveling line. Also state how many stops there are.

Please see this JS fiddle containing my code for this exercise.

HTML:

<form id="trip-planner-form">
 <label for="origin">Origin</label>
 <select name="origin" id="origin"></select>
 <label for="destination">Destination</label>
 <select name="destination" id="destination"></select>
 <button type="submit">Check</button>
</form>

<div id="planner-result"></div>

JS Code:

const data = [
 id: 'alamein',
 label: 'Alamein',
 stops: [
 'Flinders Street',
 'Richmond',
 'East Richmond',
 'Burnley',
 'Hawthorn',
 'Glenferrie'
 ]
, 
 id: 'glen-waverly',
 label: 'Glen Waverly',
 stops: [
 'Flagstaff',
 'Melbourne Central',
 'Parliament',
 'Richmond',
 'Kooyong and Tooronga'
 ]
, 
 id: 'sandringham',
 label: 'Sandringham',
 stops: [
 'Southern Cross',
 'Richmond',
 'South Yarra',
 'Prahran',
 'Windsor'
 ]
];

const form_submit = document.getElementById('trip-planner-form');
const planner_result = document.getElementById('planner-result');
const origin_select = document.getElementById('origin');
const destination_select = document.getElementById('destination');


DOMReady();


function DOMReady() 
 console.log('dom ready');
 loadStops(getStops(data));
 set_planner_form_events();


function getStops(lines) 
 return lines.reduce((accum, current) => 
 return accum.concat(current.stops);
 , )
 // remove duplicate
 .filter((f, index, arr) => 
 return arr.indexOf(f) === index;
 ).sort();


function loadStops(stops) 

 stops.forEach(stop => 
 var stop_option = document.createElement('option');
 stop_option.value = stop;
 stop_option.textContent = stop;
 // Append
 origin_select.appendChild(stop_option);
 destination_select.appendChild(stop_option.cloneNode(true));
 );


function set_planner_form_events() 
 form_submit.addEventListener('submit', event => 
 event.preventDefault();
 print_planner_result();
 );



function print_planner_result() 
 var origin = origin_select.value;
 var destination = destination_select.value;
 var result = '';

 if (origin === destination) 
 result = 'You selected the same station.';
 else 
 let traveling_stops = get_traveling_stops(origin, destination);

 console.log(traveling_stops);

 if (traveling_stops) 
 result = `<p>$traveling_stops.concat().join(' --> ')</p>`;
 result += `<p>total: $traveling_stops.length stops </p>`;
 else 
 result = 'Something went wrong.'
 

 

 planner_result.innerHTML = result;



function get_traveling_stops(origin, destination) 
 // Find origin and destination index
 var origin_line_index = data.findIndex(d => d.stops.indexOf(origin) > -1);
 var dest_Line_index = data.findIndex(d => d.stops.indexOf(destination) > -1);

 if (origin_line_index === -1 


function get_connecting_station(line1, line2) 

 for (var i = 0; i < line1.stops.length; i++) 
 let stop_index = line2.stops.indexOf(line1.stops[i]);
 if (stop_index > -1) 
 return line1.stops[i];
 
 

 return false;

Simple solution

The problem is trivial as there is only one hub station. This means that there is only one root between any two stations.

You need only find which line each station is on. If not the same line then find the root on starting line to the hub and from the destination station hub to the destination station.

If the problem involved a configuration that allowed more than one root between stations it becomes much more involved but your code does not show that is an issue.

Simplify using functions.

The reason why your code seams long is because you have not compartmentalised functionality in the form of functions.

You can break it down to

• Finding a line for a station


• Getting stops between stations on a line


• Finding a root between stations


• Displaying a root.

By breaking the problem into smaller parts you reduce the overall complexity and make it easier to read and maintain.

function findLineForStation(station) 
 return data.find(line => line.stops.includes(station));


function getStops(line, fromStation ,toStation, skipFirst = false) 
 const idxFrom = line.stops.indexOf(fromStation);
 const idxTo = line.stops.indexOf(toStation);
 const count = Math.abs(idxTo - idxFrom);
 const dir = Math.sign(idxTo - idxFrom);
 const stops = ;
 var i;
 for (i = skipFirst ? 1 : 0; i <= count; i ++) 
 stops.push(line.stops[idxFrom + i * dir]);
 
 return stops;

function findRoot(fromStation, toStation) 
 const hubStation = "Richmond";
 const fromLine = findLineForStation(fromStation);
 const toLine = findLineForStation(toStation);
 if (fromStation === hubStation) fromLine = toLine 
 if (fromLine === toLine) 
 return getStops(fromLine, fromStation, toStation);
 
 return [
 ...getStops(fromLine, fromStation, hubStation),
 ...getStops(toLine, hubStation, toStation, true)
 ];

function displayRoot() 
 root.textContent = findRoot(originElId.value, destinationElId.value).join(" => ");


originElId.addEventListener("change",displayRoot);
destinationElId.addEventListener("change",displayRoot);
const data = [
 id: 'alamein',
 label: 'Alamein',
 stops: ['Flinders Street', 'Richmond', 'East Richmond', 'Burnley', 'Hawthorn', 'Glenferrie']
, 
 id: 'glen-waverly',
 label: 'Glen Waverly',
 stops: ['Flagstaff', 'Melbourne Central', 'Parliament', 'Richmond', 'Kooyong and Tooronga']
, 
 id: 'sandringham',
 label: 'Sandringham',
 stops: ['Southern Cross', 'Richmond', 'South Yarra', 'Prahran', 'Windsor']
];

From <select name="origin" id="originElId"><option value="Burnley">Burnley</option><option value="East Richmond">East Richmond</option><option value="Flagstaff">Flagstaff</option><option value="Flinders Street">Flinders Street</option><option value="Glenferrie">Glenferrie</option><option value="Hawthorn">Hawthorn</option><option value="Kooyong and Tooronga">Kooyong and Tooronga</option><option value="Melbourne Central">Melbourne Central</option><option value="Parliament">Parliament</option><option value="Prahran">Prahran</option><option value="Richmond">Richmond</option><option value="South Yarra">South Yarra</option><option value="Southern Cross">Southern Cross</option><option value="Windsor">Windsor</option></select><br>
Destination <select name="destination" id="destinationElId"><option value="Burnley">Burnley</option><option value="East Richmond">East Richmond</option><option value="Flagstaff">Flagstaff</option><option value="Flinders Street">Flinders Street</option><option value="Glenferrie">Glenferrie</option><option value="Hawthorn">Hawthorn</option><option value="Kooyong and Tooronga">Kooyong and Tooronga</option><option value="Melbourne Central">Melbourne Central</option><option value="Parliament">Parliament</option><option value="Prahran">Prahran</option><option value="Richmond">Richmond</option><option value="South Yarra">South Yarra</option><option value="Southern Cross">Southern Cross</option><option value="Windsor">Windsor</option></select>
<div id="root"></div>