Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Ok so in a programming test I was given the following question.

Question 1 (1 mark)

Spot the potential bug in this section of code:

void Class::Update( float dt )
{
    totalTime += dt;
    if( totalTime == 3.0f )
    {
        // Do state change
        m_State++;
    }
}

The multiple choice answers for this question were.

a) It has a constant floating point number where it should have a named constant variable

b) It may not change state with only an equality test

c) You don't know what state you are changing to

d) The class is named poorly

I wrongly answered this with answer C.

I eventually received feedback on the answers and the feedback for this question was

Correct answer is a. This is about understanding correct boundary conditions for tests. The other answers are arguably valid points, but do not indicate a potential bug in the code.

My question here is, what does this have to do with boundary conditions? My understanding of boundary conditions is checking that a value is within a certain range, which isn't the case here. Upon looking over the question, in my opinion, B should be the correct answer when considering the accuracy issues of using floating point values.

share|improve this question
    
Maybe this is a stupid point, but if you are going to provide a code example for a test question, why would a variable without it's declaration be thrown in there? Should be a COMPLETE example to be answered. Test writer needs to be more specific and less presumptuous. –  Ben DeMott Aug 29 '12 at 6:33

4 Answers 4

up vote 8 down vote accepted

I think the test question is ambiguously vague.

The problem, of course, is that you're comparing a floating-point number with something (the type of what is being compared is not specified), whereas comparing within a given range (an epsilon) is more appropriate, because comparing floating point numbers with == is unreliable.

Whether the floating point number is being compared to a "named constant" or not is an irrelevant detail.

Another choice example of how test questions created by academics without field experience generally suck.

share|improve this answer
    
As far as I know this test was created by a programmer at a games company. –  ctor Aug 28 '12 at 22:48
4  
As a games programmer, he has a certain point of view. The method is a pretty leaky abstraction anyway, but the more I look at it, the more I see that the == sign is the real problem; it should be >=, unless the dev has something very specific in mind like object collision (in which case you still need an epsilon). Focusing on things like "named constant variables" is taking one's eye off the ball. –  Robert Harvey Aug 28 '12 at 22:51

I think the real bug in the code resides in the fact that there's a possibility that the code adds the input together and expects it to reach a certain value. This may never happen.

Consider this case:

Update(0.1f);
Update(1.1f);
Update(0.2f);
Update(1.8f);

After the last call to Update, the internal TotalTime variable has the value of (0.1+1.1+0.2+1.8) 3.2f (disregarding possible floating-point errors). As such, the test for TotalTime == 3.0f will fail from that point onwards, nothing will ever fix that problem during program's lifetime (except potential variable wrap-around).

This problem has nothing to do with the input being floating-point (although it too is a valid problem in this case), however. A similar problem may arise even if the input was integers. Consider the case where Update() takes the dt variable as milliseconds and as such it's integer. What if the comparison then was something like if(TotalTime == 3000) { (to test for a possibility that TotalTime has cumulated to 3000 millisecond). What if the function input never accumulates to exactly 3000? What if it is exactly 2999 at some point, and then the function is called with the value of 2? The TotalTime variable grows from 2999 to 3001, and as such never reaches 3000, and as such the test fails.

To fix this problem the test should use the >= operator instead which asserts that the "TotalTime is 3.0f or more". In such case no bug would occur regardless of the input to the function.

As such, I don't believe given answers are sufficient.

share|improve this answer
    
(or negative numbers :P) –  sparkleshy Aug 29 '12 at 5:16

The right answer should be b. All other points are somewhat valid1 as well, but comparing floating point values for equality is something that should be done with a lot of care.

For example, adding ten 0.2s and ten 0.1s makes a 3 that is not equal to 3.0 (link to ideone).

Using "magic numbers" and using non-descriptive class names are wrong things too, but they are not errors. Comparing a floating-point constant for equality, on the other hand, is nearly certainly an error.


1 Except c: it's a classic "nonsensical" answer.

share|improve this answer
1  
Yes, b is the answer I would have chosen. –  Robert Harvey Aug 28 '12 at 22:11
    
I've never really quite understood why this happens. I know the basics of floating point operations. Would I be right in saying it's to do with the actual binary string of the value? If I converted both c from your example and 3.0f into their equivalent binary string that they would return different binary strings while still representing a floating point value of 3.0f? –  ctor Aug 29 '12 at 0:08
    
@Loggie The difference between the 3.0 produced as a result of additions and the 3.0 constant is a tiny difference attributable to a representation error. The problem is that 0.1 cannot be represented exactly as a sum of negative powers of 2, so there is a tiny error. When you add fractions with errors, the errors add up. When the number with errors is printed, however, the error part is beyond the default number of significant digits the cout prints, so the number is rounded to 3.0. Its binary representation, however, is 3.00000000000000044408921, so the == comparison fails. –  dasblinkenlight Aug 29 '12 at 0:36

3.0 is the boundary. that increments m_State when it its reached.

The Point is that 3.0f is hard coded. The Author of the question thinks it should have been a named constant variable.

My question here is, what does this have to do with boundary conditions? My understanding of boundary conditions is checking that a value is within a certain range, which isn't the case here

So yes, that is the case here.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.