zzxdw
zzxdw

Reputation: 65

Is there any ways to make the code more clean

This is my code, I wrote this part twice, how to make the code more clean?

if (res.getNano == 0)
              JString(res.getEpochSecond.toString)
              else
              JString(res.getEpochSecond.toString+ " seconds " +res.getNano.toString + " nanoseconds")

Upvotes: 1

Views: 67

Answers (1)

stefanobaghino
stefanobaghino

Reputation: 12794

My best recommendation would be to not do too much within a single scope: make small functions that address one concern at a time. And don't shy away from breaking down one-liners and name intermediate results. I understand that one-liners can be pleasant to write, but they may cause unnecessary unease on the eyes of the reader.

In my edit:

  1. I removed the use of recursion to normalize inputs in favor of a second method (an overload in this case, feel free to rename it if you think it makes sense).
  2. I factored out the logic to render an Instant as a JString into a single method called twice.
  3. I factored out the logic to get from either a LocalDateTime or LocalDate to an Instant into a couple of overloaded methods to keep the main logic a bit cleaner from details
  4. I also used camelCase instead of snake_case, but this is a personal preference, please do stick to snake_case if it's your and yours team preference.

I also thought about making one method return a Try[JValue] and let a second one address what to do with the result, but I think this could be enough for now. Evaluate whether that's the case based on your context.

Furthermore, all methods here are public since this is a simple plain script. I would encourage you to limit the visibility so that only those you want to be exposed are (from what I can see, I would probably leave only the original timeToEpoch(params) as public).

You can play around with this code here on Scastie.

import java.time.format.DateTimeFormatter
import java.time.{LocalDate, LocalDateTime, ZoneId, ZoneOffset}

import org.json4s._

import scala.util.Try

def timeToEpoch(params: List[JValue]): JValue =
  params match {
    case List(JString(timestamp), JString(pattern)) =>
      timeToEpoch(timestamp, pattern, "UTC")
    case List(JString(timestamp), JString(pattern), JString(timezone)) =>
      timeToEpoch(timestamp, pattern, timezone)
  }

def timeToEpoch(
    timestamp: String,
    pattern: String,
    timeZone: String
): JValue =
  Try {
    val formatter = DateTimeFormatter.ofPattern(pattern)
    val df =
      formatter.parseBest(timestamp, LocalDateTime.from _, LocalDate.from _)
    df match {
      case dateTime: LocalDateTime =>
        formatInstant(toInstant(dateTime), ZoneId.of(timeZone))
      case date: LocalDate =>
        formatInstant(toInstant(date), ZoneId.of(timeZone))
      case _ =>
        JNothing
    }
  }.getOrElse(JNothing)

def toInstant(dateTime: LocalDateTime, timeZone: ZoneId): Instant = {
  val offset = timeZone.getRules.getOffset(LocalDateTime.now)
  val offsetAsString = String.valueOf(offset)
  dateTime.toInstant(offsetAsString)
}

def toInstant(date: LocalDate, timeZone: ZoneId): Instant = {
  date.atStartOfDay(timeZone).toInstant
}

def formatInstant(i: Instant): JString = {
  if (res.getNano == 0)
    JString(s"${res.getEpochSecond}")
  else
    JString(s"${res.getEpochSecond} seconds ${res.getNano} nanoseconds")
}

Upvotes: 3

Related Questions