3

I came across a strange bug and I can't figure out why it occurs. If I invoke my original function, roundToMidnight() is not called and the date is not rounded.

My original function, what doesn't work:

suspend operator fun invoke(reference: Reference) = reference.tagId
    ?.let { tagRepository.getTag(it) }
    ?.uploadDate ?: Date()
    .apply { time += accountRepository.getAccount().first().defaultExpiryPeriod }
    .roundToMidnight()
}

What does work:

suspend operator fun invoke(reference: Reference): Date {
    val date = reference.tagId
        ?.let { tagRepository.getTag(it) }
        ?.uploadDate ?: Date()
        .apply { time += accountRepository.getAccount().first().defaultExpiryPeriod }
    return date.roundToMidnight()
}

roundToMidnight() returns a new instance of Date

fun Date.roundToMidnight(): Date {
    val calendar = Calendar.getInstance()

    calendar.time = this
    calendar[Calendar.HOUR_OF_DAY] = 23
    calendar[Calendar.MINUTE] = 59
    calendar[Calendar.SECOND] = 59
    calendar[Calendar.MILLISECOND] = 0

    return Date(calendar.timeInMillis)
}

What causes the differences in both functions? I'd say they would be exactly the same and I see myself refactoring the bugless function into the original in a month time, because I forgot this happens.

Michiel
  • 767
  • 4
  • 19
  • 3
    `apply` and `roundToMidnight` are called on `Date()`, and only if expression before `?:` returns null – IR42 Jun 07 '20 at 11:06
  • Thank you! Could it be inlined so `apply` and `roundToMidnight` are called on `uploadDate ?: Date()` instead of `Date()`? – Michiel Jun 07 '20 at 11:19
  • 2
    use brackets `(reference?. ... ?.uploadDate ?: Date()).apply { ... }.roundToMidnight()` – IR42 Jun 07 '20 at 11:25
  • I'd highly recommend rewriting this snippet of code without using `let` or `apply`. As you could see for yourself, these functions made your code harder to read and reason about and introduced a subtle bug. Code terseness is not always a good thing. – Egor Jun 07 '20 at 14:38

1 Answers1

0

As suggested by Egor, an expression body is not always the best solution. Rewrote it to this.

suspend operator fun invoke(reference: Reference): Date {
    val tag = reference.tagId?.let { tagRepository.getTag(it)}
    val uploadDateOrNow = tag?.uploadDate ?: Date()

    uploadDateOrNow.time += defaultExpiryPeriod()
    return uploadDateOrNow.roundToMidnight()
}

private suspend fun defaultExpiryPeriod() = accountRepository
    .getAccount().first()
    .defaultExpiryPeriod

Working on a project of my own and boy, do I miss being criticized in PRs ones in a while.

Michiel
  • 767
  • 4
  • 19