3
\$\begingroup\$

I'm struggling with some code that should be a very simple computation of a duration. The purpose of the code is to generate a random delay, constrained by an upper and lower limit. The reason I'm looking for advise here is that what should be a simple task, has turned into a behemoth of type conversions:

package main

import (
        "math/rand"
        "time"
)

func main() {
        // parameters, hardcoded for illustration purposes only
        tMin := float32(1.2)
        tMax := float32(2.7)

        // intention is approximately this:
        // delay := tMin + rand.Float32() * (tMax - tMin)

        delay := time.Duration((tMin + rand.Float32() * (tMax - tMin)) * float32(time.Second))

        time.Sleep(delay)
}

Questions:

  • Apart from rounding errors (which I'm happy to ignore here), does this code have any actual bugs that I missed?
  • I know that elegance is in the eye of the beholder, but is there any way to write this more elegantly? In particular, I'd like to reduce the number of required casts, preferably down to zero.
\$\endgroup\$
3
\$\begingroup\$

Encapsulate implementation details and any ugliness in a function. Readability is paramount.

For example, a more elegant solution,

package main

import (
    "math/rand"
    "time"
)

// delay returns a random delay, in nanoseconds,
// between min and max seconds.
func delay(min, max float64) time.Duration {
    // time doesn't go backwards
    if min < 0 || min > max {
        return 0
    }
    // delay in seconds
    d := min + rand.Float64()*(max-min)
    // delay in nanoseconds
    return time.Duration(d * float64(time.Second))
}

func main() {
    dMin, dMax := 1.2, 2.7

    time.Sleep(delay(dMin, dMax))
}

By design, Go is type safe. Conversions between diffent types must be explicit.

Unless you have a very large number of occurrences of a floating-point variable and you can tolerate the loss of precision, use type float64, not float32.


behemoth : something of monstrous size. Merriam-Webster

a behemoth of type conversions. uli

I only found a few type conversions in the code you posted.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you for the review. I agree that putting things into functions can be used to hide some ugliness from obscuring the view on the relevant code. Concerning my choice to name this a behemoth, it's the perceived relation between code that does something and code that's only necessary due to the circumstances. The latter includes two conversions and the time.Second constant. \$\endgroup\$ – uli Mar 3 at 17:44
1
\$\begingroup\$

Toying with this, I stumbled across another alternative. According to my own initial thought, it would have been cheating, but I'd present it here because I think that it's a compromise that should at least be considered. The idea is to simply use a different base duration, in this case milliseconds instead of seconds:

tMinMS := 1200
tMaxMS := 2700

delayMS := tMin + rand.Intn(1 + tMax - tMin)

time.Sleep(time.Duration(delay) * time.Millisecond)

Notes here:

  • This works with just one conversion and the time.Milliseconds constant, which is better. The trick is that the conversion, even though it discards fractions, only causes a rounding error of less than 1ms. Doing the same to the original code would impose rounding errors of 1000 times as much.
  • Everyone used to calculating with units will notice and cringe from the fact that this multiplies a time.Duration with a time.Duration. Which is why I still don't like this solution.
  • I've been looking at replicating the time module using float64 representing seconds as alternative. I should check if anyone did that already before reinventing the wheel...
| improve this answer | |
\$\endgroup\$

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.