8
\$\begingroup\$

So for example, getTimes("12:00", "15:30"); should return: ["12:00", "12:30", "13:00", "13:30", "14:00", "14:30", "15:00", "15:30"]

The input parameters are always in :30 or :00 intervals(so 06:00, 06:30, 07:00, ...). getTimes("06:00", "17:00"); -> should work

getTimes("06:30", "17:00"); -> should work

getTimes("06:25", "17:00"); -> doesn't have to work because the from or until time can never be "06:25"

It also only has to work for the range 00:00 - 23:30 the same day.

What I have right now is this:

function getTimes(from, until){
            //"01/01/2001" is just an arbitrary date
            var until = Date.parse("01/01/2001 " + until);
            var from = Date.parse("01/01/2001 " + from);
            //*2 because because we want every 30 minutes instead of every hour
            var max = (Math.abs(until-from) / (60*60*1000))*2;
            var time = new Date(from);
            var hours = [];
            for(var i = 0; i <= max; i++){
                //doubleZeros just adds a zero in front of the value if it's smaller than 10.
                var hour = this.doubleZeros(time.getHours());
                var minute = this.doubleZeros(time.getMinutes());
                hours.push(hour+":"+minute);
                time.setMinutes(time.getMinutes()+30);
            }
            return hours;
}

How can this be improved?

\$\endgroup\$
4
\$\begingroup\$

Naming (more later)

I'd recommend renaming your function to something more specific than getTimes to avoid collisions with other getTimes functions. A more specific name also provides more information on what the function does.

function createHalfHourIntervals() {}

Looser Coupling

Another potential improvement would be to decouple your time formatting from your time interval creating. You could use a sleek functional programming technique to easily add the formatting after you've made the intervals:

function createHalfHourIntervals(from, until){
  // ...
  var time = new Date(from);
  var intervals = []; // more clear name than hours

  for (var i = 0; i <= max; i++) {
    intervals.push(time);
    time.setMinutes(time.getMinutes() + 30);
  }
}

function formatDateHHcolonMM(date) {  // funny name but specific
  var hour = date.getHours();
  var minute = date.getMinutes();
  return doubleZeros(hour) + ":" + doubleZeros(minute);
}

var intervals = createHalfHourIntervals()
  .map(formatDateHHcolonMM); // map will create a new array of formatted intervals from the array returned by createHalfHourIntervals

This separates your formatting logic from your interval creation logic, making your code more reusable and cohesive. If another programmer were less familiar with your code, but wanted to change the formatting of your intervals in someway, that task is made much simpler in this format. Each section of code doing one task, in general, can make the project easier to understand. However, if you don't plan on reusing much of the code in getTimes or working with other developers, sometimes it can just be simpler to keep all the code contained in one single location.

Good Optional Ideas

Although you may not want it now, it's possible that you may want to create arrays of intervals of different lengths in the future. Again, this depends on what you actually need, but 20min intervals or even interval times offset by 5min [7:05, 7:35] can be quite common. You could prepare better for the future change by reworking your function a bit. Your "01/01/2001 ", "*2" and "+30" are what some consider magic numbers, which tend to make the code less readable--sometimes even requiring their own comments. A good variable name oftentimes is better than a comment, so you may want to give these numbers their own variable names, improving clarity while preparing for a future refactoring:

function createHalfHourIntervals(from, until) {
  var arbitraryDay = "01/01/2001 ";
  // ...

  var intervalLength = 30;
  var intervalsPerHour = 60 / intervalLength;
  var milisecsPerHour = 60 * 60 * 1000;
  // are you sure that Math.abs(until - from) is the behavior you want. It seems like preparation to allow the user to put the 'from' and 'until' parameters in the other order.
}

That's pretty much all of your code! Hope you find this helpful, and keep up the good work!

\$\endgroup\$
2
\$\begingroup\$

Imo, the Date class is a red herring. This problem is better modeled as integers. A day has 48 half hour intervals. Let the numbers 0 through 47 be your model. Then you need only create:

  1. A function to convert from time format to an integer
  2. From an integer back to a time format
  3. A utility that takes two integer endpoints and returns a range

These are all essentially one-liners:

var toInt  = time => ((h,m) => h*2 + m/30)(...time.split(':').map(parseFloat)),
    toTime = int => [Math.floor(int/2), int%2 ? '30' : '00'].join(':'),
    range  = (from, to) => Array(to-from+1).fill().map((_,i) => from + i),
    eachHalfHour = (t1, t2) => range(...[t1, t2].map(toInt)).map(toTime);

console.log(eachHalfHour('12:00', '15:30'))
\$\endgroup\$
  • \$\begingroup\$ I really like how direct your solution is. It's quite impressive. Maybe it's just written a bit too technically for some people? (You look like a c programmer on ES6 steroids) Keep up the awesome coding! \$\endgroup\$ – Joshua Dawson May 17 '16 at 5:49
  • 1
    \$\begingroup\$ Thanks. Usually these types of solutions are popular, I'm happy with this one. It's worth noting that, in spirit, I'm the opposite of a C programmer. Don't let the superficial syntactic similarity between code like this and golfed C code mislead you. The code above is extremely high-level, whereas C code is low level. The code above is basically a translation of "convert the start and end times to integers, fill in all the integers between them, then convert everything back to a time". It's declarative and functional, whereas C is usually imperative and procedural. \$\endgroup\$ – Jonah May 17 '16 at 5:56
  • \$\begingroup\$ @jonah Hi i like your solution and most efficient coding, in my case i need a below format for single digit values 00.00,00.30,01.00,01.30,02.00,02.30,....etc upto 23.30 , currently which is displaying 0.00,0.30,1.00, which helps more you do the below stuff in same solution \$\endgroup\$ – Sahir Mohamed Sep 6 '18 at 19:35
1
\$\begingroup\$
function GenTimePullDown(MinuteInterval) {
    var o, n, i, g, r, S, l;
    o = "<option value='00:00'>12:00 AM</option>", n = 0, i = 0, r = 0, S = 0, g = 0, l = "AM";
    for (var e = 1; e < 60 / MinuteInterval * 24; e++) 60 == (i += MinuteInterval) && (g = n += 1, n >= 12 && (l = "PM"), i = 0), r = n > 12 ? n - 12 : n, 0 == n && (r = 12), 1 == n.toString().length && (g = "0" + n.toString(), g = "0" + n.toString()), 1 == r.toString().length && (r = "0" + r.toString()), S = 1 == i.toString().length ? "0" + i.toString() : i, o = o + "\n<option value='" + (g.toString() + ":" + S.toString()) + "' >" + (r.toString() + ":" + S.toString() + " " + l) + "</option>";
    return o;
}

Use the above function to get the HTML content of a Pull-down(SELECT) box:

Usage: let's say you want to get the time interval then you would call the above function as :

GenTimePullDown(15) and below would be the sample output:

<option value='00:00'>12:00 AM</option>
<option value='00:15' >12:15 AM</option>
<option value='00:30' >12:30 AM</option>
<option value='00:45' >12:45 AM</option>
<option value='01:00' >01:00 AM</option>
<option value='01:15' >01:15 AM</option>
<option value='01:30' >01:30 AM</option>
<option value='01:45' >01:45 AM</option>
<option value='02:00' >02:00 AM</option>
<option value='02:15' >02:15 AM</option>
<option value='02:30' >02:30 AM</option>
<option value='02:45' >02:45 AM</option>
<option value='03:00' >03:00 AM</option>
<option value='03:15' >03:15 AM</option>
<option value='03:30' >03:30 AM</option>
<option value='03:45' >03:45 AM</option>

Another usage sample usage with complete HTML is as below:

    <!DOCTYPE html>
<html>
<body>

<p>Populate a 15 minute increment time pull down for the entire 24 hours</p>

<p id="demo"></p>
<select id="pulldown"></select>
<script>
function GenTimePullDown(MinuteInterval) {
    var o, n, i, g, r, S, l;
    o = "<option value='00:00'>12:00 AM</option>", n = 0, i = 0, r = 0, S = 0, g = 0, l = "AM";
    for (var e = 1; e < 60 / MinuteInterval * 24; e++) 60 == (i += MinuteInterval) && (g = n += 1, n >= 12 && (l = "PM"), i = 0), r = n > 12 ? n - 12 : n, 0 == n && (r = 12), 1 == n.toString().length && (g = "0" + n.toString(), g = "0" + n.toString()), 1 == r.toString().length && (r = "0" + r.toString()), S = 1 == i.toString().length ? "0" + i.toString() : i, o = o + "\n<option value='" + (g.toString() + ":" + S.toString()) + "' >" + (r.toString() + ":" + S.toString() + " " + l) + "</option>";
    return o;
}

document.getElementById("pulldown").innerHTML = GenTimePullDown(15);
</script>

</body>
</html>
\$\endgroup\$
  • \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ – Toby Speight Feb 25 '19 at 12:31

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.