search

addyosmani / memoize.js

A faster JavaScript memoizer
http://addyosmani.com
143 stars 19 forks source link

currentArg is implicitly global #2

Closed cowboy closed 12 years ago

cowboy commented 13 years ago

You didn't lint your code. While you're fixing this issue, add some comments.

addyosmani commented 13 years ago

Just pushed an update: currentArg should no longer be implicitly global (my fault as I should have more thoroughly checked Dmitry's changes). use strict added as per lint.

Hint is reporting the following issues. I and Mathias are aware of why the second is happening - we can fix it, but at a (albeit very minor) perf cost. I don't feel comfortable with the idea of adding 'new' to line 10, but will fix/rewrite.

Line 10: hash += (currentArg === Object(currentArg)) ? JSON.stringify(currentArg) : currentArg; Missing 'new' prefix when invoking a constructor.

Line 11: fn.memoize || (fn.memoize = {}); Expected an assignment or function call and instead saw an expression.

addyosmani commented 12 years ago

Fixed with: https://github.com/JamieMason/memoize.js/commit/f8b25f84fb53370b3c87767cae8a5b9c5be4fc66

We should however add additional comments. Adding to the todo list.