[FryPan]

Close close close close close close

[eithan1231]

1) you're declaring the useragent everywhere you need it, just store in a constant variable.
2) try name your textbox's.
3) when you catch all exceptions, do something with them, don't just ignore them.
3) use url encoding.
4) dont just assume the response was json, check the content-type header.
5) why is the main project called "tests?" lol

[Owl]

4) dont just assume the response was json, check the content-type header.

If JSON is returned as standard I wouldn't bother.

[abratar]

If JSON is returned as standard I wouldn't bother.

If the url has changed?
If the url is beeing redirected?
If it throws an error like unreachable?
Many other things...

[Owl]

If the url has changed?
If the url is beeing redirected?
If it throws an error like unreachable?
Many other things...

None of which can be determined from the content-type header.

[0xDEAD10CC]

If the url has changed?
If the url is beeing redirected?
If it throws an error like unreachable?
Many other things...

That would be determined all by the HTTP response code. Not any of the headers.

404, 303, 301, etc..

Still it's good pre-validation if you want to avoid potential deserialization issues. I see nothing wrong with adding in sanity checks.

Other notes - SaveFileDialog is IDisposable btw. And that try {} catch {} style is horrible.

[Ninja]

If it throws an error like unreachable?

Then it would throw a WebException and you can use the WebException.Response property.

---

On line 123 on the MainForm.cs you are getting the WebResponse object but you are not disposing of it. You are only disposing of the response stream and the stream reader.
The WebRequest by default has the method "GET" set, so no need to set it again.
KeepAlive is by default true, so no need to set it.
You could re-use the code to set WebRequest headers. You could make a (extension-)method for WebRequest that sets the headers automatically, or loads them from a collection and sets them.
ShowDialog() returns a DialogResult enum, you should check if the value is "DialogResult.OK".
And finally, instead of doing TextBox.Text = "" you can do TextBox.Clear().

[0xDEAD10CC]

Then it would throw a WebException and you can use the WebException.Response property.

---

On line 123 on the MainForm.cs you are getting the WebResponse object but you are not disposing of it. You are only disposing of the response stream and the stream reader.
The WebRequest by default has the method "GET" set, so no need to set it again.
KeepAlive is by default true, so no need to set it.
You could re-use the code to set WebRequest headers. You could make a (extension-)method for WebRequest that sets the headers automatically, or loads them from a collection and sets them.
ShowDialog() returns a DialogResult enum, you should check if the value is "DialogResult.OK".
And finally, instead of doing TextBox.Text = "" you can do TextBox.Clear().

Some of these are a little too picky but still all fair points. For re-setting default values I don't think there's anything wrong in doing that as it just adds verbosity to the code with no real adverse side effects. Readability is always a nice thing to have so if there's things that are explicit in the code, it isn't always a terrible thing. I think some people sleep better at night too knowing that they don't have to rely on default behavior to stay the same. You never know what minor changes Microsoft makes between framework versions based on feedback of other developers.

[Ninja]

Some of these are a little too picky but still all fair points. For re-setting default values I don't think there's anything wrong in doing that as it just adds verbosity to the code with no real adverse side effects. Readability is always a nice thing to have so if there's things that are explicit in the code, it isn't always a terrible thing. I think some people sleep better at night too knowing that they don't have to rely on default behavior to stay the same. You never know what minor changes Microsoft makes between framework versions based on feedback of other developers.

True, however, I was just pointing out that they OP does not have to set them. Maybe OP didn't know the default values and thought he had to set them.

[Icey.]

FYI the api your using when getting the download route is using a session token (hash) that you got to swap out after abit of time.
Not that hard 2 add since already did it for my version. Also added a playlist download function.

If u need any help adding it just DM me :)

[FryPan]

Awesome tips guys. Tyvm, will improve the code.