6
\$\begingroup\$

I'm currently writing a Scala Object that converts time 24hr format (i.e. 00:00 - 23:59) into Time As Words (i.e. Eight o'clock, Half past eight, Quarter to nine etc).

How can I refactor the current ConvertTime function (Scala Object) into a more functional version using Pattern Matching?

I wanted to originally split out the function into smaller functions but in order to determine the correct case based on the expression they need to be kept within the following order to determine the correct time as words, by eliminating previous conditions.

I found an answer to the following post Multiple of Conditions by Daniel C Sobral that uses pattern matching. I'm not sure if the following approach could be addapted for my case. Any advice will be appreciated.

Python version of word lookup:

words_dict = {1: 'one', 2: 'two', 3: 'three', 4: 'four', 5: 'five',
                  6: 'six', 7: 'seven', 8: 'eight', 9: 'nine', 10: 'ten',
                  11: 'eleven', 12: 'twelve', 13: 'thirteen',
                  14: 'fourteen', 16: 'sixteen', 17: 'seventeen',
                  18: 'eighteen', 19: 'nineteen', 20: 'twenty',
                  21: 'twenty one', 22: 'twenty two', 23: 'twenty three',
                  24: 'twenty four', 25: 'twenty five', 26: 'twenty six',
                  27: 'twenty seven', 28: 'twenty eight', 29: 'twenty nine'}

Python version of if-elif-else conditions:

def time_conversion(words_dict, hours, minutes, period):
    """Return time as words
    based on relevant condition"""
    if hours == 12:
        hours2 = words_dict.get(1)
    else:
        hours2 = words_dict.get(hours+1)
    if hours == 12 and minutes == 0 and period == 'before midday':
        time_words = 'Midnight'
    elif hours == 12 and minutes == 0 and period == 'after midday':
        time_words = 'Midday'
    elif minutes == 0:
        time_words = "{0} o'clock {1}.".format(str(words_dict.get(hours)).title(),
                                               period)
    elif minutes == 15:
        time_words = "Quarter past {0} {1}.".format(words_dict.get(hours),
                                                    period)
    elif minutes == 30:
        time_words = "Half past {0} {1}.".format(words_dict.get(hours),
                                                 period)
    elif minutes == 45:
        time_words = "Quarter to {0} {1}.".format(hours2,
                                                  period)
    elif minutes < 30:
        min_str = words_dict.get(minutes).capitalize()
        min_num = "" if minutes == 1 else "s"
        time_words = "{0} minute{1} past {2} {3}.".format(min_str,
                                                          min_num,
                                                          words_dict.get(hours),
                                                          period)
    else:
        min_str = words_dict.get(60 - minutes).capitalize()
        min_num = "" if 60 - minutes == 1 else "s"
        time_words = '{0} minute{1} to {2} {3}.'.format(min_str,
                                                        min_num,
                                                        hours2,
                                                        period)
    return time_words

Current Scala Object.

package time

/**
  * Created by PeterW on 6/14/2017.
  */
object TimeAsWords extends App {
  // Input string argument is based on 24hr format 23:59
  val InputTime = args(0)

  // regular expression string
  val Exp24hr = """^(([01]\d|2[0-3]):([0-5]\d)|23:59)$"""

  // a Map of words equivalent based on integer value
  val WordsLookup = Map(1 -> "one", 2 -> "two", 3 -> "three", 4 -> "four", 5 -> "five",
                        6 -> "six", 7 -> "seven", 8 -> "eight", 9 -> "nine", 10 -> "ten",
                        11 -> "eleven", 12 -> "twelve", 13 -> "thirteen", 14 -> "fourteen",
                        16 -> "sixteen", 17 -> "seventeen", 18 -> "eighteen", 19 -> "nineteen",
                        20 -> "twenty", 21 -> "twenty one", 22 -> "twenty two", 23 -> "twenty three",
                        24 -> "twenty four", 25 -> "twenty five", 26 -> "twenty six", 27 -> "twenty seven",
                        28 -> "twenty eight", 29 -> "twenty nine")

  // a Map of time periods
  val PeriodLookup = Map(1 -> "before midday", 2 -> "after midday")

  def TimeAsWords(InputTime: String, Exp24hr: String,
                  WordsLookup: Map[Int, String], PeriodLookup: Map[Int, String]) = {

    def ExtractTime(InputTime: String, PeriodLookup: Map[Int, String]): (Int, Int, String) = {
      // extract 12hours, minutes and period from input string
      val HrsInt = InputTime.take(2).toInt
      val Hrs12Int = 12 - ((-HrsInt % 12) + 12) % 12
      val MinInt = InputTime.takeRight(2).toInt
      val PdString = if (HrsInt < 12) PeriodLookup(1) else PeriodLookup(2)
      Tuple3(Hrs12Int, MinInt, PdString)
    }
    def ConvertTime(WordsLookup: Map[Int, String], hours: Int, minutes: Int, period: String): String = {
      // Convert Hours12, Minutes and Period into Time As Words
      // using WordsLookup Map and conditional expressions, return the appropriate string
      val HoursTo = if (hours == 12) 1 else hours + 1
      val MinPastPlural = if (minutes > 1) "s" else ""
      val MinToPlural = if (60 - minutes > 1) "s" else ""
      val Midnight = "Midnight"
      val Midday = "Midday"
      val Oclock = String.format("%s o'clock %s.", WordsLookup(hours).capitalize, period)
      val QuaterPast = String.format("Quarter past %s %s.", WordsLookup(hours), period)
      val HalfPast = String.format("Half past %s %s.", WordsLookup(hours), period)
      val QuarterTo = String.format("Quarter to %s %s.", WordsLookup(HoursTo), period)
      lazy val MinPast = String.format("%s minute%s past %s %s.",
                                        WordsLookup(minutes).capitalize, MinPastPlural, WordsLookup(hours), period)
      lazy val MinTo = String.format("%s minute%s to %s %s.",
                                      WordsLookup(60 - minutes).capitalize, MinToPlural, WordsLookup(HoursTo), period)
      if(hours == 12 && minutes == 0 && period == "before midday"){
        Midnight
      } else if(hours == 12 && minutes == 0 && period == "after midday"){
        Midday
      } else if(minutes == 0){
        Oclock
      } else if(minutes == 15){
        QuaterPast
      } else if(minutes == 30){
        HalfPast
      } else if(minutes == 45){
        QuarterTo
      } else if(minutes < 30){
        MinPast
      } else{
        MinTo
      }
    }
    // validate format, convert to words else return error message
    if (InputTime.matches(Exp24hr)) {
      val (hours, minutes, period) = ExtractTime(InputTime, PeriodLookup)
      val TimeString = ConvertTime(WordsLookup, hours, minutes, period)
      println(TimeString)
    } else {
      println("Input String doesn't match required format: 00:00 - 23:59")
    }
  }
  TimeAsWords(InputTime, Exp24hr, WordsLookup, PeriodLookup)
}
\$\endgroup\$
8
\$\begingroup\$

Output quality

$ scala time.TimeAsWords 00:01
One minute past twelve before midday.
$ scala time.TimeAsWords 11:58
Two minutes to twelve before midday.

I would expect "One minute past midnight" and "Two minutes to noon", respectively.

I suppose that local customs may vary, but I don't think that "before midday" and "after midday" are common expressions in English. I would divide the day into morning, afternoon, evening, and night.

$ scala time.TimeAsWords 00:00
Midnight

Why did the first example end with a ., while this output doesn't?

Code organization

In essence, the task can be broken down into two: parsing the string as a tuple of hours and minutes, then formatting the hours and minutes as English. That is not very obvious to me in your code; the intent is obscured by a lot of unnecessary assignments and parameter-passing. I'd like to see one line that looks like this:

println(formatTime(parseTime(args(0))))

A lot of parameter-passing could be eliminated if you scoped variables properly. For example, PeriodLookup could be defined inside TimeAsWords, and WordsLookup could be defined inside ConvertTime. But I don't see any point to defining PeriodLookup at all, rather than just hard-coding its two values.

Parsing

For the parsing, you are using a regex to perform the validation, but then you end up taking two-character slices to do the real work. It would be simpler if you took advantage of the regex to capture the groups of digits.

I don't know why your regex mentions 23:59 as a special case.

Unfortunately, your parsing function (ExtractTime) produces its result as a triple in 12-hour format. If you did not convert from 24-hour format to 12-hour format at that stage, then both the parsing and the formatting code would be simpler.

The parsing routine can just perform the validation as well. Just throw an exception if it fails. On failure, I recommend printing the error message to System.err, to avoid contaminating System.out with invalid output. I'd also exit the application with a non-zero status.

Formatting

The code would be clearer if you work entirely in lowercase, and capitalize just the first letter of the end result.

Instead of if-else chains, pattern matching would be more idiomatic. It would also enforce some discipline: picking the relevant format is primarily dictated by the minutes, so perform pattern matching on minutes without considering hours at that point.

Instead of lazy val, I'd make the formatting a composition of functions: one function that does the overall formatting, a second function one that converts the hour to English, and maybe a third function that converts the minutes to English. Notice that this leads to neater code: in the solution below, I hardly need to define any variables at all!

I would rename WordsLookup to englishNum. In Scala, a map lookup looks and feels like a function call, so it's not important to put "lookup" in the name. englishNum(5) reads better at the call site than WordsLookup(5).

Suggested solution

object TimeAsWords extends App {
  def parseTime(hhmm: String): (Int, Int) = {
    """^([01]\d|2[0-3]):([0-5]\d)$""".r.findAllIn(hhmm).matchData foreach {
      md => return (md.group(1).toInt, md.group(2).toInt)
    }
    throw new IllegalArgumentException("Input string doesn't match required format: 00:00 - 23:59")
  }

  def formatTime(hhmm: (Int, Int)): String = {
    val englishNum = Map(
      1 -> "one", 2 -> "two", 3 -> "three", 4 -> "four", 5 -> "five",
      6 -> "six", 7 -> "seven", 8 -> "eight", 9 -> "nine", 10 -> "ten",
      11 -> "eleven", 12 -> "twelve", 13 -> "thirteen", 14 -> "fourteen",
      16 -> "sixteen", 17 -> "seventeen", 18 -> "eighteen", 19 -> "nineteen",
      20 -> "twenty", 21 -> "twenty-one", 22 -> "twenty-two",
      23 -> "twenty-three", 24 -> "twenty-four", 25 -> "twenty-five",
      26 -> "twenty-six", 27 -> "twenty-seven", 28 -> "twenty-eight",
      29 -> "twenty-nine"
    )

    def hourFmt(h: Int): String = h match {
      case 0 | 24      => "midnight"
      case 12          => "noon"
      case _ if h < 12 => englishNum(h) + " in the morning"
      case _ if h < 18 => englishNum(h - 12) + " in the afternoon"
      case _ if h < 21 => englishNum(h - 12) + " in the evening"
      case _           => englishNum(h - 12) + " at night"
    }

    def minuteFmt(m: Int): String = "%s %s".format(
      englishNum(m),
      if (m == 1) "minute" else "minutes"
    )

    def fmt(m: Int): (Int => String) = m match {
      case  0          => hourFmt(_)
      case 15          => "quarter past " + hourFmt(_)
      case 30          => "half past " + hourFmt(_)
      case 45          => h => "quarter to " + hourFmt(h + 1)
      case _ if m < 30 => h => "%s past %s".format(minuteFmt(m), hourFmt(h))
      case _           => h => "%s to %s".format(minuteFmt(60 - m), hourFmt(h + 1))
    }

    val (hh, mm) = hhmm
    fmt(mm)(hh).capitalize
  }

  try {
    println(formatTime(parseTime(args(0))))
  } catch {
    case e: IllegalArgumentException => System.err.println(e.getMessage)
    System.exit(1)
  }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the valuable feedback and suggestions. Would you mind explaining how hh is being passed to the fmt function. Also you used anonymous function to call hourFmt within the fmt function for case 0, 15 and 30 then switched to a method call for case 45, < 30 and > 30. Can you explain your reasoning for the following so that I can learn from you solution you provided. \$\endgroup\$ – Peter Wilson Jun 17 '17 at 10:19
  • 1
    \$\begingroup\$ fmt is a function that considers the minute argument and returns a function that, when called with the hour as an argument, produces a complete result. \$\endgroup\$ – 200_success Jun 19 '17 at 7:01
  • 2
    \$\begingroup\$ In retrospect, maybe it would have been better to write all of the cases within fmt in the same style. The reason I couldn't write them all with _ is that that shortcut doesn't work when there are other function calls in the expression, or when there is a +1 involved. \$\endgroup\$ – 200_success Jun 19 '17 at 7:04
  • \$\begingroup\$ More idiomatic english than "before midday" and "after midday" are "A.M." and "P.M.", and you can unabbreviate that to "ante meridian" and "post meridian". I would still use "noon" and "midnight" over "twelve", for clarity \$\endgroup\$ – Caleth Jun 19 '17 at 9:32
  • \$\begingroup\$ Thanks @200_success for the explanation and feedback. I used your solution and made changes based on that o'clock should only be used when the minutes are 0. I've attached my final version of the solution above. \$\endgroup\$ – Peter Wilson Jun 19 '17 at 12:02

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.