Sort and describe itinerary segments

Given some incoming some data in JSON format, representing unsorted train, bus, and flight tickets:

[{'from':'A', to:'B',...}, {'from':'C', to:'D',...}, {'from':'B', to:'C',...}];

… I must sort tickets (A-B-B-C-C-D) and generate text (description) with all information about each trip (number of flight, gate, seat - it comes with JSON objects). But with some requirements:

• it must be "API format".

extensible code (should provide way to add information about different type of transport).

• clean and easy code.

I want to be honest with you, imagine that I'm junior in your team, and that I write for real project.

Here is my solution.



`var tickets = [
{
from: "London",
to: "Paris",
transportType: "train",
transport: {
"number": "54S",
"seat": "23"
}
},
{
from: "Berlin",
to: "Akrich",
transportType: "bus",
transport: {
"number": "SR465",
"seat": "56",
"gate": "2A",
}
},
{
from: "Paris",
to: "Berlin",
transportType: "flight",
transport: {
"number": "SR465",
"seat": "56",
"gate": "2A",
"baggage": "will be automatically transferred from your last leg"
}
},
{
from: "Moscow",
to: "London",
transportType: "flight",
transport: {
"number": "N554",
"seat": "4A",
"gate": "22",
"baggage": "drop at ticket counter 344"
}
}
];

function TripSorter ( tickets ) {
this.from = {};
this.to = {};

this.path = [];
this.tickets = [];

if ( tickets ) {
this.importTickets( tickets );
}
}

// copy data in this.tickets.
// create "from" ( contains only "from" points of route ) and "to" objects (hashMap);
TripSorter.prototype.importTickets = function ( tickets ) {
var self = this;
self.tickets = [

Organization


How I should organize code inside of this file (where to place
"transport constructor" and where "TripSorter")?

Declare things before using them. Declare dependencies first.

• Function declarations and class definitions (constructor functions) go to the top, example data and code to the bottom.

• TripSorter instantiates and holds Transport instances, so declare Transport first.

Inheritance


Is it correct to use Transport constructor and inherit from it or I
must used switch(type of transport) - case (message + )? What way is
easy to extend?

Inheritance is an extensible approach. However, it requires some overhead and you need to declare some kind of factory function - here: getTrip. This function features a switch statement. In JavaScript, it is often advised to replace switch statements with maps, usually object literals.

Also, your Transport objects currently don't have any methods. They even don't hold any meaningful data apart from one message attribute. Once the constructor function returns an instance, you just use it to retrieve the message.

Additionally, by pushing subclass functionality which is shared by some but not all subclasses - e.g. initialization of gate and baggage - upwards to the parent Transport constructor, you attempt to reduce code repetition at the cost of clear semantics and maintainable code:

let train = new Train({
  from: "Beijing",
  to: "Tianjin",
  transport: {number: "CR120"}
});
// Why do trains have a gate and baggage attribute?
console.log(train); // Object { type: undefined, gate: "", baggage: "...", ... }

You redundantly store type information in the type attribute. However, your Transport instances already have type information attached to them:

train instanceof Train // true

I suggest you ditch inheritance and the additional overhead by directly mapping transport types to functions (which are objects, too) which return descriptions:

const descriptors = {
  "walk": (from, to) => `Walk from ${from} to ${to}.`,
  ...
}
descriptors["walk"]("Berlin", "Hamburg"); // Walk from Berlin to Hamburg.

Constructor functions

Don't return anything from a constructor function. You currently return this.m in Train, Flight and so on. Luckily, this.m is undefined - and when returning undefined from a constructor function, the new operator returns the newly created object instance.

API design

Your TripSorter is stateful and its methods must be called in a correct order. Calling importTickets(tickets) after already having supplied tickets to the constructor results in unexpected behavior. Calling getTrip before buildTrip is meaningless. Et cetera. I don't see any good reason to split one indivisible 'sort tickets' functionality into several public methods.

Print results


I must add some functionality to print generated text to html - is it
ok that I put it inside of TripSorter prototype or its better to use
different function?

So far, you defined some models and business logic. You should not mix those with presentation logic - this should be part of your 'view' layer. By keeping those parts of your application separate, you can test and reuse them much easier.

Error handling


I used console.error to handle error of ticket format — is it correct
way?

Your TripSorter.prototype.importTicket methods logs errors to the console when the from or to attribute of a ticket is missing. However, it still resumes operations and returns the first connected sub-trip, ignoring other connected segments.

Also, you do explicitly check for single-space strings but still accept multi-space strings:

if ( !e.from || e.from === ' ' ) { ... }

You also don't check for cycles and you don't validate the transportType and transport attribute. In some cases, you simply ignore the incorrect ticket and continue silently, sometimes you throw a TypeError and sometimes, you log an error to the console. All in all, your input validation and error handling feels incomplete and random.

I suggest you write robust code that expects valid tickets but still returns consistent results for 'invalid' attributes. Your TripSorter cannot possibly validate from and to attributes, as the knowledge about valid city names is part of another business unit in your code. You could / should therefore validate tickets beforehand and e.g. encapsulate them in a class that doesn't allow construction of invalid tickets. Fail early and document clearly how and when you fail:

// @throws {Error} when itinerary is ambiguous or circular.

Comments


Are the comments normal?

Your comments mainly focus on implementation details:

// start of trip - unique key in obj from compared to obj to.
TripSorter.prototype.getStartTrip = function () { ... }

However, I recommend more 'documentation style' comments that tell the users of your functions, classes and modules about their purpose, the

StackExchange Code Review Q#162439, answer score: 2