public class Timer
{
private Boolean _IsReady;
public Boolean IsReady
{
get
{
if (_IsReady)
{
return true;
}
if (sw.ElapsedMilliseconds >= Duration)
{
IsReady = true;
return true;
}
return false;
}
set
{
if (value)
{
sw.Stop();
}
_IsReady = value;
}
}
public long TicksLeft
{
get
{
return (IsReady) ? 0 : Duration - sw.ElapsedMilliseconds;
}
}
public long Duration { get; set; }
private Stopwatch sw = new Stopwatch();
public Timer(long MillisecondsDuration) : this(MillisecondsDuration, false) {}
public Timer(long MillisecondsDuration, Boolean StartReady)
{
Duration = MillisecondsDuration;
if (StartReady)
{
IsReady = true;
}
else
{
IsReady = false;
sw.Start();
}
}
public void Reset()
{
sw.Reset();
sw.Start();
}
public void ForceReady()
{
IsReady = true;
}
}
Refactorings
No refactoring yet !
Matt Freeman
September 28, 2008, September 28, 2008 23:33, permalink
On trying to read the code, I was confused about where StartReady can from for a good few seconds. It's a style issue but I think you should follow conventions on this one have your constructor parameter start with lowercase. Don't make me think!
public Timer(long millisecondsDuration) : this(millisecondsDuration, false) {}
public Timer(long millisecondsDuration, Boolean startReady)
Matthew McDole
September 29, 2008, September 29, 2008 20:16, permalink
Cleaned up a little, removeted set for a couple of the properties where it wasn't needed.
public class Timer
{
public long Duration { get; set; }
private readonly Stopwatch _sw = new Stopwatch();
public long TicksLeft
{
get
{
return (IsReady) ? 0 : Duration - _sw.ElapsedMilliseconds;
}
}
private Boolean _isReady;
public Boolean IsReady
{
get
{
if (_isReady) return true;
if (_sw.ElapsedMilliseconds >= Duration)
{
_isReady = true;
_sw.Stop();
}
return _isReady;
}
}
public Timer(long millisecondsDuration) : this(millisecondsDuration, false) { }
public Timer(long millisecondsDuration, Boolean startReady)
{
Duration = millisecondsDuration;
if (startReady)
{
_isReady = true;
}
else
{
_isReady = false;
_sw.Start();
}
}
public void Reset()
{
_sw.Reset();
_sw.Start();
}
public void ForceReady()
{
_isReady = true;
_sw.Stop();
}
}
John Sietsma
October 13, 2008, October 13, 2008 03:15, permalink
It seems the class is designed around the _isReady flag. You have to a lot of effort to keep it current and in fact there is a bug cause you missed a spot.
Timer t = new Timer( 10000, false );
Console.WriteLine( t.IsReady );
t.Reset();
Console.WriteLine( t.IsReady );
prints false twice, even though it should be ready.
So, here is my "readyless" design. I also renamed some members.
btw, I've rearranged the members. I like public, protected, private so the interface is first up when you're reading the class.
public class Timer
{
public Timer( long millisecondsDuration ) : this( millisecondsDuration, false )
{
}
public Timer( long millisecondsDuration, bool start )
{
Duration = millisecondsDuration;
if( start )
_sw.Start();
}
public long Duration
{
get { return _duration; }
private set { _duration = value; }
}
public long TicksLeft
{
get { return _sw.IsRunning ? Math.Max( 0, Duration - _sw.ElapsedMilliseconds ) : 0; }
}
public Boolean IsRunning
{
get { return _sw.IsRunning && TicksLeft != 0; }
}
public void Start()
{
_sw.Reset();
_sw.Start();
}
private long _duration;
private readonly Stopwatch _sw = new Stopwatch();
}
Hosam Aly
January 7, 2009, January 07, 2009 11:12, permalink
John: IsRunning could be implemented as "return TicksLeft != 0", as the condition "_sw.IsRunning" is already performed in "TicksLeft". You could also make Duration an auto-property if you'd like to (saving a few lines of code).
This is a quick timer class I whipped up.. wanted to see if people had suggestions to refactor it.
Timer should have:
Timer(millisecondsduration, bool if it should start ready or begin timer right away)
Timer.IsReady - Return True if duration is up or was started ready through constructor
Timer.TicksLeft - Milliseconds until timer reaches duration
Timer.Reset() - Start timer over again
Timer.ForceReady() - Force timer to stop and set it to ready.