Looking to develope skills

[Arjay]

In addition to what memeloo said, you also only want to have one DateTime object (not 3).

With regard to the overuse of ToString() that I've mentioned in the past. Consider revisiing, this:

//'Set Alarm' Button
private void btn_alarm_Click(object sender, EventArgs e)
{
  int hr;
  int min;
  int sec;
  hr = int.Parse(num_hour.Value.ToString());
  min = int.Parse(num_min.Value.ToString());
  sec = int.Parse(num_sec.Value.ToString());
  SetAlarm(hr, min, sec);
}

to this

 
//'Set Alarm' Button
private void btn_alarm_Click(object sender, EventArgs e)
{
  SetAlarm( Convert.ToInt32( num_hour.Value )
    , Convert.ToInt32( num_min.Value )
    , Convert.ToInt32( num_sec.Value ) );
}

In this case, you don't really need the local hr, min, sec integer variables. Also, convert the value to a string only to parse it to an integer doesn't make much sense.

Lastly, when you post code, please use code tags (i.e [ CODE]your code here[/ CODE] minus the spaces).

[Dragster93]

if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute && DateTime.Now.Second == dt.Second)
            {
                spWave.Play();
            }

I wish for this to be checked and verified at very second of the day. Could somebody please tell me what I should write in the 'for' loop???

[Arjay]

I would create a helper method to do this for you. The helper method checks if the current interval has been met. If it has, it increments the next time interval and returns true; if not, it returns false.

 
///<summary>
/// Checks if the current time has passed the next sync time
/// If so, increments the next sync time and returns true.
///</summary>
///<param name="dtNow">Current time</param>
///<returns>True if passed next sync time; false otherwise</returns>
privatebool CheckAndIncrementNextTimeSync ( DateTime dtNow )
{
  bool syncTime = false;

 
  // Check if the current time has passed the next time
  // sync time. 
  if ( _nextTimeSync < dtNow )
  {
    _nextTimeSync = dtNow.AddMinutes( _timeSyncInterval );
    syncTime = true;
  }
  return syncTime;
}
 

You'll want to create a similar method and call it in a while loop and perform the action if the method returns true. For my needs, I call it as follows:

 
///<summary>
/// Sync the hand reader time with the PC service machine time
///</summary>
///<param name="rsiHandReader"></param>
privatevoid SyncTime( CRsiHandReader rsiHandReader )
{
  DateTime dtNow = DateTime.Now;
 
  if ( CheckAndIncrementNextTimeSync( dtNow ) )
  {
    RSI_TIME_DATE rsiTimeDate = new RSI_TIME_DATE( );
 
    rsiTimeDate.year = Convert.ToByte( dtNow.Year - 2000 );
    rsiTimeDate.month = Convert.ToByte( dtNow.Month );
    rsiTimeDate.day = Convert.ToByte( dtNow.Day );
    rsiTimeDate.hour = Convert.ToByte( dtNow.Hour );
    rsiTimeDate.minute = Convert.ToByte( dtNow.Minute );
    rsiTimeDate.second = Convert.ToByte( dtNow.Second );
    
    rsiHandReader.CmdPutTime( rsiTimeDate );
 
    Debug.WriteLine( String.Format( "SyncTime: {0}", dtNow.ToString( ) ) );
  }
}

[Dragster93]

Could you please explain these two methods to me??? Because, I can't seem to understand either of them, though you have given the explaination in comments.
Thanks!!

[Arjay]

From what I understand you have an alarm clock that want to play a wave file when the clock goes off. You do this by setting the alarm time to some point in the future and then check it every second.

The code I posted was from a Windows Service that sent and retrieved data to a piece of hardware (i.e. Biometric hand scanner).

In my example, the _nextTimeSync is a DateTime class field and represented the time that I should sync the time in the biometric hand scanner (i.e on my system, the biometric clock time was sync'd with the PC/Network time).

For me, I just wanted to set the next time sync on a periodic basis, so when the windows service first started, I set the next time sync to some time in the future by using DateTime.AddMinutes( ) method (this code isn't shown).

Then I simply checked the _nextTimeSync time with the current time. To help with this, I created the CheckAndIncrementNextTimeSync method which checks to see if the current time is >= the _nextTimeSync time. If it is, it increments the _nextTimeSync time based on the sync interval and returns true.

The CheckAndIncrementNextTimeSync method gets called in a loop at a fixed interval. For me, it's every 2 minutes. This is done within the SyncTime method which calls the CheckAndIncrementNextTimeSync method, if that method returns true, it sends the current pc time to the hardware.

In my opinion, the code you need is close (depending whether you want to have the alarm clock go off on a fixed interval). I would rename the _nextTimeSync field to something like _nextTimeAlarm and initially set this to the time when you want the alarm to go off.

[Dragster93]

Thank you very much for explaining the method to me!!! =)

I shall now show you my code and explain to you how it works. Kindly tell me whether I'm going right so far.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Media;

namespace WindowsFormsApplication1
{
    public partial class Form1 : Form
    {
        
        public Form1()
        {
            InitializeComponent();
        }

        //The funtion that handles the calculations of the alarm
        public static void SetAlarm(int hours, int minutes, int seconds)
        {
            string wav_loc = "c:\\testjazz.wav";
            SoundPlayer spWave;
            spWave = new SoundPlayer(wav_loc);

            int year, month, day;

            year = int.Parse(DateTime.Now.Year.ToString());
            month = int.Parse(DateTime.Now.Month.ToString());
            day = int.Parse(DateTime.Now.Day.ToString());

            DateTime dt = new DateTime(year, month, day, hours, minutes, seconds);

            if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute && DateTime.Now.Second == dt.Second)
            {
                spWave.Play();
            }
            return;
        }
        
        //'Set Alarm' Button
        private void _Click(object sender, EventArgs e)
        {
            SetAlarm(Convert.ToInt32(dt_time.Value.Hour), Convert.ToInt32(dt_time.Value.Minute), Convert.ToInt32(dt_time.Value.Second));
        }
    }
}

On the form, when the 'btn_alarm' button is clicked, it called the 'btn_alarm_Click' method. In that method, another method is called along with some passed parameters. This is the 'SetAlarm' method. This is where the entire calculation takes place.

And I think this:

if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute && DateTime.Now.Second == dt.Second)
            {
                spWave.Play();
            }

does the work of your 'CheckandIncrementNextTimeSync' method. If not, please let me know.

Also, I wish to know, how exactly is it that I can have this 'SetAlarm' method called over and over again. I tried using an infinite loop but then the program just hangs. I understood why that was happening. It was because the loop just kept going on and on without any results! But anyways, please try and find a solution to my problem. I'm also working on different ways. Will inform you as soon as I'm done. =)

[Dragster93]

Dude, if you don't mind, could you help me out with another problem also?

public static bool SetAlarm(int hours, int minutes, int seconds)
        {
            string wav_loc = "c:\\testjazz.wav";
            SoundPlayer spWave;
            spWave = new SoundPlayer(wav_loc);

            int year, month, day;

            year = int.Parse(DateTime.Now.Year.ToString());
            month = int.Parse(DateTime.Now.Month.ToString());
            day = int.Parse(DateTime.Now.Day.ToString());

            DateTime dt = new DateTime(year, month, day, hours, minutes, seconds);

            if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute && DateTime.Now.Second == dt.Second)
            {
                spWave.Play();
                return true;
            }
        }

I wish for this function to only return true or false (not anything else) but I can't seem to do that. Could you help me out?

[memeloo]

I wish for this function to only return true or false (not anything else)

LoL there is nothing else to return... or do you mean: true/false/maybe ? you really should read some book.

[Dragster93]

LoL there is nothing else to return... or do you mean: true/false/maybe ? you really should read some book.

Then why is it asking me to return so value for the parameters I've passed???
The exact message was, "'WindowsFormsApplication1.Form1.SetAlarm(int, int, int)': not all code paths return a value".....

[memeloo]

because you return only true inside your IF and by default a function returns false. to get rid of this warning write before the method's closing brace "return false".

[Dragster93]

Actually, the whole point of the function is to return true (in this case). The thing is that I can't check in the 'if', whether this function has returned true or not. And the 'if' is initiated inside another function which is the 'void_Click' function (if you look at my code on top).
So....
1) How can I make the function return either true/false without having a problem with either of the parameters of the function???

2) How do I find out, in the 'void_Click' function, whether the 'SetAlarm' returned true???

[memeloo]

2) How do I find out, in the 'void_Click' function, whether the 'SetAlarm' returned true???

I'm sorry, but you have no idea what you're doing how do you expect a method that returns nothing (void) to return true/false? ... I give up .

[Dragster93]

memeloo!!!! THANK YOU!!!!! =)

You were right!!!! =)
I did what you said....and the program works!!!!! =)

Ok, now I'll explain to you how the entire program works by posting the slightly changed code that I wrote!

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Media;

namespace WindowsFormsApplication1
{
    public partial class Form1 : Form
    {
        
        public Form1()
        {
            InitializeComponent();
        }

        //The funtion that handles the calculations of the alarm
        public static bool SetAlarm(int hours, int minutes, int seconds)
        {
            bool check = false;
            string wav_loc = "c:\\testjazz.wav";
            SoundPlayer spWave;
            spWave = new SoundPlayer(wav_loc);

            int year, month, day;

            year = int.Parse(DateTime.Now.Year.ToString());
            month = int.Parse(DateTime.Now.Month.ToString());
            day = int.Parse(DateTime.Now.Day.ToString());

            DateTime dt = new DateTime(year, month, day, hours, minutes, seconds);

            // && DateTime.Now.Second == dt.Second
            if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute)
            {
                spWave.Play();
                check = true;
            }
            return check;
        }
        
        //'Set Alarm' Button
        private void btn_alarm_Click(object sender, EventArgs e)
        {
            for (int i = 1; i != 0; i++)
            {
                if (SetAlarm(Convert.ToInt32(dt_time.Value.Hour), Convert.ToInt32(dt_time.Value.Minute), Convert.ToInt32(dt_time.Value.Second)))
                {
                    break;
                }
            }
        }
    }
}

Exactly what happens is, when the 'Set Alarm' button on the form is clicked, it calls the 'btn_alarm_Click' method. In this method, a FOR loop starts (which is infinite) which initiates the IF loop. In the IF loop, the condition is "If SetAlarm method returns true, then break the loop". The reason why I break the loop is so that the FOR loop will not continue to run even after the alarm time has been reached (otherwise I don't think it's gonna work). So anyways, when the 'SetAlarm' method is called, it calculates whether the current time is same as the alarm time (you can check the above code to understand how it checks). If the current time matches the alarm time, it plays a '.wav' file.

Now, right after this plays, what I had done was, I wrote "return true" in the very next line, right after the "spWave.Play()" line, which meant (like you said) it was still in the IF. So what I did was, I created a bool, set it to true, and then wrote "return bool" (which is now true) right before the moethod's closing braces (again, like you said).

Now comes the running part. It compiled and ran perfectly fine. But when I click the Set Alarm button, the program practicaly freezes (as it is stuck in the FOR loop). Is there any way to run this FOR loop in background by using the BackgroundWorker component of WinForms???

[Arjay]

Your code has problems.

1) Consider the line

 
year = int.Parse(DateTime.Now.Year.ToString());

Remember when I told you about converting a value to a string and then parsing the string into an integer? This is a waste because DateTime.Now.Year is already an integer.
2) Calling DateTime.Now repeatedly may be problematic. You do this in several areas. Keep in mind that each time you call it, you retrieve a different 'Now' time. Typically, you call DateTime.Now once and then extract the partial time values from the one date.

 
DateTime dtNow = DateTime.Now;
int year = dtNow.Year;
int day = dtNow.Day;

3) The following snippet may not be reliable.

 
if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute)
{
  spWave.Play();
  check = true;
}

Just set the alarm time to the time in the future and check the whole time.

For example:

if ( dtAlarm <= DateTime.Now )
{
  spWave.Play();
  check = true;
}

4) Instead of the funky for loop of

 
for (int i = 1; i != 0; i++)
{
}

how about a while loop?

 
while( true )
{
}

5) The UI freezes because you have an infinite loop and you are stealing all the cpu cycles. The proper way to fix this is to use a secondary thread. That way, you can disable the start button, but still retain UI control. If you don't want to use a secondary thread, you can put a Threading.Sleep( 1000 ) statement inside the while loop (but this is really a hack).

 
while( true )
{
  if (SetAlarm( Convert.ToInt32(dt_time.Value.Hour),
                    Convert.ToInt32(dt_time.Value.Minute),
                    Convert.ToInt32(dt_time.Value.Second) ) )
  {
    break;
  }
  Threading.Sleep( 1000 );
}

6) Lastly, we really don't need your commentary on how the code works other than a few statements of what you intend the result to be.
This is because if you've done a good job writing the code, we'll be able to follow along by just reading the code. If we can't follow along,
then that indicates a problem with your code (part of the goal of writing good code is to make it so that others can easily follow it without it
requiring lengthy explaination).
we'll be able to read the code

[Dragster93]

Your code has problems.

1) Consider the line

 
year = int.Parse(DateTime.Now.Year.ToString());

Remember when I told you about converting a value to a string and then parsing the string into an integer? This is a waste because DateTime.Now.Year is already an integer.

I did that because otherwise, it would say, can't implicitely convert DateTime to Int.

2) Calling DateTime.Now repeatedly may be problematic. You do this in several areas. Keep in mind that each time you call it, you retrieve a different 'Now' time. Typically, you call DateTime.Now once and then extract the partial time values from the one date.

 
DateTime dtNow = DateTime.Now;
int year = dtNow.Year;
int day = dtNow.Day;

So what you're saying is that I should store the DateTime value in a variable and then use that variable rather than calling DateTime() everytime???
[/QUOTE]

3) The following snippet may not be reliable.

 
if (DateTime.Now.Hour == dt.Hour && DateTime.Now.Minute == dt.Minute)
{
  spWave.Play();
  check = true;
}

Just set the alarm time to the time in the future and check the whole time.

For example:

if ( dtAlarm <= DateTime.Now )
{
  spWave.Play();
  check = true;
}

Thanks, I shall do that.

4) Instead of the funky for loop of

 
for (int i = 1; i != 0; i++)
{
}

how about a while loop?

 
while( true )
{
}

Thanks for the idea.

5) The UI freezes because you have an infinite loop and you are stealing all the cpu cycles. The proper way to fix this is to use a secondary thread. That way, you can disable the start button, but still retain UI control. If you don't want to use a secondary thread, you can put a Threading.Sleep( 1000 ) statement inside the while loop (but this is really a hack).

 
while( true )
{
  if (SetAlarm( Convert.ToInt32(dt_time.Value.Hour),
                    Convert.ToInt32(dt_time.Value.Minute),
                    Convert.ToInt32(dt_time.Value.Second) ) )
  {
    break;
  }
  Threading.Sleep( 1000 );
}

Yes, I understood why it's freezing, that is why I tried to use the backgroundworker to do the calcuation in background. But I seem to be having a little trouble with it. I use 'backgroundworker.dispose()' to free the resources so that if the Set Alarm button is clicked a second time to set a different time, the it shouldn't display an exception saying, "The thread is busy". But that seems to work only for the second click. If you click the third time, again the same problem occurs. I shall try using the catch and throw exception for that problem and see if it works

6) Lastly, we really don't need your commentary on how the code works other than a few statements of what you intend the result to be.
This is because if you've done a good job writing the code, we'll be able to follow along by just reading the code. If we can't follow along,
then that indicates a problem with your code (part of the goal of writing good code is to make it so that others can easily follow it without it
requiring lengthy explaination).
we'll be able to read the code

Oops. :P
My bad. Next time onwards I shall put the comments in my code itself.