5
\$\begingroup\$

Hypothesis

There is a paid parking lot, with the following rates: $1 for the first hour and $0.5 for every subsequent hour. The parking capacity is of 10 spaces.

Required

Make an activity management system for the Parking with the following characteristics:

  • Any parking interval is rounded up, to the nearest hour.
  • When a new car enters the parking lot, the parking system takes in the registration number of the car.
  • When a car leaves the parking, the system issues a summary, depending on the duration of the stay. What information do you consider essential as part of the summary?
  • Any customer can see the list of cars parked in the parking lot at a given time.

Technical requirements

  • The implementation will be done using JavaScript, all data being stored in memory, with no need for permanent storage.
  • Write clean code.
  • Create a simple GUI that allows entering and reading data from the computer; display the data on the same page.

Solution

I used Bootstrap 4 for aesthetics. The script itself uses only JavaScript. jQuery is included for the corect functioning of Bootstrap.

let cars = [];
const addCarButton = document.querySelector('#carButton');
const minLicenseeLength = 7;
const payPerHour = 0.5;
const payFirstHour = 1;
const totalPlaces = 10;

const formatDate = (date) => {
  var hours = date.getHours();
  var minutes = date.getMinutes();
  var ampm = hours >= 12 ? 'PM' : 'AM';
  hours = hours % 12;
  hours = hours ? hours : 12;
  minutes = minutes < 10 ? '0' + minutes : minutes;
  var strTime = hours + ':' + minutes + ' ' + ampm;
  return date.getDate() + "/" + (date.getMonth() + 1) + "/" + date.getFullYear() + " " + strTime;
}

const secondsToHours = (d) => {
  d = Number(d);
  let h = Math.ceil(d / 3600);
  return h;
}

const renterTable = () => {
  let results = '';
  for (var i = 0; i < cars.length; i++) {
    let licensee = cars[i].licensee;
    let arrival = formatDate(cars[i].arrival);
    let leave = cars[i].leave === '-' ? '-' : formatDate(cars[i].leave);

    results += `<tr>
          <td>${licensee}</td>
          <td>${arrival}</td>
          <td>${leave}</td>
          <td>${showStatus(cars[i])}</td>
          <td class="text-right">${makeBill(cars[i])}</td>
          <td class="text-right">
            <button data-row="${i}" onclick="showSummary(event)" data-toggle="modal" data-target="#myModal" class="btn btn-sm btn-success">Summary</button>
                    </td>
        </tr>`;
  }
  document.querySelector("#parking tbody").innerHTML = results;
}

const showStatus = (car) => {
  return car.isParked ? "Parked" : "Has left";
}

const changeStatus = (event) => {
  cars[event.target.dataset.row].isParked = false;
}

const setLeaveTime = (event) => {
  cars[event.target.dataset.row].leave = new Date(Date.now());
}

const countAvailablePlaces = (event) => {
  document.querySelector('#placesCount').innerHTML = totalPlaces - cars.length;
}

const setClassForBadge = () => {
  let badgeClassName = cars.length == totalPlaces ? 'badge badge-danger' : 'badge badge-success';
  document.querySelector('#placesCount').setAttribute('class', badgeClassName);
}

const calculateHoursBilled = (car) => {
  let arrivedAt = new Date(car.arrival).getTime();
  let leftAt = new Date(car.leave).getTime();
  return secondsToHours((leftAt - arrivedAt) / 1000); //duration in seconds
}

const makeBill = (car) => {
  let hoursBilled = calculateHoursBilled(car);
  let billValue = car.isParked ? "-" : "$" + (payFirstHour + (hoursBilled - 1) * payPerHour);
  return billValue;
}

const printSummary = (event) => {
  let car = cars[event.target.dataset.row];
  let sumarryTable = `<table class="table table-bordered m-0">
        <tr>
          <td class="font-weight-bold">Registration number</td>
          <td>${car.licensee}</td>
        </tr>
        <tr>
          <td class="font-weight-bold">Arrival</td>
          <td>${formatDate(car.arrival)}</td>
        </tr>
        <tr>
          <td class="font-weight-bold">Departure</td>
          <td>${formatDate(car.leave)}</td>
        </tr>
        <tr>
          <td class="font-weight-bold">Billable hours</td>
          <td>${calculateHoursBilled(car)}</td>
        </tr>
        <tr>
          <td class="font-weight-bold">Bill value</td>
          <td>${makeBill(car)}</td>
      </tr></table>`;

  document.querySelector('#modalBody').innerHTML = sumarryTable;
}

const showSummary = (event) => {
  changeStatus(event);
  setLeaveTime(event);
  renterTable();
  printSummary(event);

  //Free the parking place, 3 seconds after the summary is released
  setTimeout(function() {
    freeSpot(event);
  }, 3000);
}

const addCar = () => {
  let newLicensee = document.querySelector("#carValue").value;

  let newCar = {
    licensee: newLicensee,
    arrival: new Date(),
    leave: '-',
    isParked: true
  }

  // Add new car to the cars array

  document.querySelector('#message').style.display = 'none';
  if (newLicensee.length >= minLicenseeLength && cars.length < totalPlaces) {
    cars.unshift(newCar);
  } else {
    if (newLicensee.length < minLicenseeLength) {
      document.querySelector('#message').style.display = 'block';
    }
  }

  if (cars.length == totalPlaces) {
    document.querySelector('#carButton').setAttribute('disabled', true);
  }

  setClassForBadge();

  //Update places count
  countAvailablePlaces(event);

  // Empty text box
  document.querySelector("#carValue").value = '';

  // Render the table
  renterTable();
}

const freeSpot = (event) => {
  cars.splice(event.target.dataset.row, 1);
  setClassForBadge();

  if (cars.length == totalPlaces) {
    document.querySelector('#carButton').setAttribute('disabled');
  } else {
    document.querySelector('#carButton').removeAttribute('disabled');
  }

  // Render Table again after delete 
  renterTable();
  //Update places count
  countAvailablePlaces(event);
}

// Add new car to the array
addCarButton.addEventListener('click', addCar);

// Render Table
renterTable();

//Show places count at page load
countAvailablePlaces(event);
#addForm {
  position: relative;
}

#message {
  display: none;
  position: relative;
  font-size: 10px;
  position: absolute;
}

#placesCount {
  font-size: 90%;
}

#parking th,
#parking td {
  white-space: nowrap;
  font-size: 14px;
}

#myModal .font-weight-bold {
  font-weight: 500 !important;
}
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.5.2/css/bootstrap.min.css">
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.5.2/js/bootstrap.min.js"></script>

<div class="container">
  <div class="card my-3">
    <div class="card-header px-3 d-flex">
      <h5 class="m-0">Parking management</h5>
      <div class="ml-auto">
        <span id="placesCount" class="badge badge-success"></span> available places
      </div>
    </div>
    <div class="card-body p-0">
      <div class="input-group p-2" id="addForm">
        <input type="text" class="form-control" id="carValue" placeholder="Registration number">
        <div class="input-group-append">
          <button id="carButton" class="btn btn-sm btn-success">Park car</button>
        </div>
        <p id="message" class="text-danger m-0">Registration number invalid</p>
      </div>
      <div class="table-responsive">
        <table id="parking" class="table table-striped m-0">
          <thead>
            <tr>
              <th>Registration no</th>
              <th>Arrival</th>
              <th>Departure</th>
              <th>Status</th>
              <th class="text-right">Bill</th>
              <th class="text-right">Actions</th>
            </tr>
          </thead>
          <tbody></tbody>
        </table>
      </div>
    </div>
  </div>
</div>

<div class="modal" id="myModal">
  <div class="modal-dialog">
    <div class="modal-content">
      <div class="modal-header">
        <h6 class="modal-title">Sumar Parcare</h6>
        <button type="button" class="close" data-dismiss="modal">&times;</button>
      </div>
      <div id="modalBody" class="modal-body"></div>
    </div>
  </div>
</div>

Do you see any problems with this application, including from a security stand-point?

Thanks!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ A method like countAvailablePlaces is doing two things: the maths and the rendering of the result. These concerns could be separated. \$\endgroup\$ – 52d6c6af Oct 20 '20 at 14:31
5
\$\begingroup\$

Prefer const over let and var. When you declare a variable with let, you're indicating to any reader of the code that the variable may be reassigned at any time. When the reassignment isn't actually necessary for the code to work, this results in undesirable cognitive overhead - it may be a constant worry in the back of one's mind "This variable name may be reassigned at any time, so what it's referring to now may not be what it was originally assigned to.* See https://softwareengineering.stackexchange.com/questions/278652/how-much-should-i-be-using-let-vs-const-in-es6

Best to avoid var entirely, it has too many problems to be worth using nowadays - most importantly, it has unintuitive function scope instead of block scope.

String padding When you want to pad the start of a string with some leading characters to make the string a particular length (eg, here, 9 and 10 to 09 and 10 respectively), the most appropriate method to use is padStart

Template literals are great when you need to interpolate multiple variables into a string - they're often preferable to the style of ' + someVar + '.

Taking the above 3 tips into consideration, the formatDate function can be refactored to:

const formatDate = (date) => {
  const hoursMilitary = date.getHours();
  const minutesToDisplay = String(date.getMinutes()).padStart('0', 2);
  const ampm = hoursMilitary >= 12 ? 'PM' : 'AM';
  const hoursToDisplay = (hoursMilitary % 12) || 12;
  const strTime = `${hoursToDisplay}:${minutesToDisplay} ${ampm}`;
  return `${date.getDate()}/${date.getMonth() + 1}/${date.getFullYear()} ${strTime}`;
}

secondsToHours can be made much more concise, and without reassignment of the parameter:

const secondsToHours = d => Math.ceil(d / 3600);

/ will coerce non-numbers to numbers; you don't need to call Number on d first.

renterTable Lots of improvements can be made here. First, the function name isn't entirely intuitive - what does something called renterTable (noun) do? Is it a variable that contains an HTMLTableElement? Does it create a table and return it? No, it re-renders the table in the DOM given the data in the cars array. Consider calling it renderTable or renderRenterTable (verb) instead.

Constructing HTML by concatenating user input is unsafe You have:

  results += `<tr>
        <td>${licensee}</td>
        <td>${arrival}</td>
        <td>${leave}</td>
        <td>${showStatus(cars[i])}</td>
        <td class="text-right">${makeBill(cars[i])}</td>
        <td class="text-right">
          <button data-row="${i}" onclick="showSummary(event)" data-toggle="modal" data-target="#myModal" class="btn btn-sm btn-success">Summary</button>
                  </td>
      </tr>`;
}
document.querySelector("#parking tbody").innerHTML = results;

This will allow for arbitrary code execution. Imagine if someone said:

Try plugging in the following license plate when registering a car, you won't believe what happens next!

<img src onerror='alert("evil")'>

where the alert("evil") can be replaced with anything the phisher wants. Then some not-too-intelligent users might fall for it and get their login info / account funds / etc compromized.

This can be fixed by making sure the licensee contains only valid characters first, eg, turn:

let newLicensee = document.querySelector("#carValue").value;
if (!/^[a-z\d]+$/.test(newLicensee)) {
  // display error message to user: license invalid
  return;
}

But that's still only a patch job. If the script gets developed in the future, I'd be afraid of accidentally not sanitizing an input before inserting it into the DOM. When user input is involved, I'd prefer to avoid string interpolation entirely with HTML, and instead insert the textContent into the element after the element is put into the document, eg:

const tr = document.querySelector("#parking tbody").appendChild(document.createElement('tr'));
tr.innerHTML = `
  <td></td>
  <td></td>
  <td></td>
  ...
`;
tr.children[0].textContent = licensee;
tr.children[1].textContent = arrival;
tr.children[2].textContent = leave;
// ...

Or, even better, for larger projects, consider using a framework that allows for the concise interpolation of input into the DOM, like React:

const CarRow = (carInfo) => (
  <tr>
    <td>{carInfo.licensee}</td>
    <td>{carInfo.arrival}</td>
    <td>{carInfo.leave}</td>
    ...

All of the above also applies to the sumarryTable, which has the same sort of concatenated-HTML vulnurability

Spelling Spelling matters in programming - properly spelled variable names help prevent bugs. Call it summaryTable instead. Also, the word "licensee" refers to the person who holds a license, but your variable refers to the license plate string. Consider calling it licenseID instead of licensee; it'll be less confusing.

Avoid innerHTML For similar reasons to the above, only use innerHTML when you need to insert HTML markup. If you just want to insert text into an element, use textContent; it'll be safer, faster, and won't have problems with characters that have a special meaning in HTML. This:

document.querySelector('#placesCount').innerHTML = totalPlaces - cars.length;

can be

document.querySelector('#placesCount').textContent = totalPlaces - cars.length;

(Could also save the reference to #placesCount instead of re-selecting it each time the function is called)

Avoid inline handlers Inside the row markup, you have:

<button data-row="${i}" onclick="showSummary(event)" data-toggle="modal" data-target="#myModal" class="btn btn-sm btn-success">Summary</button>

Inline handlers have too many problems to be worth using in modern code. They require global pollution and have a crazy scope chain, among other issues. Attach the event listener properly using JavaScript instead. Either attach the listener to each button when the button gets inserted into the DOM, or, possibly a better option, use event delegation. See comments below too:

table.addEventListener('click', (event) => {
  if (!event.target.matches('button')) {
    return;
  }
  // I renamed this from "row" to "rowIndex" to make the variable name more precise;
  // it's not an actual row, it's only an index
  const { rowIndex } = event.target.dataset;
  // Now that the rowIndex has been extracted, grab the car,
  // then pass the car along instead of extracting it inside each function:
  const car = cars[rowIndex];
  changeStatus(car);
  setLeaveTime(car);
  
  renderRenterTable();
  printSummary(car);

  //Free the parking place, 3 seconds after the summary is released
  setTimeout(function() {
    freeSpot(rowIndex);
  }, 3000);
});

Prefer dot notation when possible, it's more concise and easier to read - you have

document.querySelector('#placesCount').setAttribute('class', badgeClassName);
// ...
if (cars.length == totalPlaces) {
  document.querySelector('#carButton').setAttribute('disabled');
} else {
  document.querySelector('#carButton').removeAttribute('disabled');
}

These can be changed to:

document.querySelector('#placesCount').className = badgeClassName;
// ...
document.querySelector('#carButton').disabled = cars.length === totalPlaces;

(remember to always use === instead of ==; == has weird rules that readers of a script should not have to understand in order to grasp the logic being implemented)

Modern syntax Modern syntax and modern methods are being used in the source code, which is good - it makes for clean, concise, readable code. For production code, use Babel and polyfills to automatically transpile the source down to ES5 so that obsolete browsers like IE can run the code too.

\$\endgroup\$
3
  • \$\begingroup\$ The application supposes the existence of a system that reads vehicle registration plates, so I think there is no need for validation. \$\endgroup\$ – Razvan Zamfir Oct 20 '20 at 19:03
  • 3
    \$\begingroup\$ Your question specifically asked about security, and did not say that the input was trustworthy. Even if the license happens to be trustworthy here, the general approach of concatenating HTML is still not a good idea, as detailed in the answer - when developing the script further, it's easy to make a mistake and open your users up to arbitrary code execution. \$\endgroup\$ – CertainPerformance Oct 20 '20 at 19:17
  • \$\begingroup\$ Please give an answer to this question too. Thanks! \$\endgroup\$ – Razvan Zamfir Oct 26 '20 at 23:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.