3
\$\begingroup\$

This changes the application's volume through the Trackbar (1-100, 1% to 100%):

public static class NativeMethods
{
    //Winm WindowsSound
    [DllImport("winmm.dll")]
    internal static extern int waveOutGetVolume(IntPtr hwo, out uint dwVolume);
    [DllImport("winmm.dll")]
    internal static extern int waveOutSetVolume(IntPtr hwo, uint dwVolume);
}

private void trackBar1_Scroll(object sender, EventArgs e)
{
    uint CurrVol = ushort.MaxValue / 2;
    NativeMethods.waveOutGetVolume(IntPtr.Zero, out CurrVol);
    ushort CalcVol = (ushort)(CurrVol & 0x0000ffff);
    int NewVolume = ((ushort.MaxValue / 100) * trackBar1.Value);
    uint NewVolumeAllChannels = (((uint)NewVolume & 0x0000ffff) | ((uint)NewVolume << 16));
    NativeMethods.waveOutSetVolume(IntPtr.Zero, NewVolumeAllChannels);
    label1.Text = Convert.ToString("Volume: " + trackBar1.Value + "%");
}

Is it possible to make String.Format use multiple arguments/values?

For example:

 String Status = String.Format("Current Buffer: {0}", waveProvider.BufferedDuration.Milliseconds, TimesBufferClear) + " Clear: " + TimesBufferClear.ToString() + " Ping: " + PingReply + " Buffer: " + SendStream.BufferMilliseconds;

There, I would like to use it on everything, as they all work pretty much the same. But if I have to write String.Format every time, it will take up much more space.

Is this code okay, or can it be improved?

\$\endgroup\$
3
\$\begingroup\$

There isn't much to review in 6 lines of code. But oh well.

  1. Native methods return error codes, so it is good practice to handle those codes somehow, not just ignore them. For example, if method reports, that there is no sound driver or no sound card or w/e, it makes sense to notify user somehow. Write warning in the status bar of your window or something.
  2. It is meaningless to initialize CurrVol, since its value will be overriden anyway, wont it? And, if its your real code, you dont have to recieve current volume in order to set it to begin with.
  3. Convert.ToString should be refactored to String.Format("Volume: {0}%", trackBar1.Value);. More complex example:

    var status = String.Format("Current Buffer: {0} Clear: {1} Ping: {2} Buffer: {3}", waveProvider.BufferedDuration.Milliseconds, TimesBufferClear, PingReply, SendStream.BufferMilliseconds);
    
\$\endgroup\$
  • \$\begingroup\$ Yeah i know, but thanks for answering anyway. And How am i supposed to handle the errors? I know ishould handle them, but, i don´t know in what way? True that´s it´s overidden, didn´t think of that. But what do you mean "if it´s your real code you don´t need..." . Don´t i need to have the Currvol var?. And nice, looks much better now at the string! \$\endgroup\$ – Zerowalker Aug 29 '13 at 18:03
  • \$\begingroup\$ @Zerowalker, i dont see you using Currvol anywhere in your code (except for calculating CalcVol which you dont use either). So i think first three lines can be removed from event handler. As for strings - see my answer. \$\endgroup\$ – Nikita B Aug 30 '13 at 4:18
  • \$\begingroup\$ @Zerowalker, also, if you are using Visual Studio i recommend you installing ReSharper (at least trial version). It will help you track common mistakes and improve your code style. \$\endgroup\$ – Nikita B Aug 30 '13 at 4:24
  • \$\begingroup\$ Very true, didn´t notice that, that´s embarrassing, thanks. And nice, perfect, now i understand why it had "0" in it. Ah heard of that, will look it up! \$\endgroup\$ – Zerowalker Aug 30 '13 at 8:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.