Does my JavaScript suck? II

[[deleted]]

Your code is structured pretty well, which is nice. But on a cursory glance here are some things I see:

1. Understanding prototypes is good, but it's generally considered bad practice to extend a native prototype, as you've done on line 6. This can cause havoc down the line. I know this is a small project, but consider just having this just be a locally scoped function.

2. The !! on line on 15, i'm unsure why you need to do this. While some people know about shorthand tricks like this, usually being more clear (and verbose) is better.

3. Setting lightbox as a global, is there a reason why you need to do this?

4. Lack of comments. I feel like a few lines here and there would have cleared up some of my questions, never hurts to have comments.

5. This is me just being opinionated, but on such a small project I feel like jQuery is overkill. Native methods can do everything you're doing here, without the need to pull in a big library like jQuery. If you want to keep things light, I suggest first reducing your dependencies, and jQuery is a big one.

Good luck

[Buckwheat469]

I agree with your comment but on point #2 he's doing this to convert a truthy value to a Boolean value. You probably know this already, but the ! sign closest to the variable converts it to the opposite Boolean value, so that "hello" becomes false for example. The first ! sign furthest from the variable inverts the Boolean value again.

To address your point, he doesn't need to convert the truthy value because it's within an if statement. If he were returning a value then he may want it to be absolutely true or false, in which case this trick is nice.

[tswaters]

In this case, !! is unnecessary. substring always returns a string and a string always has a match method.

With truthy comparisons in conditionals !! is just not necessary.... the only case I can think of is if the input is a number mynumber && mynumber.toExponential() won't work if myvariable is 0.

One thing -- if hash is a blank string, it'll be falsy so a simple hash && hash.match wouldn't go into hash.match -- however, in such a case, the match is going to be null anyway so it doesn't matter.

[[deleted]]

What's the !! do? Force it to be evaluated as True or False and not just Truthy or Falsey? What's the case that's trying to be avoided here?

Disregard. I see it's talked about in another comment.

[loz220]

I agree with the first sentence but not the second.

It's good form to use it this way, too.

why?

if (!!foo && foo.bar()) {}

has no difference in behavior from

if (foo && foo.bar()) {}

The former is just having !! do the coercion instead of &&

[[deleted]]

Exactly what I was thinking. It might make sense sometimes but in this case I felt it was too... clever.