Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have either created something beautiful or something monstrous, and I'm not really sure which, and I don't know where else to turn but here. What I've done:

var MyObject = function(opts) { 
    //init code, set vals, runs when 'new MyObject()' is called
}

MyObject.prototype = (function() {
    //private helper functions
    //scoping is a pain, but doable
    function myPrivateFunction() {}

    return {
        publicFunc1: function() { /* uses myPrivateFunction() */ }
        , publicFunc2: function() { /* uses myPrivateFunction() */ }
    };
})();

MyObject.prototype.publicFunc3 = function() {}
MyObject.prototype.publicFunc4 = function() {}
MyObject.prototype.publicFuncEtc = function() {}

To my surprise, this works, and takes care of a pretty significant problem of creating reusable private functions for various public functions. Of course, I have to do some scoping for the this object, but I feel it's a small price to pay for being able to use private reusable functions. I left the other functions on the outside to avoid having to deal with scoping.

My question is: is this a code smell? and as a corollary, is there a better way to do this?

share|improve this question
1  
publicFunc3 can't see myPrivateFunction... sort of smells. And of course privates don't have a this, which sort of defeats OO design. Why not just wrap the whole thing in a IIFE and put the private stuff inside? – Dagg Apr 24 '13 at 22:37
    
yes.. that's because it doesn't need to. is that bad? – Jason Apr 24 '13 at 22:39
    
It's probably not good... someone else working with the code will probably assume all public "methods" have access to all "private" ones. Personally, I think not worrying about trying to make things private makes life a lot easier when dealing with languages with no concept of private ;) – Dagg Apr 24 '13 at 22:40
    
that's a fair point. – Jason Apr 24 '13 at 22:41
    
@Dagg the whole thing in my actual code is wrapped in an IIFE. I never thought to just drop the private funcs inside of that and be done with it. Thank you for your input! I wish I could accept your comment as the answer ;) – Jason Apr 24 '13 at 22:46

The commenters pretty much said it all:

  • publicFunc3 can't see myPrivateFunction <- not good
  • I would declare function() { /* uses myPrivateFunction() */ } as a private function and then just return a pointer to it:

    MyObject.prototype = (function() {
        //private helper functions
        //scoping is a pain, but doable
        function myPrivateFunction() {}
        function myPrivateFunction2() {/* uses myPrivateFunction() */ }
        function myPrivateFunction3() {/* uses myPrivateFunction() */ }
    
        return {
            publicFunc1: myPrivateFunction2
          , publicFunc2: myPrivateFunction3
        };
    })();
    
  • I know there is a "comma first" movement out there, but it looks silly to me

Other than that, this code does not look monstrous to me, if it works for you, then why not.

share|improve this answer
    
you should try going comma-first for a bit ;) – Jason Mar 21 '14 at 17:22

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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