https://kotlinlang.org logo
#codereview
Title
# codereview
l

LeoColman

11/24/2021, 1:07 AM
Hey guys!
Copy code
var millis = 123L
var i = 0
buildList<Pair<TimeUnit, Long>> {
  add(Year to (millis / MillisInYear))
  millis -= (get(i++).second * MillisInYear)
  add(Month to (millis / MillisInMonth))
  millis -= (get(i++).second * MillisInMonth)
  add(Day to (millis / MillisInDay))
  millis -= (get(i++).second * MillisInDay)
  add(Hour to (millis / MillisInHour))
  millis -= (get(i++).second * MillisInHour)
  add(Minute to (millis / MillisInMinute))
  millis -= (get(i++).second * MillisInMinute)
  add(Second to (millis / 1_000))
  millis -= (get(i++).second * 1_000)
  add(Millisecond to millis)
}
This code is used to make a timer/counter (picture) However, it's very ugly as it is. How can I enhance this?
j

Joffrey

11/24/2021, 8:06 AM
You can probably use the duration API for this to avoid manual calculations
👍 1
l

LeoColman

11/24/2021, 9:36 AM
Do you have any pointer on how this could be done with duration? I tried, but still had to make manual conversions
manual calculations rather
b

bezrukov

11/24/2021, 11:18 AM
I think Duration api won't help with months/years (because months are different, as well as years) but for days,hours... you can use toComponents:
Copy code
123.milliseconds.toComponents { days, hours, minutes, seconds, nanoseconds ->
    //...
}
👍 1
r

rook

11/24/2021, 4:19 PM
Copy code
listOf(Year, Month, Day, Hour, Minute, Second, Millisecond).forEach {
  val unitsInTotal = millis / it.millis
  add(it to unitsInTotal)
  millis -= unitsInTotal * it.millis
}
👍 2
d

Daniel

11/24/2021, 7:23 PM
Best don't mix responsibilities. First calculate the individual parts and then create a list from it. When the content of the list is fixed (which seems to be the case, from years to milliseconds) - you don't even need a list. Best create a class which contains the individual parts
👍 3
j

Joffrey

11/24/2021, 7:34 PM
@Daniel I beg to differ here. If all elements are treated the same way (as a collection), there is no reason to enumerate them in many places in code such as in properties of a class. It's best to treat them as a collection to avoid repeating code everywhere. Typically here, you would be repeating the calls to the calculation, the list of properties of the class, the usage of these properties in the UI code, and any other usage of those fields. If later you want to add one new unit (such as nanos), you would also need to update all of those places
👍 1
j

jimn

11/26/2021, 8:48 AM
you should be able to do (without looking at the docs here )
listOf( {duration.days} to {duration-=it.asDays()},  {duration.hours} to {duration-=it.asHours}, )
and so on to reduce a duration and leave the remainders for further reduction
👍 1
3 Views