4

We have some js code splitted in many files. We have a core file that defines code used by many other js files.

Currently we have something like this:

core.js:

window.mycore = function() {

    var myfunction1 = function() {
    };
    var myfunction2 = function() {
    };
    var myfunction3 = function() {
        //..
        var a =  myfunction1(b);
        //..        
    };
    //...
    // many "myfunction"
    //...
    var myfunctionN = function() {
    };
    var publish = function() {
        for(var i = 0; i < arguments.length; i++) {
            try {
                window.mycore[arguments[i]] = eval('(' + arguments[i] + ')');
            }
            catch(e) {
                Log.err(600, arguments[i], e);
            }
        }
    };
    publish("myfunction1", "myfunction7", "myfunction8",/*...*/"myfunctionM")
}

app.js:

// ...
// ...
var result = window.core.myfunction1("myparam");
// ...
// ...

Note that none core methods are declared as members of the window.core object. Instead they are attached to the core object with the publish function.

This has some pros:

  • The core code can reference any core function without the need of writing "window.core."
  • We avoid writing "var myfunction = window.mycore.myfunction = function() ..." in every public function declaration
  • The exposed methods can be seen centraliced.

But, the use of eval in the publish function is bringing us problems when using code analysis tools since they don't tend to understand eval declarations.

So, here is my question. Which is the better way to improve this code, so we can keep the advantages mentioned but eradicating the eval declaration. I am aware of the solution of sending to the publish function some name/value pairs like publish({'myfunction1': myfunction1}, ... ), but I also want to avoid function name repetitions. Consider that I am not looking for radical changes since there is a lot of code written already.

Thanks!

gztomas
  • 3,030
  • 3
  • 27
  • 38

3 Answers3

2

I'm not sure I understand completely your reasons for using the "publish" method, but is there any reason your not just returning an object with the correct functions from your constructor?

ie:

window.mycore = (function() {
   var myFunc1 = function(a) {
      alert(a);
   };

   var myFunc2 = function(b) {
      // call to other function in the same scope
      myFunc1(b);
   }

   ...

   // at the end just expose the public members you want
   return {
      myFunc1: myFunc1,
      myFunc2: myFunc2
   };
})();

or

window.mycore = (function() {
   return {
      myFunc1: function(a) {
         alert(a);
      },
      myFunc2: function(b) {
         this.myFunc1(b);
      }
   };
})();

or, yet another way to end up with the same object :) ... as always there are different ways to get there

(function(){

    var o = {};

    o.func1 = function(a) {
        alert(a);
    }

    o.func2 = function(b) {
        this.func1(b);
    }

    window.mycore = o;

})();
Jaime
  • 6,736
  • 1
  • 26
  • 42
  • I want to avoid repeating function names like it's happening in myFunc1: myFunc1. Consider there are many functions. – gztomas May 10 '12 at 14:56
  • see my edit. You could do it that way. Besides your actually repeating the function names anyway, once for the declaration and once in the call to the publish function, aren't you? – Jaime May 10 '12 at 15:02
  • It has the con that whenever I want to reference myFunc1 in myFunc2 I have to write window.mycore.myFunc1 – gztomas May 10 '12 at 15:07
  • well, in the second version, you just have to write this.myFunc1, not the global object, in the first version you don't need to do anything. – Jaime May 10 '12 at 15:12
  • Calling func2 fails as func1 can't be called directly. – gztomas May 10 '12 at 15:40
0

So, at a fundamental level, I think it would have benefitted you to have written those name spaces as objects. But thats a whole different subject entirely. (and it disqualifies based on the fact that you dont want to do a lot of refactoring).

With that said, my first idea was that you could probably sidestep the need for eval by using the .call() or .apply() method. What they allow you to do is to chain a function call out of your function name. but that doesn't apply to a "string" which is what you're giving your publish function.

so after googling, this is how you execute a function from a string:

var fn = window[settings.functionName];
if(typeof fn === 'function') {
    fn(t.parentNode.id);
}

https://stackoverflow.com/a/912642/680578

Community
  • 1
  • 1
Kristian
  • 21,204
  • 19
  • 101
  • 176
0

Personally I prefer the @Jaime approach, but maybe you may do something like

window.mycore = function() {

    function myfunction1() {
    };
    function myfunction2() {
    };
    function myfunction3() {
       //..
       var a =  myfunction1(b);
       //..        
    };
    //...
    // many "myfunction"
    //...
    function myfunctionN() {
    };
    var publish = function() {
       for(var i = 0; i < arguments.length; i++) {
          try {
             window.mycore[arguments[i].name] = arguments[i];
          }
          catch(e) {
             Log.err(600, arguments[i].name, e);
          }
       }
    };
    publish(myfunction1, myfunction7, myfunction8,/*...*/myfunctionM);
}
abidibo
  • 4,175
  • 2
  • 25
  • 33