Elevator program code challenge

I recently applied for a job where the company gave me the following prompt for a code challenge. They have since told me "yeah, no thanks" but provided me with no feedback about why they came to that decision.

Would you be so kind as to review my code? You can even be harsh. I'd just like to know what this program is so severely lacking that it didn't warrant a phone call or an interview.


Prompt:


Generate markup HTML and CSS Add functionality to existing code

Turn in your work Provide feedback


Phase 1:


For this portion of the code challenge, you will develop code that
renders an elevator panel that matches the provided mockup. You
will then be adding JS code to make the elevator panel function
according to the functional requirements below. You can control which
way the elevator moves by selecting the “floor” buttons on the panel.
As soon as a button is selected, the elevator will begin moving toward
that floor.


UI Requirements:


A display panel that indicates the number of the current floor Green
and red lights indicating whether the elevator is going up (green) or
down (red) Four floor buttons


Four floors behind the elevator panel


Functional Requirements:


A floor can be selected from the elevator panel.


When a floor is selected, its button is highlighted.


Clicking on a floor button in the panel will cause the floors in the
background to scroll up and down.


Multiple floors can be selected and will be executed in the order that
they were selected.


The chosen button should remain highlighted until the floor is
reached.


The current floor is shown on the display panel.


The elevator's current direction (up/down) is indicated by
highlighting the corresponding green and red indicators in the
display panel.


A visual cue is provided when you have arrived at the selected floor.


Selecting a button anchor does not navigate or change the

In situations like this, you probably get about 30 seconds to make a good first impression. During that time, the two salient observations I had were:

-
Excessive comments: If you need to say everything twice, then your code isn't speaking for itself. Use comments to explain things that aren't obvious to someone who is reading the code. In particular, document function parameters, special values, and unusual conventions.

Don't write clutter like this — the unnecessary comment just annoys the reader.

// reset current target to null
this.targetFloor = null;

-
Modelling: The challenge was probably meant as an exercise in writing model-view-controller code. Consider that there are panel buttons, a numeric floor display, and two arrows that all act as outputs. A loose coupling is called for, in which each of those UI elements can redraw their state based on the elevator state.

Here is an example of what I would be looking for.



var ElevatorController = (function() {
/**
* Constructor. Takes an array of the names of the floors, from bottom
* to top.
*/
function ElevatorController(floors) {
this.floors = floors.map(function(fl) { return fl.toString(); });
this.travelTime = 1500; // milliseconds from floor to floor
this.currentIndex = 0;
this.queue = [];
this.callbacks = [];

this.elevator = {
motion: 0, // -1 = Downward, 0 = Stopped; +1 = Upward
currentFloor: this.floors[0]
};
}

/**
* Registers a function to be called when an event occurs.
* Returns this object, for convenient chaining.
*
* The callback will be called with two parameters. The first
* parameter is the elevator state, which is an object with
* attributes named "motion" (+1, 0, or -1) and "currentFloor".
* The second parameter is a string describing the event type:
* "up", "down", "arrived", or "floor".
*/
ElevatorController.prototype.addCallback = function(callback) {
this.callbacks.push(callback);
return this;
};

ElevatorController.prototype.removeCallback = function(callback) {
for (var i = this.callbacks.length - 1; i >= 0; i--) {
if (this.callbacks[i] === callback) {
this.callbacks.splice(i, 1);
}
}
return this;
};

/**
* Informs callbacks of the current state by sending them a "floor"
* event.
*/
ElevatorController.prototype.refresh = function() {
fireEvent(this, 'floor');
return this;
};

ElevatorController.prototype.press = function(floor) {
var index = this.floors.indexOf(floor.toString());
if (index ctrl.queue[0]) {
ctrl.elevator.motion = -1;
fireEvent(ctrl, "down");
}
if (!ctrl.interval) {
ctrl.interval = window.setInterval(function() { react(ctrl); }, ctrl.travelTime);
}
}

function fireEvent(ctrl, event) {
for (var i = 0; i
.display, .panel {
width: 9em;
text-align: center;
}
.display {
margin: auto;
}
.display div {
display: inline-block;
margin: 0.2em;
}
.number {
padding: 0.2em;
width: 2em;
text-align: right;
}
#up-indicator.lit {
color: green;
}
#down-indicator.lit {
color: red;
}
.panel {
margin: auto;
}
.panel button {
display: block;
margin: auto;
width: 3em;
}
.panel button.lit {
color: yellow;
}


⬇︎

⬆︎

4
3
2
1
P1
P2
`

StackExchange Code Review Q#75303, answer score: 3