5

Have been trying to learn more about functional programming by looking at the underscore documentation and attempting to write my own versions of the more frequently used functions.

Coming across "memoize" - I was having trouble wrapping my head around it, and found some information in Crockford's 'The Good Parts'.

  _.memoize = function(func) {
    // 'cache' object is used to hold the result of the memoized fn's call
    var cache = {};
    var recur = function(n) {
      var result = cache[n];
      if (typeof result === 'undefined') {
        result = func.apply(this, arguments);
        cache[n] = result;
      }
      return result;
    }
    return recur;
  };

Could you please help me understand if my use of .apply was even necessary and if there is any core improvement I can make to this code? Really appreciate the help!

4
  • Yes, the use of apply is necessary because you're unaware of how many arguments func will have Commented Sep 12, 2015 at 20:18
  • So that's the code you've written yourself? And there's nothing you don't understand about it? Or what parts of underscore don't you understand? Commented Sep 12, 2015 at 21:01
  • @Bergi - I was able to arrive at this solution, but I wanted to see if there were any errors or improvements I could make. As I mentioned below, I have since changed out the use of typeof and rely on hasOwnProperty instead. However, I'm trying to figure out how I can prevent the memoized function from running any more times than it needs to. Commented Sep 12, 2015 at 21:11
  • 1
    Maybe the question was better be asked at Code Review then. But it's OK. Commented Sep 12, 2015 at 21:12

1 Answer 1

7

There are some obvious issues with correctness:

1) You cache the result based only on the value of the first argument, so calling a function with different arguments will produce a wrong result!

let add = memoize((a, b) => a + b)
add(1, 2)
> 3
add(1, 3)
> 3

2) You use undefined to represent a function which has not been called yet, but undefined is a valid return value. See how the side effect happens twice:

let log = memoize(() => console.log('hello'))
log()
> hello
log()
> hello

You can instead use cache.hasOwnProperty to detect if something is in the cache.

I'd recommend for you to write a test suite. Tests are quite valuable in finding these bugs. Think up other edge cases where your function might not work properly. Standalone utilities like this really lend themselves to test-driven coding.

Sign up to request clarification or add additional context in comments.

2 Comments

Hey, thanks so much for the clear reply. It's very helpful. I went ahead and refactored taking into account that using 'undefined' could still return a valid return value. I replaced the line instead with if (!cache.hasOwnProperty(result)) {
Think about var log = memoize(console.log.bind(console)); log("hasOwnProperty"); log("hasOwnProperty");

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.