Update youtube-feeds.js

[skeggse]

I converted and standardized the syntactic style to that of node's core modules, including changing the indentation to spaces.
I removed redundant code and comments, adding comments where more documentation was needed. Some code is verbose enough to document itself.
I added custom error constructors for some common error patterns, to reduce code redundancy.
I added strict checking (=== and !==) where applicable. Strict checking doesn't have the expensive job of coercing types to match, and thus is faster.
I added semicolons, which you seemed to have neglected. Semicolons, while not technically required for execution, are necessary for valid code.
I lowered the default timeout to five seconds, because, while the industry standard is 30 seconds, I've found that 30 seconds is far too long to wait if the network is goofing up. In almost all cases, you'll know within 5 seconds whether your request will complete.

Many of these changes are partially subjective.

[fvdm]

Thanks for cleaning up the code, truly appreciate it!

I added semicolons, which you seemed to have neglected. Semicolons, while not technically required for execution, are necessary for valid code.

Yes I disregard semicolons: they are not required, using the module with or without them won't make a difference, you don't need them to indicate linebreaks (that's what the linebreak is for), and they remind me of endless late 'debug code staring' nights and mornings with PHP. Also I don't see how they add to validation. Maybe in the enterprise world, but that's another rant...

To keep my code readable, I only use them where absolutely needed. Like one-line parts:

switch( stuff ) {
  case 'hello': more = 'dog'; break
  case 'some': more = 'cat'; break
}

This is not meant to be offensive to you, I just have a semicolon trauma that nobody has ever successfully cured me from. Not even coffee.

---

Again, thanks a lot for the effort to meet code standards and improve efficiency. I'm away from my computer for a week, so feel free to add more commits if you like and I'll review it all next weekend or early week 18.

[skeggse]

Interesting...I have a bit of the opposite--no semicolons cause me considerable strife. I learned client-side JavaScript before Node.js, and if you wanted to do anything efficiently, you had to minify your code for the client-side. The minification process requires semicolons in all applicable places. Evidently, I became used to it and it is now readable for me.

Well, anyway, I suppose it's your choice in the end. For me, straight C-style syntax is by far more readable than, say, CoffeeScript, which is the other extreme. Using semicolons makes it more compatible, syntactically, with the core modules, but that doesn't affect anyone not looking at the source.

Still, many sources agree with using semicolons, though there are plenty that disagree.

[fvdm]

I reconsidered my position regarding at least this specific module. Although I prefer my own style (who doesn't) sooner or later someone may run into compatibility issues when integrating (parts of) this code into theirs. It is being used by quiet a lot of people and some may use/port it for web scripts and therefore may get minimizer problems.

Even though I released the code as public domain, allowing everyone to use and alter it in any way they like, it feels a bit silly when everyone has to make the same changes again and again only to meet standards. Also the moment you change the code you probably need to integrate future updates manually, bypassing npm's updater.

@skeggse I still need to test your PR, but the semicolons can stay ;)

[skeggse]

Okay :)

Travis appears to have approved this pull request, meaning that it built (and ran?) successfully.

[fvdm]

There is no test script yet, so for now Travis only does a rough syntax check :P

[skeggse]

Ah, ok.
On Apr 26, 2013 9:40 AM, "Franklin van de Meent" notifications@github.com
wrote:

There is no test script yet, so for now Travis only does a rough syntax
check :P

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-17078239
.

[fvdm]

I reviewed the PR and what I'm really going change is the timeout to 5 sec and strict checking. I can't get used to the spaces and semicolons, visually it becomes a big pile of text I can't properly maintain. I also don't see added value in the error constructors and other minor edits.