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
    
Just be glad it was so easy to spot that you do not want to work for this company. Sometimes you figure that out too late. –  nwp Sep 5 at 19:23

5 Answers 5

up vote 13 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
7  
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
    
Academics should know this! I'm sure I learned this in my first C course at university. It wasn't a C course even, it was called "Structured and Object Oriented programming" as first programming course in the first semester of the first year, and C was used/introduced to explain the structured programming concepts. –  Emile Vrijdags Sep 5 at 19:20

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) –  naiad 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

Everything about this question indicates that the teacher is not very good at his job.

  • bad language/grammar: the first 2 answers he refers to the program as it, in the 3rd answer the program is called you. To be more consistent and have better style it should have been at least: "You don't know what state it is changing to."
  • in the first answer the "constant floating point number" is confusing and should be "float literal", which is more precise and is much more commonly used to describe.. a literal :)
  • in the example a variable is called "m_State" where the convention is that the "m_" prefix indicates that the variable is a member variable. But then what is totalTime, it is not defined in the function, so it most probably is also a member variable and apparently not following the convention. Maybe its a global, but when the "m_" prefix is used, the "g_" prefix is also usually used to indicate a global variable. It could be a variable which is defined in that translation unit only (for example in the .cpp file where this function is defined), but that is just bad style. So yeah, whats up with this variable..
  • every answer is correct:
    • a) What does the literal 3.0f represent, it is indeed bad style to write code like that. The reader of the code has no idea why 3.0 is a 'boundary condition'. However it doesn't break the code.
    • b) When comparing floats for equality, a better way to do that is: bool floats_are_equal(float a, float b) { return std::abs(a - b) < std::numeric_limits<float>::epsilon() } And even this is frowned upon in some circles, they use a reinterpret_cast<T>() to convert the bit-representation the float to an (unsigned) int (with the same bit-representation in memory) and then you can do an exact comparison. Also this is clearly the thing that is most wrong with this piece of code, because this most likely will cause the code to just not do what it is intended to do. All the other answers more have to do with style.
    • c) You don't know which state the program is switching to, that is correct. It tells you it is probably the next state, this could be enough depending on the context. I would rate this answer as least correct ;)
    • d) a class named Class is indeed very poorly named, obviously. And I think this is worse than using a literal. Though it is about on the same level.

In conclusion, this question does not convey any useful information about the person taking the test. In general multiple choice questions are not a good way to test if someone understands something. It is ok if you have to memorize facts. But in this case a free response would have been sooo much better.

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.