Calculating parking fees, given start and end timestamps

Question

Since I have just started Javascript, I am trying to solve a few problems and improve myself. I encountered such a question. I tried to do everything step by step from the question, but I do not think that my code is efficient. Can someone help me develop it?

Please code for a JavaScript function that accepts two arguments that are; parking "date and time"(timestamp) and return "date and time"(timestamp). At an airport for car parking, the following rules are applied.

Parking costs;

€2 for the first 20 minutes,
rising to €4 for up to 40 minutes,
rising to €6 for up to one hour,
rising to €7 for up to two hours,
rising to €9 for up to three hours,
rising to €11 for up to four hours,
rising to €13 for 4-8 hours and
rising to €15 for 8-24 hours.

€16 for the first 24 hours and after that each additional day is charged at €9.

Here is my code:

function msToHours(milisecond) {
        let time = milisecond;

        let hour = (time / 60000) / 60;

        return hour;
    }


    //Mins to Hours Function
    function minToHours(miniute){

        let time = miniute;

        let hr = (miniute /60);

        return hr;

    }


    //Finding the nth day Function

    function add24HR(hour, cb) {

        let arr = new Array();

        for (let i = 0; i < hour; i++) {
            if (i % 24 == 0) {
                arr.push(i)
            }
        }
    
        return  `Your Parking Fee is £${(arr.length*cb-cb) + 16}.(${arr.length} days)`
    }

    //Main Function

    const parkingFees = (parkingDate, returnDate) => {

        //Defining dates
        var park = new Date(parkingDate)
        var returned = new Date(returnDate);

        //Variables
        var penaltyFee = 9;
        let totalPrice;


        //Time between park and return (miliseconds)
        let totalTime = returned - park

        //MiliSeconds to Hour
        let totalPark = msToHours(totalTime);

        //Mins to Hours
        if (totalPark <= minToHours(20)) {
            return `Your parking fee is only £${2}.`
        } 

        else if(totalPark > minToHours(20) && totalPark <= minToHours(40)){
            return `Your parking fee is only £${4}.`
        }

        else if(totalPark > minToHours(40) && totalPark <= minToHours(60)){
            return `Your parking fee is only £${6}.`
        }
        else if(totalPark > minToHours(60) && totalPark <= minToHours(120)){
            return `Your parking fee is only £${7}.`
        }
        else if(totalPark > minToHours(120) && totalPark <= minToHours(180)){
            return `Your parking fee is only £${9}.`
        }
        else if(totalPark > minToHours(180) && totalPark <= minToHours(240)){
            return `Your parking fee is only £${11}.`
        }
        else if(totalPark > minToHours(240) && totalPark <= minToHours(480)){
            return `Your fparking fee is only £${13}.`
        }
      
        else if(totalPark > minToHours(480) && totalPark < minToHours(1440)){
            
            return `Your parking fee is only £${15}.`
        }

        else if(totalPark > minToHours(1440) && totalPark < minToHours(2880)){
            
            return `Your parking fee is only £${16}.`
        }

        //if totalPark > 24 HRS

        else {

            totalPrice = add24HR(totalPark, penaltyFee)

        }

        return totalPrice;

    }

    document.querySelector("body").innerHTML = (parkingFees("5/12/2020 18:30", "5/18/2020 18:30"))

Answer 1

The main issue I see with the code is the repetitive if/elses at the bottom. Repetitive code is annoying to write, takes some time to read, and is often a bit more likely to have bugs than DRY code. To identify the parking fee, I'd make an array of ranges and their associated fees, then find the appropriate range:

const getCost = (totalHoursParked) => {
  const costs = [
    // [Maximum number of hours, Associated cost]
    [1/3, 2],
    [2/3, 4],
    [1, 6],
    [2, 7],
    [3, 9],
    [4, 11],
    [8, 13],
    [24, 15],
  ];
  if (totalHoursParked > 24) {
    const days = Math.ceil(totalHoursParked / 24);
    return 16 + 9 * (days - 1);
  }
  const found = costs.find(([maxHours]) => totalHoursParked <= maxHours);
  return found[1];
};

Just calculate the hours stayed, then invoke the function above.

Other suggestions regarding your current code:

Always use const when you can - only use let when you must warn readers of the code that you may be reassigning the variable in the future.

Avoid var - If you're going to write in modern ES6+ syntax, great - in which case, better to avoid var, which has problems such as an unintuitive function scope and automatically assigning to a property of the global object when on the top level.

Prefer [] over new Array() - it's shorter to write and easier to read, and using the constructor may tempt you to pass it a number, which will result in a sparse array (sparse arrays can have some very strange behaviors; better to avoid them entirely).

Prefer strict equality - better to avoid == and !=, since they have confusing coercion rules a reader of the code should not have to have memorized in order to be confident of the logic the code is implementing.

add24HR is weird While you can loop one-by-one over the hours and increment when encountering every 24th, it'd be much easier to just divide the number of hours by 24 and take the .ceil or .floor of it as needed.

Use meaningful, precise variable names that clearly indicate what the variable contains. Would someone just glancing at the script know what park means, or the difference between totalTime and totalPark, without looking at where those are defined? Probably not - better to call them something like parkDate, returnDate, totalMSParked, totalHoursParked. The penaltyFee = 9 doesn't make a whole lot of sense either except after one reads the description of the problem and some of the code. parkingFees is a function, so it should probably have a verb in its name, like calculateParkingFees.

Only use string interpolation when you need to interpolate For example, this:

return `Your parking fee is only £${4}.`

may as well be

return `Your parking fee is only £4.`

Semicolons Some of your lines have semicolons, but a lot of lines are missing semicolons too. Best to be stylistically consistent - either use semicolons or don't. Unless you're an expert, I'd highly recommend using semicolons, else you will probably occasionally run into the pitfalls of automatic semicolon insertion.

In all, I don't think there's much need in the code for anything other than a small wrapper around the getCost function:

const calculateParkingFees = (parkDateStr, returnDateStr) => {
  const parkDate = new Date(parkDateStr);
  const returnDate = new Date(returnDateStr);
  const totalMSParked = returnDate - parkDate;
  const totalHoursParked = totalMSParked / (1000 * 3600);
  return `Your parking fee is £${getCost(totalHoursParked)}.`;
}

const getCost = (totalHoursParked) => {
  const costs = [
    // [Maximum number of hours, Associated cost]
    [1/3, 2],
    [2/3, 4],
    [1, 6],
    [2, 7],
    [3, 9],
    [4, 11],
    [8, 13],
    [24, 15],
  ];
  if (totalHoursParked > 24) {
    const days = Math.ceil(totalHoursParked / 24);
    return 16 + 9 * (days - 1);
  }
  const found = costs.find(([maxHours]) => totalHoursParked <= maxHours);
  return found[1];
};

console.log(calculateParkingFees("5/12/2020 18:30", "5/18/2020 18:30")); // 6 days
console.log(calculateParkingFees("5/12/2020 18:30", "5/12/2020 19:45")); // bit more than an hour
console.log(calculateParkingFees("5/12/2020 18:30", "5/12/2020 19:00")); // 30 minutes

Answer 2

The first two things I noticed when looking at your code were:

1: It's unclear why you use a function expression, with const, for the main function, and then use function declarations, with function, for the other functions?

2: Obviously there's a lot of repetition in your code. The string 'Your parking fee is' is repeated ten times. That is not very DRY.

The first thing to do, when presented with a problem like this, is analyze what needs to be done:

1. We need to compute the parking fee given two dates.
2. We need to output the parking fee.

So, let's dive right in and make a little "framework" for this:

function computeParkingFee(parkingDateTime, returnDateTime) {
  // ..... do something .....
}

function outputParkingFee(parkingFee) {
  // ..... do something .....
}

let parkingFee = computeParkingFee('5/12/2020 18:30', '5/18/2020 18:30');
outputParkingFee(parkingFee);

I've chosen function declarations because they will do the job. I'm not going for the shortest possible code here, I want it to be easy to read and to understand.

Let's do the easy bit first, the output of the fee. Almost anything will do, so I'll copy your code:

function outputParkingFee(parkingFee) {
  document.querySelector("body").innerHTML = "Your parking fee is £" + parkingFee + ".";
}

By putting the 'Your parking fee is' string in this function, it exists only once, and, more to the point, in the place where it is actually used. In other words, the output is formatted in the function that generates the output. This makes it easier to modify the output later on.

The more interesting bit is how to compute the parking fee. The smallest time difference, used by the parking meter, is a minute, so it makes sense to compute the time difference in minutes. Let's make a separate function for that:

function timeDifferenceInMinutes(startDateTime, finishDateTime) {
  let start  = new Date(startDateTime);
  let finish = new Date(finishDateTime);
  return Math.floor(((finish - start) / 1000) / 60);
}

I use the Math.floor() function so that only full minutes are added to the result. Now we can make a list, containing the amount of minutes parked and the associated cost:

var parkingCosts = [[  0, 2.00],
                    [ 20, 2.00],
                    [ 40, 2.00], 
                    [ 60, 1.00],
                    [120, 2.00],
                    [180, 2.00],
                    [240, 2.00],
                    [480, 2.00]];

Now this is where analyzing the problem is important. First of all, the customer always spends £2.00. Then after 20 minutes that rises to £4.00, so another £2.00 is added. After 40 minutes another £2.00 is added, after 60 minutes £1.00 is added, and so on.

This list therefore contains the fee due after the given number of minutes have passed.

Note how I always write out money with two numbers after the decimal point. This makes it somewhat easier to recognize these as being money, not just numbers. It's a personal preference.

The list stops when a whole day is reached, because we can easily compute the fee for days, so we don't need to put that in the list. Now we need to use this array, and put it all together. So the whole code for computeParkingFee() becomes this:

var parkingCosts = [[  0, 2.00],
                    [ 20, 2.00],
                    [ 40, 2.00],
                    [ 60, 1.00],
                    [120, 2.00],
                    [180, 2.00],
                    [240, 2.00],
                    [480, 2.00]];
var firstDayFee  = 16.00;
var nextDayFee   = 9.00;

function timeDifferenceInMinutes(startDateTime,finishDateTime) {
  let start  = new Date(startDateTime);
  let finish = new Date(finishDateTime);
  return Math.floor(((finish - start) / 1000) / 60);
}

function computeParkingFee(parkingDateTime, returnDateTime) {
  let minutes = timeDifferenceInMinutes(parkingDateTime, returnDateTime);
  let parkingFee = 0.00;
  if (minutes > 1440) {
    let days = Math.ceil(minutes / 1440);
    parkingFee += firstDayFee + nextDayFee * (days - 1);
  }  else {
    for (let cost of parkingCosts) {
      if (minutes > cost[0]) {
        parkingFee += cost[1];
      } else {
        break;
      }
    }
  }
  return parkingFee;
}

Here is a working JSFiddle.

Having all the fees in one place makes it easier for the "parking manager" to change the fees, without having to look through all the code. The computations themselves are very straightforward. If the duration is more than a day I use firstDayFee and nextDayFee to compute the parking fee, otherwise I loop through the list as I explained.

Basically I replaced your long if (...) {} else if (...) {} else if (...) {} else if ...... by a handy list which I loop through.

This is very simple code, so efficiency is not really that important. I've concentrated on making the code easy to understand and easy to modify.

Answer 3

It isn't completely clear what type the inputs would have from the question, since they just say "timestamp," but the simplest form of timestamp is Unix style timestamp you get from Date.now(). It's just the number of milliseconds since January 1 1970 00:00:00, and the timezone is always UTC. It's usually what programs should operate on, because it means they don't have to worry about localized date formats, time zones, daylight savings, etc.

So I would have started there with understanding the problem as "write a function with the signature function parkingFee(inTimeMillis, outTimeMillis)". You're not even tasked to do output, though you should probably create some output to check your answer.

I don't mind the if ... else if ... structure. Other suggested solutions involving tables are probably reflexively thinking about how they'd write it if the fee structure was pulled from a database or some other sort of configuration mechanism. I would just write the simplest code possible matching the problem statement as close as possible, which is given as if ... else if ... . For this reason, I like the original code better than some of the other answers.

(I would just drop the redundant checks of the style a > x in if (a <= x) { return ... } else if (a > x && a < y) { ... } since if a > x wasn't true we would have returned already...)

I imagine if there are bugs in my version below, they will be pretty easy to spot when I proofread it or when others review it.

function parkingFee(inTimeMillis, outTimeMillis) {
    const duration = outTimeMillis - inTimeMillis;
    if (minutes(duration) < 20) {
        return 2.00;
    } else if (minutes(duration) < 40) {
        return 4.00;
    } else if (hours(duration) < 1) {
        return 6.00;
    } else if (hours(duration) < 2) {
        return 7.00;
    } else if (hours(duration) < 3) {
        return 9.00;
    } else if (hours(duration) < 4) {
        return 11.00;
    } else if (hours(duration) < 8) {
        return 13.00;
    } else if (hours(duration) < 24) {
        return 15.00;
    } else {
        const additionalDays = Math.ceil(days(duration)) - 1
        return 16.00 + 9.00 * additionalDays;
    }
}

function days(millis) {
    return hours(millis) / 24;
}

function hours(millis) {
    return minutes(millis) / 60;
}

function minutes(millis) {
    return seconds(millis) / 60;
}

function seconds(millis) {
    return millis / 1000;
}

Answer 4

Check with management what the meaning of "24 hours" is in connection with daylight savings time. Is it literally 24 times 60 times 60 seconds, or is it the timespan until a correct clock displays the same time again, which is usually 24 hours, but sometimes 23 or 25 hours? If I park from Monday 5:35pm to Friday 5:30pm and I'm charged five days, I'd likely be very, very annoyed and cause you a lot of trouble.

Obviously you'd also be in trouble if I stay from Monday 5:35 pm to Tuesday 6:10 pm and because of DST this is less than 24 hours and and you charge me for two days. And apart from DST, you are in trouble if I stay 24 1/2 hours and you charge 16 + 9 euros which is more than the 15 euros for 24 hours plus 4 euros for forty minutes. You need to clarify that.

So for longer stays, you'd want to use some OS method that helps you to correctly determine the number of days. And now you are in trouble, because that depends on your time zone. Which you don't have. Your management needs to decide how important avoiding complaints is; in their place I'd count the first day as 25 hours and nobody will complain.