Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

My first comment would be to add some blank lines between those functions of yours. You should also add some comments describing the purpose of the functions as well. My preferred style of documentation is usually JSDoc, but you can always find a different style as well.

Secondly, even though you say that you're fine with patching Javascript's builtin object prototypes, it's still a really bad idea. See this Stackoverflow question this Stackoverflow question for more details on that.

From looking at your code, I've also noticed that you don't use braces in many places, like here:

if (typeof find == 'string') return this.split(find).join(replace);

Generally it's good to use braces, even if you're only running one line of code. A famous example of this ending up very badly is the Apple SSL bug. The bug looked something like this:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
 goto fail;
 goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */

So, in short, use braces.

Other than that, your code looks really nice!

My first comment would be to add some blank lines between those functions of yours. You should also add some comments describing the purpose of the functions as well. My preferred style of documentation is usually JSDoc, but you can always find a different style as well.

Secondly, even though you say that you're fine with patching Javascript's builtin object prototypes, it's still a really bad idea. See this Stackoverflow question for more details on that.

From looking at your code, I've also noticed that you don't use braces in many places, like here:

if (typeof find == 'string') return this.split(find).join(replace);

Generally it's good to use braces, even if you're only running one line of code. A famous example of this ending up very badly is the Apple SSL bug. The bug looked something like this:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
 goto fail;
 goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */

So, in short, use braces.

Other than that, your code looks really nice!

My first comment would be to add some blank lines between those functions of yours. You should also add some comments describing the purpose of the functions as well. My preferred style of documentation is usually JSDoc, but you can always find a different style as well.

Secondly, even though you say that you're fine with patching Javascript's builtin object prototypes, it's still a really bad idea. See this Stackoverflow question for more details on that.

From looking at your code, I've also noticed that you don't use braces in many places, like here:

if (typeof find == 'string') return this.split(find).join(replace);

Generally it's good to use braces, even if you're only running one line of code. A famous example of this ending up very badly is the Apple SSL bug. The bug looked something like this:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
 goto fail;
 goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */

So, in short, use braces.

Other than that, your code looks really nice!

Source Link
Ethan Bierlein
  • 15.9k
  • 4
  • 60
  • 146

My first comment would be to add some blank lines between those functions of yours. You should also add some comments describing the purpose of the functions as well. My preferred style of documentation is usually JSDoc, but you can always find a different style as well.

Secondly, even though you say that you're fine with patching Javascript's builtin object prototypes, it's still a really bad idea. See this Stackoverflow question for more details on that.

From looking at your code, I've also noticed that you don't use braces in many places, like here:

if (typeof find == 'string') return this.split(find).join(replace);

Generally it's good to use braces, even if you're only running one line of code. A famous example of this ending up very badly is the Apple SSL bug. The bug looked something like this:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
 goto fail;
 goto fail; /* MISTAKE! THIS LINE SHOULD NOT BE HERE */

So, in short, use braces.

Other than that, your code looks really nice!

default

AltStyle によって変換されたページ (->オリジナル) /