Patrick El-Hage

Refactoring A Simple JavaScript App

January 06, 2015

Over 8 months ago I coded my first interactive web app — a sleep cycle calculator called Slumbr. Slumbr suggests optimal times to sleep or wake up by estimating when you’ll most likely be in your lightest phase of sleep.

I haven’t looked at the code in quite some time so I figured that since I’m more experienced now than I was 8 months ago, it would be a fun exercise to see how my first iteration of Slumbr can be improved.

The original javascript file is a total 150 lines, and after refactoring it I was easily able to reduce the number of lines by more than half, to 70.

Here’s the original file- notice any redundancy?:

First Iteration

//Obtaining current Time
var now = new Date() //Gets current Time
var cycle = 90 * 60000 //This is the length of a sleep cycle, expressed in milliseconds
function sleepNow() {
  document.getElementById('1').innerHTML = ''
  document.getElementById('2').innerHTML = ''
  document.getElementById('3').innerHTML = ''
  document.getElementById('4').innerHTML = ''
  document.getElementById('5').innerHTML = ''
  document.getElementById('6').innerHTML = ''
  var i = 0
  for (i = 1; i < 7; i++) {
    var d = new Date(now.getTime() + 900000 + i * cycle) //Adding a cycle for each loop
    var hh = d.getHours()
    var m = d.getMinutes()
    var dd = 'AM'
    var h = hh
    if (h >= 12) {
      h = hh - 12
      dd = 'PM'
    }
    if (h == 0) {
      h = 12
    }
    if (m < 10) {
      m = '0' + m
    }
    if (i == 1) {
      document.getElementById('6').innerHTML += h + ':' + m + ' ' + dd + '  '
    } else if (i == 2) {
      document.getElementById('5').innerHTML += h + ':' + m + ' ' + dd + '  '
    } else if (i == 3) {
      document.getElementById('4').innerHTML += h + ':' + m + ' ' + dd + '  '
    } else if (i == 4) {
      document.getElementById('3').innerHTML += h + ':' + m + ' ' + dd + '  '
    } else if (i == 5) {
      document.getElementById('2').innerHTML += h + ':' + m + ' ' + dd + '  '
    } else if (i == 6) {
      document.getElementById('1').innerHTML += h + ':' + m + ' ' + dd + '  '
    }
  }
}

function wakeUp() {
  //RETRIEVE TIMES FROM PERSON
  var hourInput = parseInt(document.getElementById('hourInput').value)
  var minInput = parseInt(document.getElementById('minInput').value)

  //MAKE SURE IT IS EMPTY EACH TIME YOU ARE PRESSING CALC
  document.getElementById('1').innerHTML = ''
  document.getElementById('2').innerHTML = ''
  document.getElementById('3').innerHTML = ''
  document.getElementById('4').innerHTML = ''
  document.getElementById('5').innerHTML = ''
  document.getElementById('6').innerHTML = ''

  //CHECKING TO MAKE SURE INPUTS ARE CORRECT
  if (
    isNaN(hourInput || minInput) ||
    (hourInput > 12 || minInput < 0 || minInput > 59 || hourInput === '')
  ) {
    document.getElementById('xxx').innerHTML =
      'Please use the correct HH:MM format'
  }
  if (document.getElementById('myonoffswitch').checked && hourInput == 12) {
    hourInput = 0
  }

  var wakeTime = new Date(0, 0, 0, hourInput, minInput)

  //IF SET TO WAKE UP, THEN DO THIS
  if (document.getElementById('myonoffswitch2').checked) {
    for (i = 6; i > 0; i--) {
      var d = new Date(wakeTime.getTime() - i * cycle)
      var hh = d.getHours()
      var m = d.getMinutes()
      var dd = 'AM'
      var h = hh
      if (h >= 12) {
        h = hh - 12
        dd = 'PM'
      }
      if (h == 0) {
        h = 12
      }
      if (m < 10) {
        m = '0' + m
      } else if (i == 1) {
        document.getElementById('6').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 2) {
        document.getElementById('5').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 3) {
        document.getElementById('4').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 4) {
        document.getElementById('3').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 5) {
        document.getElementById('2').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 6) {
        document.getElementById('1').innerHTML += h + ':' + m + ' ' + dd + '  '
      }
    }
  }
  //IF SET TO SLEEP, THEN DO THIS
  if (!document.getElementById('myonoffswitch2').checked) {
    for (i = 1; i < 7; i++) {
      var d = new Date(wakeTime.getTime() + i * cycle)
      var hh = d.getHours()
      var m = d.getMinutes()
      var dd = 'AM'
      var h = hh
      if (h >= 12) {
        h = hh - 12
        dd = 'PM'
      }
      if (h == 0) {
        h = 12
      }
      if (m < 10) {
        m = '0' + m
      }
      if (i == 1) {
        document.getElementById('6').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 2) {
        document.getElementById('5').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 3) {
        document.getElementById('4').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 4) {
        document.getElementById('3').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 5) {
        document.getElementById('2').innerHTML += h + ':' + m + ' ' + dd + '  '
      } else if (i == 6) {
        document.getElementById('1').innerHTML += h + ':' + m + ' ' + dd + '  '
      }
    }
  }
}

Ew. Let’s take a look at a slightly improved version:

Refactored version

var now = new Date() //Gets current Time
var cycle = 90 * 60000 //Length of a sleep cycle, expressed in ms

function convertHour(h) {
  if (h >= 12) {
    return h - 12
  } else if (h == 0) {
    return 12
  } else {
    return h
  }
}

function convertMin(m) {
  return m < 10 ? '0' + m : m
}

function setAM_PM(h) {
  return h == 0 ? 'PM' : 'AM'
}

function sleepNow() {
  for (i = 1; i < 7; i++) {
    var d = new Date(now.getTime() + 900000 + i * cycle) //Adding a cycle for each loop
    var h = convertHour(d.getHours())
    var m = convertMin(d.getMinutes())
    var dd = setAM_PM(h)

    document.getElementById(i).innerHTML = h + ':' + m + ' ' + dd
  }
}

function wakeUp() {
  //RETRIEVE TIMES FROM PERSON
  var hourInput = parseInt(document.getElementById('hourInput').val)
  var minInput = parseInt(document.getElementById('minInput').val)

  //CHECKING TO MAKE SURE INPUTS ARE CORRECT
  if (
    isNaN(hourInput || minInput) ||
    (hourInput > 12 || minInput < 0 || minInput > 59 || hourInput === '')
  ) {
    document.getElementById('xxx').innerHTML =
      'Please use the correct HH:MM format'
  }
  //IF MIDNIGHT, SET HOUR TO ZERO (JS uses 24 hour time)
  if ($('#myonoffswitch').is(':checked') && hourInput == 12) {
    hourInput = 0
  }

  var wakeTime = new Date(0, 0, 0, hourInput, minInput)

  //IF SET TO WAKE UP, THEN DO THIS
  if ($('#myonoffswitch2').is(':checked')) {
    for (i = 6; i > 0; i--) {
      var d = new Date(wakeTime.getTime() - i * cycle)
      var h = convertHour(d.getHours())
      var m = convertMin(d.getMinutes())
      var dd = setAM_PM(h)

      document.getElementById(i).innerHTML = h + ':' + m + ' ' + dd
    }
  }
  //IF SET TO SLEEP, THEN DO THIS
  if (!$('#myonoffswitch2').is(':checked')) {
    for (i = 1; i < 7; i++) {
      var d = new Date(wakeTime.getTime() + i * cycle)
      var h = convertHour(d.getHours())
      var m = convertMin(d.getMinutes())
      var dd = setAM_PM(h)

      document.getElementById(i).innerHTML = h + ':' + m + ' ' + dd
    }
  }
}

This refactored version of Slumbr still has some repetition that can get factored out, so I might revisit this again and cut it down even more.

I'm Patrick El-Hage and I live and work in San Francisco. I'm also on Twitter.