Float Overflow.go comments

Issues & Feature Request
1

I just want to post a quick comment about Float Overflow.go, and I recommend that it is updated:

  1. The tests should be updated to use a tolerance greater than 0. Currently, the tolerance is zero. This means that any small difference in logic will cause the test to fail. For example, result = value * 1.1 will return a very slightly different value than result = value + (value * 0.1).
  2. The test cases will only pass if math/big.Float is used due to approximation that is used even if float64 values are used in the estimateHeight function.
  3. In the usability test, the test input is 1000000.1, and the output value is expected to be 1100000.11. This is not representable by a float32 value, so I had to change the signature of the estimateHeight function to return a float64 value. Since the tests aren’t under the developer’s control, this can cause issues depending upon how the tests are written. It works OK in this case, but it should be made clear or the function signature shouldn’t be changed, especially since this is a “beginner” lab.

Based on the situation in the lab, my opinion is that it should be sufficient for a developer to:

  1. Apply an empty check on the input string.
  2. Change all calculations inside estimateHeight to use float64.
  3. Do calculations.
  4. Check the bounds to make sure the value can be converted to a float32.
  5. Convert to float32 and return a float32 (in order to avoid changing the interface)

Thoughts?

1 Like
2

Hi @lkm35t firstly, thanks for your detailed feedback. It makes sense. Let me give you some context:

  1. We introduce this challenge few years ago and surprised how many users never heard about float overflow. Originally we thought it would be a beginner challenge. We are upgrading the points for this challenge to be medium level. Thanks for letting us know in Go it is still showing as beginner.
  2. Calculation should not be done using float64. The requirement for math.bigFloat is intentional. In fact, we want to teach developers the danger of upcasing (say to float64) to address overflow issues. Upcasing only shift the problem to a higher number and doesn’t address the root cause. If you are interested, see my videos and articles about this (See Remediation section in Numeric overflow lab)
  3. To keep this easier and follow your suggestion, we have updated the function interface to float64 and test remained as they were.

Quick note issue with parsing. I know the suggestion solution showed the following construct. However, there is issue with the way value is parsed. it doesn’t cater for IEE747 53.10 precision and hence the test for 0 value is parsed to 0xc0000a8ee8 . Therefore, it fails the negative check.

bigFloat, _ := new(big.Float).SetString(height)

if bigFloat.Cmp(big.NewFloat(1)) == -1 {
		ArithmeticError := errors.New("values <= 0 not allowed")
		return 0.0, ArithmeticError
}

The suggested solution (in hint) is updated to a different solution that shows a correct parsing and passes the zero amount check. If you want to see the new solution, start the challenge and open “view solution hint”.

Please let me know if you have any further thoughts on this. Good discussion.