6

I'm following a tutorial on how to use capture video using the Windows MediaCapture API on Windows Phone and on the code examples, some of the variables are set to null just before assigning a new instance.

private void InitCaptureSettings() {
    _captureInitSettings = null;
    _captureInitSettings = new Windows.Media.Capture.MediaCaptureInitializationSettings();
    _captureInitSettings.AudioDeviceId = "";
    _captureInitSettings.VideoDeviceId = "";
    _captureInitSettings.StreamingCaptureMode = Windows.Media.Capture.StreamingCaptureMode.AudioAndVideo;
    _captureInitSettings.PhotoCaptureSource = Windows.Media.Capture.PhotoCaptureSource.VideoPreview;

    if (_deviceList.Count > 0) {
        _captureInitSettings.VideoDeviceId = _deviceList[0].Id;
    }
}

Is there any reason why this should be done?

Thanks

GLlompart
  • 261
  • 5
  • 18
  • In that method? No, it's pointless. However, when not following an example, member variables are typically set during object initialization to a programmer-defined "default" value as some defaults won't fit with how they are to be used. – krillgar Mar 11 '15 at 14:03
  • 1
    It changes the behavior. If `MediaCaptureInitializationSettings` constructor throws exception, `captureInitSettings` will be null, if you don't it will be previous value. But generally no one writes code like this. – Sriram Sakthivel Mar 11 '15 at 14:03
  • 1
    @SriramSakthivel: Yup, I noticed that as I was writing an answer. – Jon Skeet Mar 11 '15 at 14:05

3 Answers3

10

The only point in doing this would be if the MediaCaptureInitializationSettings constructor could throw an exception, and you wanted to make sure that in that case the variable didn't still have a reference to an "old" object. That's pretty rarely useful, in my experience. (If a method like this throws an exception, I'd try to avoid using the object it was initializing...)

I'd recommend doing all of this with an object initializer:

_captureInitSettings = new MediaCaptureInitializationSettings
{
    AudioDeviceId = "",
    VideoDeviceId = _deviceList.Count > 0 ? _deviceList[0].Id : "",
    StreamingCaptureMode = StreamingCaptureMode.AudioAndVideo,
    PhotoCaptureSource = PhotoCaptureSource.VideoPreview
};

This has two benefits:

  • It's simpler code, IMO... much less repetition
  • It only sets the variable's value if the whole object initializer completes. If setting one property fails, you don't end up with a reference to a half-initialized object.
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    Apart from the exception behavior which I noted already looking at the OP's code, I was about to comment that was written by some stupid. But later realized it was from msdn, Oh no!. Ummm, that doesn't make it a better code, it is still a stupid code. – Sriram Sakthivel Mar 11 '15 at 14:16
  • @SriramSakthivel: Yes, unfortunately there are plenty of bad samples on MSDN :( – Jon Skeet Mar 11 '15 at 14:16
  • Yes Jon, there are even code snippets which even hurts you. Here is one [example](http://stackoverflow.com/questions/27360252/when-using-filestream-readasync-should-i-open-the-file-in-async-mode/27360773#27360773) – Sriram Sakthivel Mar 11 '15 at 14:18
1

I consider it good practice to set all fields of a newly constructed class to some well known default value.

Why?

First it is basic "code hygiene".

Second: while this makes not much sense in a small example which just shows a constructor, imagine a scenario where the fields of this class are modified at a later stage. If during this modifications something goes wrong, i.e. an Exception is thrown, it may leave your class in a bad state.

DrKoch
  • 9,556
  • 2
  • 34
  • 43
  • 4
    Fields are already initialized to default values - null, 0 etc. There's no need to do that explicitly. (It's not clear that this is during construction at all, given that it's in a method.) – Jon Skeet Mar 11 '15 at 14:13
0

In your example we cannot tell if it serves a purpose or not.

What you are doing is setting the current instance of MediaCaptureInitializationSettings to NULL and then creating a new instance of MediaCaptureInitializationSettings.

The variable you set to NULL doesn't affect the new instance.

CathalMF
  • 9,705
  • 6
  • 70
  • 106
  • Follow the link if you want to see the whole example. It's just a fragment of it – GLlompart Mar 11 '15 at 14:04
  • 2
    You never set an *object* to null. You set a *variable* to null. It doesn't make any different to the object that the variable's old value referred to. – Jon Skeet Mar 11 '15 at 14:05
  • I don't see the fix - you're still saying "setting the current instance [...] to NULL". – Jon Skeet Mar 11 '15 at 14:14
  • @JonSkeet Im not sure what you mean?? _captureInitSettings is an instance of MediaCapture.... and hes setting it to NULL. – CathalMF Mar 11 '15 at 16:06