503dc458e66746c5ac681e7057d209dd

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.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
    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 !

16ca67da61d897cbea7d28a3a19636bd

Matt Freeman

September 28, 2008, September 28, 2008 23:33, permalink

No rating. Login to rate!

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! 

1
2
   public Timer(long millisecondsDuration) : this(millisecondsDuration, false) {}
        public Timer(long millisecondsDuration, Boolean startReady)
503dc458e66746c5ac681e7057d209dd

Matthew McDole

September 29, 2008, September 29, 2008 20:16, permalink

No rating. Login to rate!

Cleaned up a little, removeted set for a couple of the properties where it wasn't needed.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
    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();
        }
    }
37ce1520d319fa8095d05aa475951616

John Sietsma

October 13, 2008, October 13, 2008 03:15, permalink

No rating. Login to rate!

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.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
    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();
    }

Your refactoring





Format Copy from initial code

or Cancel