Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($cacheFactory): Make Cache.put return the value #1583

Closed
wants to merge 1 commit into from
Closed

feat($cacheFactory): Make Cache.put return the value #1583

wants to merge 1 commit into from

Conversation

taralx
Copy link

@taralx taralx commented Nov 15, 2012

Using cache.put is slightly awkward because you can't just wrap an expression with it like you can a standard object.

Coffeescript with plain object:
cache = {}
(key) -> cache[key] ?= genValue(key)

Coffeescript with $cacheFactory cache:

cache = $cacheFactory.get('cache')
(key) -> cache.get(key) ? (r = genValue(key); cache.put(key, r); r)

With this change:

cache = $cacheFactory.get('cache')
(key) -> cache.get(key) ? cache.put(key, genValue(key))

@pkozlowski-opensource
Copy link
Member

@taralx This looks good apart from the fact that the test could be simplified (let me know if I've missed anything).

Could you re-work the commit message as described in http://docs.angularjs.org/misc/contribute ?
You would also have to sign the CLA as well.

@taralx
Copy link
Author

taralx commented Nov 16, 2012

Updated.

@pkozlowski-opensource
Copy link
Member

@taralx Did you sign the CLA?

@taralx
Copy link
Author

taralx commented Nov 16, 2012

Yes.

@@ -46,13 +46,15 @@ function $CacheFactoryProvider() {

refresh(lruEntry);

if (isUndefined(value)) return;
if (isUndefined(value)) return value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is useless because return undefined is the same thing as return.

@IgorMinar
Copy link
Contributor

I was inclined to say no to this change because it looked like only coffee script would benefit from it, but it turns out that even in js you can simplify the code thanks to the ability to use ternary operators.

Before:

function getVal(key) {
  var val = cache.get(val);
  if (val === undefined) {
    val = getnerateVal(key);
    cache.put(key, val);
  }
  return val;
}

Before with inlined assignment:

function getVal(key) {
  var val = cache.get(val);
  if (val === undefined) {
    cache.put(key, val = getnerateVal(key));
  }
  return val;
}

After:

function getVal(key) {
  var val = cache.get(val);
  if (val === undefined) {
    val = cache.put(key, getnerateVal(key));
  }
  return val;
}

After w/ ternary operator:

function getVal(key) {
  var val = cache.get(val);
  return (val === undefined)
      ? cache.put(key, getnerateVal(key))
      : val;
}


it("should return value from put", inject(function($cacheFactory) {
function DummyClass() {}
var obj = new DummyClass();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just var obj = {}, we just need to be able to test object identity and for that object literal is good enough.

IgorMinar pushed a commit that referenced this pull request Nov 24, 2012
This allows common programming patterns to be expressed with more
concise code.

See #1583 for code examples.
@IgorMinar
Copy link
Contributor

landed as 168db33

thanks!

@IgorMinar IgorMinar closed this Nov 24, 2012
IgorMinar pushed a commit to IgorMinar/angular.js that referenced this pull request Nov 26, 2012
This allows common programming patterns to be expressed with more
concise code.

See angular#1583 for code examples.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants