0
\$\begingroup\$

I am running a timer for updating some of my operations, such as buffers. But I also want ping to be added.

It is added, but I think it can be improved. This is because I think it's running on the GUI thread, which is not preferred as it will freeze the GUI if something goes wrong with it. I think such a thing has occurred, and I haven't been able to locate the problem until now.

But, I am not asking for the solution for that problem, as there are other stuff. I am basically asking for ways to improve this piece of code as it doesn't seem to be optimal in any way.

private void timer1_Tick(object sender, EventArgs e)
{
 try
 {
 if (waveProvider.BufferedDuration.Milliseconds > 40 && AudioDevices.SelectedItem.ToString() != "Wasapi Loopback")
 {
 waveProvider.ClearBuffer();
 TimesBufferClear++;
 }
 currping.Text = "Current Buffer: " + waveProvider.BufferedDuration.Milliseconds.ToString() + " Clear: " + TimesBufferClear.ToString() + " Ping: " + pingSender.Send(otherPartyIP.Address).RoundtripTime.ToString() + " Buffer: " + SendStream.BufferMilliseconds;
 }
 catch (Exception ex)
 {
 MessageBox.Show(ex.Message + "Timer");
 }
}

As you can see, I have a label which contains all the things I want to update. It is fine, and I'd rather have one than many in this case.

But is this a good way to do it? It doesn't seem that good when I look at it; it's just a mess, even if it works.

Also, the PingSender is created and disposed of outside of the thread. I suppose that is better than just "using" it every timer run.

Are there any ideas of what can be done to improve it?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Aug 20, 2013 at 23:49
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

As it is not really much code, let us just focus on the label part and a little naming.

TimesBufferClear++;

If this is a variable, we should use camelCase casing for the name. If it is a property it follows the naming convention.

currping 

Can be renamed to currentPingStatus, which is more meaningful.

Now let us refactor the assignment to the Text property of this label:

String formatPattern = "Current Buffer: {0} Clear: {1} Ping: {2} Buffer: {3}";
currentPingStatus.Text = String.Format(formatPattern,
 waveProvider.BufferedDuration.Milliseconds.ToString(),
 TimesBufferClear.ToString(),
 pingSender.Send(otherPartyIP.Address).RoundtripTime.ToString(),
 SendStream.BufferMilliseconds);
answered Aug 22, 2014 at 7:46
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.