Skip to main content
Commonmark migration
Source Link

##Bugs

Bugs

##Bugs

Bugs

Source Link
rolfl
  • 97.5k
  • 17
  • 218
  • 418

##Bugs

You have a couple of bugs in your code which will crop up occasionally. The first I noticed is this:

// check if the decimal is 9 or 8 we are gointo round up
location = multiply - float64(seconds)
if location > 0.5 {
    seconds += 1
}

There's two things in there, firstly, the comment is a lie ;-) The code does nothing with 8 or 9 decimals... it rounds "half-up".

The second issue is that the rounding is inaccurate if/when the location is something like 59.8, which your code will round-up to 60. This is a problem, because it should round up to 0, and the minutes should be increased (and, worse, if the minutes is 59, you need to round up the degrees... for example, rounding the value 8°59'59.8" should be 9° 0' 0" seconds - but your code gives 8° 59' 60")

Half-up rounding

There's an easy trick for half-up rounding when dealing with integer-casting in programs, the trick is to add a half-unit to the source value before doing any calculations, and then truncating the result as an integer. For example, to half-up round a float value n to an int, you do int(n + 0.5) You can use this in your code quite nicely.

I would also reverse the logic you have in your toDegrees function... convert everything to seconds (half-up rounded in int) and then extract the values from that. Note that you can express the location in total seconds, total minutes, and total degrees, and then get the "remainder" (modulo) to get just the incremental part:

func ToDegree(location float64) LocationDegree {
    // get the location in seconds, rounded up if needed
    seconds := int(location * 60.0 * 60.0 + 0.5)
    minutes := seconds / 60
    degrees := seconds / (60 * 60)

    return LocationDegree{
        Degree:  degree,
        Minutes: minutes % 60,
        Seconds: seconds % 60,
    }
}

Simplified ToDecimal

Similarly, for the ToDecimal function, I would keep most of the calculation in integer space... Your code is not to bad, but can be improved:

func ToDecimal(locationDegree LocationDegree) float64 {
    var result float64
    result = float64(locationDegree.Seconds) / 60.0

    result += float64(locationDegree.Minutes)

    result = result / 60.0

    result += float64(locationDegree.Degree)

    return result
}

I would have it like:

func ToDecimal(locationDegree LocationDegree) float64 {
    totalSeconds := locationDegree.Degree * 60 * 60
    totalSeconds += locationDegree.Minutes * 60
    totalSeconds += locationDegree.Seconds
    return float64(totalSeconds) / (60.0 * 60.0)
}

General issues

Your code relies on the global values lat and lon. These should be declared in a struct, and your methods should be on that struct:

type Position struct {
    Lat float64
    Lon float64
}

func (p Position) RandomLatLong(seconds int) (float64, float64) {
        location := []float64{p.Lat, p.Lon}
    ....
}

Random adjustments

Your code converts the float64 positions to °'" in order to add an adjustment of minutes and seconds. There is no need to do that. Just have the adjustment in the format of a float64 and apply it directly. For example, for the seconds, have:

randomSeconds := float64(rand.Intn(seconds))

and for the minutes have:

randomMinutes := float64(rand.Intn(minutes)) * 60.0

Now you can add those directly to the value