Delegates and Events - Trouble

[Snowbleach]

Hello, I am having trouble with the following console application. I
am using delegates/event to make a clock raise an alarm when a certain
amount of time has been reached. The following compiles, but code does
nothing else then to produce an endless loop. Any help would be
greatly appreciated, I'm probably overlooking something silly.

Regards,

Arjen.

The code:

using System;
using System.Threading;

namespace ArjenCsharp
{
public class CountdownClock //doesn't actually count down
{
private int hour;
private int minute;
private int second;

public int Hour
{
get { return hour; }
set { hour = value; }
}

public int Minute
{
get { return minute; }
set { minute = value; }
}

public int Second
{
get { return second; }
set { second = value; }
}

public bool Equals(CountdownClock theClock)
{
if ((this.hour == theClock.hour) && (this.minute ==
theClock.minute) && (this.second == theClock.second))
return true;
else
return false;
}

public override string ToString()
{
return this.Hour.ToString() + ":" + this.Minute.ToString()
+ ":" + this.Second.ToString();
}

public CountdownClock(int theHour, int theMinute, int
theSecond)
{
hour = theHour;
minute = theMinute;
second = theSecond;
}
//default constructor - takes no action
public CountdownClock()
{}

//the delegate
public delegate void AlarmEventHandler(CountdownClock o,
EventArgs e);
//public instance of the delagate
public event AlarmEventHandler myAlarmDelegate;

public void Run()
{
DateTime dt = DateTime.Now;
CountdownClock targetTime = new CountdownClock(dt.Hour,
dt.Minute+2, dt.Second);

for (;
{
Thread.Sleep(100);
DateTime updateTime = DateTime.Now;
if (this.Equals(targetTime))
{
myAlarmDelegate(targetTime, EventArgs.Empty);
}

this.Hour = updateTime.Hour;
this.Minute = updateTime.Minute;
this.Second = updateTime.Second;
}
}
}

public class Alarm
{
public void Subscribe(CountdownClock theClock)
{
theClock.myAlarmDelegate += new
CountdownClock.AlarmEventHandler(RaiseAlarm);
}

public void RaiseAlarm(CountdownClock cc, EventArgs e)
{
Console.WriteLine("Alarm raised at {0}", cc.ToString());
}
}

public class Tester
{
static void Main()
{
Tester t = new Tester();
t.Start();
}

public void Start()
{
Alarm beep = new Alarm();
CountdownClock cntDownClock = new CountdownClock();
beep.Subscribe(cntDownClock);
cntDownClock.Run();
}
}
}

 

[Morten Wennevik [C# MVP]]

Hello, I am having trouble with the following console application. I
am using delegates/event to make a clock raise an alarm when a certain
amount of time has been reached. The following compiles, but code does
nothing else then to produce an endless loop. Any help would be
greatly appreciated, I'm probably overlooking something silly.

Well, the code compiles for me and does indeed output lines to the console while the target time is effective. However, after this has happened you never exit the loop causing it to run forever, or at least for a full 24 hours before outputting new lines, and then 24 hours ...

Try adding break; after firing the event.

myAlarmDelegate(targetTime, EventArgs.Empty);
break;

Hint: when string concatenating, if any of the components being concatenated are string literals, ToString() will automatically be called on all the other components automatically. No need to do it yourself, other than perhaps readability.

 

[Ben Voigt]

Well, the code compiles for me and does indeed output lines to the console
while the target time is effective. However, after this has happened you
never exit the loop causing it to run forever, or at least for a full 24
hours before outputting new lines, and then 24 hours ...

And additionally, don't check for equality to the alarm time, check for
being past the alarm time (then advance your alarm time so you don't fire
continuously). It's possibile for your app to sleep for the one-second
interval where the times would actually compare equal.

Try adding break; after firing the event.

myAlarmDelegate(targetTime, EventArgs.Empty);
break;

Hint: when string concatenating, if any of the components being
concatenated are string literals, ToString() will automatically be called
on all the other components automatically. No need to do it yourself,
other than perhaps readability.

Actually, putting explicit ToString() saves boxing overhead for value types,
and of course if you want format strings you need to explicitly call
ToString.

 

[Arjen Logghe]

Thank you both for your help. It helped a lot. I wasn't aware my code
at the time of posting did in fact work. I saw a black screen and
figured my code was broken, but since I never displayed the time in
the cntDownClock.Run() I had no idea what the code was doing. Plus,
thinking my code was broken I terminated the application before the
alarm time could be reached, heh.. Well, another valuable lesson
learned!

Anyhow, here's the new modified Run() method, implementing your
advice:

public void Run()
{
DateTime dt = DateTime.Now;
CountdownClock targetTime = new CountdownClock(dt.Hour,
dt.Minute+1, dt.Second); //targettime = 1 minute

for (;
{
Thread.Sleep(100);
DateTime updateTime = DateTime.Now;

if (updateTime.Minute >= targetTime.Minute)
{
myAlarmDelegate(targetTime, EventArgs.Empty);
targetTime.Minute += 1;
break;
}
//display the time each second
if (updateTime.Second != second)
{
Console.WriteLine("{0}:{1}:{2}",
this.Hour.ToString(), this.Minute.ToString(), this.Second.ToString());
}

this.Hour = updateTime.Hour;
this.Minute = updateTime.Minute;
this.Second = updateTime.Second;
}
}

 

[Arjen Logghe]

Sorry for double post but the above method is bugged. It only checks
if there's a difference in updateTime and targetTime's minute.
Ofcourse, it should also check for seconds. Also, the clock object
passed to the delegate instance should not be targetTime but *this*
(cntDownClock). I think it's working as intended now .

public void Run()
{
DateTime dt = DateTime.Now;
CountdownClock targetTime = new CountdownClock(dt.Hour,
dt.Minute+1, dt.Second);

for (;
{
Thread.Sleep(100);
DateTime updateTime = DateTime.Now;

if ((updateTime.Minute >= targetTime.Minute) &&
(updateTime.Second >= targetTime.Second))
{
myAlarmDelegate(this, EventArgs.Empty);
targetTime.Minute += 1; //not really necessary
since loop breaks after this statement but could be used in continuous
loop
break;
}
//display the time each second
if (updateTime.Second != second)
{
Console.WriteLine("{0}:{1}:{2}",
this.Hour.ToString(), this.Minute.ToString(), this.Second.ToString());
}

this.Hour = updateTime.Hour;
this.Minute = updateTime.Minute;
this.Second = updateTime.Second;
}
}

 

[Ben Voigt]

Sorry for double post but the above method is bugged. It only checks
if there's a difference in updateTime and targetTime's minute.
Ofcourse, it should also check for seconds. Also, the clock object
passed to the delegate instance should not be targetTime but *this*
(cntDownClock). I think it's working as intended now .

public void Run()
{
DateTime dt = DateTime.Now;
CountdownClock targetTime = new CountdownClock(dt.Hour,
dt.Minute+1, dt.Second);

for (;
{
Thread.Sleep(100);
DateTime updateTime = DateTime.Now;

if ((updateTime.Minute >= targetTime.Minute) &&
(updateTime.Second >= targetTime.Second))

should be (updateTime.Minute > targetTime.Minute) || (updateTime.Minute ==
targetTime.Minute && updateTime.Second >= targetTime.Second)

but, why not just (updateTime >= targetTime)

?

 

[Arjen Logghe]

but, why not just (updateTime >= targetTime)

?

Doesn't that require me to overload operator >=, because the
CountDownClock class doesn't know how to compare two CountDownClock
objects?

 

[Morten Wennevik [C# MVP]]

Doesn't that require me to overload operator >=, because the
CountDownClock class doesn't know how to compare two CountDownClock
objects?

It does, but overloading operators isn't all that hard. The code below is untested, but should work

public static bool operator >=(CountdownClock c1, CountdownClock c2)
{
if (c1.Hour < c2.Hour)
return false;
if (c1.Minute < c2.Minute)
return false;
if (c1.Second < c2.Second)
return false;

return true;
}

You need to define the opposite operator as well.

public static bool operator <=(CountdownClock c1, CountdownClock c2)
{
if (c1.Hour > c2.Hour)
return false;
if (c1.Minute > c2.Minute)
return false;
if (c1.Second > c2.Second)
return false;

return true;
}

 

[Arjen Logghe]

Thank you, Morten. Sorry if my previous post implied that I don't know
how to overload operators, because I do . Anyhow, the overloaded
operators allow for a lot better readability. Afterall

(updateTime >= targetTime)

looks a lot better then

(updateTime.Hour >= targetTime.Hour && updateTime.Minute >=
targetTime.Minute && updateTime.Second >= targetTime.Second)

Regards,

Arjen.

 

[Ben Voigt [C++ MVP]]

Just took another look at this thread, and the logic provided for operator

= is totally wrong, because

(02:00:00) >= (01:15:00) returns false

Doesn't that require me to overload operator >=, because the
CountDownClock class doesn't know how to compare two CountDownClock
objects?

It does, but overloading operators isn't all that hard. The code below is
untested, but should work

public static bool operator >=(CountdownClock c1, CountdownClock
c2)
{
if (c1.Hour < c2.Hour)
return false;
if (c1.Minute < c2.Minute)
return false;
if (c1.Second < c2.Second)
return false;

return true;
}

You need to define the opposite operator as well.

public static bool operator <=(CountdownClock c1, CountdownClock
c2)
{
if (c1.Hour > c2.Hour)
return false;
if (c1.Minute > c2.Minute)
return false;
if (c1.Second > c2.Second)
return false;

return true;
}

 

[Ben Voigt [C++ MVP]]

Doesn't that require me to overload operator >=, because the
CountDownClock class doesn't know how to compare two CountDownClock
objects?

Yes, but DateTime already defines operator>=, so just use that.

 

[Arjen Logghe [void]]

Just took another look at this thread, and the logic provided for operator

(02:00:00) >= (01:15:00) returns false

Indeed, you're totally right. Great catch! Seems skimming over old
threads pays off