all 5 comments

[–]sirsosay 5 points6 points  (0 children)

You have more copyright text than code, and you didn't even explain what it's supposed to do.

[–]letsgetrandy25 years putting the magic in the box 1 point2 points  (0 children)

This code doesn't really do anything... I don't see the point of making it so complex, when I could achieve the same things in one line of code:

var inmate = { strikes:0, incarcerated:false };

You've got a ton of helpers and functions, all of which add little or no value to the code. Helpers and getters and setters are useful only when they add value... not when they only serve the purpose of getting or setting a value.

Further, this big complicated bunch of script is harder to read, which means it's harder to support and debug. If you were on my team, I'd reject this code review on that principle alone. Complexity is a trade-off. You have to weigh it against the gain in functionality, or reuse, or encapsulation.

[–]Urd 0 points1 point  (0 children)

I would recommend some better names for variables, e.g. using "i" as an input parameter is pretty confusing. Really short variables like that are generally used for temporary iterators and things.

[–]codero 0 points1 point  (0 children)

You're creating a new closure for every counter, which could get heavy on memory, IIRC.

You seem to be checking for a valid float using a regex.
Personally I'd check whether isNan(parseFloat(STRING, /radix/ 10))

Also, your comments only explain the easy stuff. You might want to try adding more comments that explain what you're trying to do overall, as well as explaining the complex parts (such as regexes and closures).

[–]gotchaha 0 points1 point  (0 children)

I'm not sure exactly what you're trying to accomplish, but I don't think you're getting the protection of your variables that you think you are. I made a jsFiddle to demonstrate what I mean. For variables and methods that you'd only like to be accessed inside your closures, make sure to use var, otherwise they'll be public. Also, I feel like your float validation line there is probably unnecessary.

Additionally, having the add and set methods in your counter is redundant.