views:

230

answers:

2

I'm making a general timer that has functionality to count up from 0 or count down from a certain number. I also want it to allow the user to add and subtract time. Everything is simple to implement except for the case in which the timer is counting down from some number, and the user adds or subtracts time from it.

For example: (m_clock is an instance of SFML's Clock)

float Timer::GetElapsedTime() {
  if ( m_forward ) {
    m_elapsedTime += m_clock.GetElapsedTime() - m_elapsedTime;
  } else {
    m_elapsedTime -= 
      m_elapsedTime - m_startingTime + m_clock.GetElapsedTime();
  }
  return m_elapsedTime;
}

To be a bit more clear, imagine that the timer starts at 100 counting down. After 10 seconds, the above function would look like 100 -= 100 - 100 + 10 which equals 90. If it was called after 20 more seconds it would look like 90 -= 90 - 100 + 30 which equals 70.

This works for normal counting, but if the user calls AddTime() ( just m_elapsedTime += arg ) then the algorithm for backwards counting fails miserably.

I know that I can do this using more members and keeping track of previous times, etc. but I'm wondering whether I'm missing some implementation that is extremely obvious. I'd prefer to keep it as simple as possible in that single operation.

+2  A: 

Your code is unnecessarily complex. The following is equivalent:

float Timer::GetElapsedTime() {
  if ( m_forward ) {
    m_elapsedTime = m_clock.GetElapsedTime();
  } else {
    m_elapsedTime = m_startingTime - m_clock.GetElapsedTime();
  }
  return m_elapsedTime;
}

and hopefully illustrates why AddTime() doesn't work: m_elapsedTime is being replaced on every call to GetElapsedTime(). The simplest solution is track added/subtracted time separately and rework GetElapsedTime() thus:

float Timer::GetElapsedTime() {
  float elapsedTime = m_forward
                      ? m_clock.GetElapsedTime()
                      : m_startingTime - m_clock.GetElapsedTime();
  return elapsedTime + m_addedTime;
}
Marcelo Cantos
Gah. same answer as me :-(
Yuliy
Well my goal was trying to not have to use another member for the functionality. If that was the case, then your first block wouldn't work because adding time directly to m_elapsedTime would require that function to use += and -=. However, if creating a separate member is the only option, then that is what I will go with.
Person
It's not an additional member. You're getting rid of m_elapsedTime, and adding in m_addedTime.
Yuliy
A: 

If you want to increase the time remaining, you can simulate that by decreasing the amount of time elapsed.

Your arithmetic expressions are more complex than they need to be: m_elapsedTime += m_clock.GetElapsedTime() - m_elapsedTime is equivalent to m_elapsedTime = m_clock.GetElapsedTime(), and m_elapsedTime -= m_elapsedTime - m_startingTime + m_clock.GetElapsedTime() is equivalent to m_elapsedTime = m_startingTime - m_clock.GetElapsedTime()`.

At this point, the problem is clear: the old value of m_elapsedTime never affects the subsequent result. I would consider adding in an offset field to handle any changes to the starting value of the timer. At this point, Timer:GetElapsedTime could just be the following:

float Timer::GetElapsedTime() {
  if ( m_forward ) {
    return offset + m_clock.GetElapsedTime();
  } else {
    return offset - m_clock.GetElapsedTime();
  }
}

where offset starts at 0 for a count-up and the start value for a count-down. Make sure and watch your signs for updating offset in AddTime()

Yuliy
Same as I said above, if you use AddTime directly on m_elapsedTime then you are required to use += and -= for the GetElapsedTime function. However, implementing a separate member seems the way to go.
Person