2
\$\begingroup\$

My intent is to use this object to cache data from web responses. The cache object should only be valid for the current data.

var myCache = {

        Cache: { Date: '', Data: [] },

        AddData: function(Name, Data){
            myCache.CheckCache();
            myCache.Cache.Data[Name] = Data;
        },

        GetData: function(Name){
            myCache.CheckCache();
            return myCache.Cache.Data[Name];
        },

        HasData: function(Name){
            myCache.CheckCache();
            var data = myCache.Cache.Data[Name];
            var val = true;
            if(typeof data === "undefined" || data == null || data === '') {
                val = false;
            }
            return val;
        },

        CheckCache: function(){

            var currentDate = myCache.GetCurrentDate();
            var CacheDate = myCache.Cache.Date;
            if(CacheDate != currentDate){
                console.log('Reset Cache');
                myCache.Cache.Data = [];
                myCache.Cache.Date = currentDate;
            }
        },

        GetCurrentDate: function(){
            var today = new Date();
            var dd = today.getDate();
            var mm = today.getMonth() + 1;
            var yyyy = today.getFullYear();
            if(dd < 10) {
                dd = '0' + dd;
            } 
            if(mm < 10) {
                mm = '0' + mm;
            } 
            var returnDate = yyyy + '-' + mm + '-' + dd;
            return returnDate;
        }
    };

When the main page loads I call

myCache.CheckCache();

When I get data in the application I use the cache object as

   var data;
   if(myCache.HasData('UserData')){
       data = myCache.GetData('UserData');
   } else{
       data = myWebApi.Get(Url);  
   }

Is there a better way of creating a cache object? Or any suggestions of improvment

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Your browser already has a cache. Why would you re-implement that? Just use HTTP standards-compliant 304 response codes with Etags. \$\endgroup\$
    – Dan
    Commented Apr 26, 2016 at 10:35
  • \$\begingroup\$ Yes, that is true. I want to re-implement it because I would like to se if it can be more efficient than just leaving it to the browser. \$\endgroup\$
    – Marcus H
    Commented Apr 26, 2016 at 11:38

1 Answer 1

2
\$\begingroup\$

There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.

Phil Karlton

I'd suggest leaving the job to the browser and to the experts that make it. Additionally, peppering your code with a lot of if-else just to use your cache is too much unnecessary effort really.

Now if you insist going in this route...

  • First, your function names. They're like from C# land. JavaScript usually uses camel case, with the very first letter in lower case.

  • There's no sense storing date in a formatted way. You can store it in milliseconds instead. You can get the current timestamp using Date.now().

  • Your cache tracks time globally, not on a per-item basis. This means there will be a period after wiping that you'll be loading stuff fresh instead of the cache.

  • An better way to clear an array is to set its length property to 0 instead of assigning another array.

  • You can take advantage of localStorage to cache content more persistently than just in memory. Note that localStorage has a size limit of 5-10Mb depending on the browser and it only stores strings (objects need be serialized first).

  • Instead of peppering your code with a lot of if-else operations, consider making a wrapper function for data-fetching mechanisms. Sort of like a decorator.

    function cacheWrapper(fn){
      var sourceId = generateIdForFn(fn);
      var isCached = MyCache.hasData(sourceId);
      return isCached ? MyCache.getData(sourceId) : fn();
    }
    
    function() originalGetter{ return someRemoteApi.getData(); }
    
    var cachingGetter = cacheWrapper(originalGetter);
    
    var potentiallyCachedData = cachingGetter();
    
  • undefined == null. It's always best to use strict comparison to avoid ambiguity.

\$\endgroup\$
2
  • \$\begingroup\$ "An better way to clear an array is to set its length property to 0 instead of assigning another array." - Why is this a better way? One mutates the original array whereas the other lets it go out of scope and get GCed. Hopefully. I think I answered my own question there.. \$\endgroup\$
    – Dan
    Commented Apr 26, 2016 at 13:25
  • \$\begingroup\$ First of all, Thank u Joseph for the answer. You are very much correct when you say my code look c#ish :) The purpose of using a created cache is just to measure if I can win some time in any way. Mayby I'm in deep water but I got a lot of time to play around just to try:) I really like your wrapper idea, I will do some research on that part. \$\endgroup\$
    – Marcus H
    Commented Apr 27, 2016 at 5:20

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.