2

I am working on some other developer's code and I noticed the following pattern being used in some of the JavaScript files.

var my_team = function() {
    var handleTeam = function() {
        //do some ajax 
        // and update selected DOM element
    }
    return {
        //main function to initiate the module
        init: function() {
            handleTeam();
        }
    };
}();

jQuery(document).ready(function() {
    my_team.init();
});

I am at a beginner level of JS development and learning best practices. I think the method above is called Closures? Is that correct?

What I am trying to achieve is:

<select name="players" id="player">
    <option>Mark</option>
    <option>Tom</option>
</select>
<select name="coaches" id="coach">
    <option>Mark</option>
    <option>Tom</option>
</select>

I want to be able to pass HTML id-attributes player and coach to init() to take some actions to manipulate DOM.

One way I know is that I could change init-function to accept two parameters and update handleTeam to take two too and so on.

init: function(param1, param2) {
    handleTeam(param1, param2);
}

This doesn't seem to be the best method as I won't be able to pass additional parameters later on unless I change the code above to accept more parameters in the list above.

My main goal is to make this functionality re-usable on other pages where I can choose default values or pass any parameters as required.

How can I make it to have default parameters and override those as required from any page?

Axel
  • 3,331
  • 11
  • 35
  • 58
NoNice
  • 391
  • 1
  • 2
  • 14
  • When you are initializing a team, it doesnt make any not of populating the team. Maybe you should extend it such that you can add members to a team? I am not sure if you are leaving things out, but there must be more properties related to the object. – Fallenreaper Oct 22 '17 at 17:40
  • It's new setup so once I know how to pass param1 and param2 I will write additional code. At the moment I have separate functions like function team(param1, param2) { } I call them directly like team("player_id", "coach_id"); ... – NoNice Oct 22 '17 at 17:51
  • @nonuce whats wrong with `init: handleTeam` ? – Jonas Wilms Oct 22 '17 at 18:07
  • Not sure what you mean? – NoNice Oct 22 '17 at 18:13
  • You could pass an object (e.g. `{param1: 'someValue', param2: 'otherValue'}`). – jcaron Oct 22 '17 at 20:33
  • Hi @NoNice, actually you are asking more than 1 (or even 2) questions. Please see my answer and if you miss something feel free to leave a comment. Meanwhile: [Closures](https://developer.mozilla.org/de/docs/Web/JavaScript/Closures), [IIFE](http://benalman.com/news/2010/11/immediately-invoked-function-expression/), [IIFE](https://developer.mozilla.org/en-US/docs/Glossary/IIFE). regards – Axel Oct 22 '17 at 21:40

1 Answers1

2

I think above method is called Closures? Is that correct?

Yes the pattern from the OPs snippet is a "Closure" and its also an "Immediately Invoked Function Expression (aka "IIFE").


As you asked for best practices, I made some subtle changes to reply to this. So its less important what I have implemented but more important how I did it (see inline comments).

If I get you right you want to achieve something like this (also added some stuff to functions body for illustration purpose):

var myTeam = (function( _sDefault, _oDefault ) { // my_team vs. myTeam? Naming convention for JS is CamelCase!

    // underscore prepended or appended to variable names is common use to show that a variable has private access
    var _handleTeam = function( sDefault, oDefault ) {
        console.log( sDefault );
        console.log( oDefault );
        // "cannot call"/"don't has access" to init() nor updatePlayer() 
    }
    return { // deploy public methods
        init: function( sDefault, oDefault ) {
            if ( !sDefault ) sDefault = _sDefault; // devs write: sDefault = _sDefault || sDefault;
            if ( !oDefault ) oDefault = _oDefault;
            _handleTeam( sDefault, oDefault );
        },
        updatePlayer: function() {
            console.log('updatePlayer');
        }
    };

})( 'default', {default: true} ); // pass values on IIFE

myTeam.init(); // initiate with default values
myTeam.init( 'custom', {default: false, custom: true} ); // initiate with custom values
myTeam.init(); // initiate again with default values
myTeam.updatePlayer();

It would be totally fine to take the above design pattern if it fits your needs. But I can see at least 2 caveats here.

  1. The private methods don't have access to the public ones deployed by the return value.
  2. Kinda hard to read and therefor harder to maintain.

So here is a pattern that I would prefer over the one above | also Closure and IIFE:

var myTeam = (function( _sDefault, _oDefault ) {

    // make sure that _oDefault can not be modified from outer scope
    _oDefault = $.extend({}, _oDefault); // *

    // declare variables with private access
    var _oThis = this, // most devs write "that" instead of "_oThis" like I do, you can see "self" also quite often
        _oBackup = {sDefault: _sDefault, oDefault: $.extend({}, _oDefault)}; // *
    var _handleTeam = function( sDefault, oDefault ) {
        // public methods are now also availabe to private ones
        _oThis.log( sDefault );
        _oThis.log( oDefault );
        return _oThis.updatePlayer();
    }

    // declare properties with public access
    this.setDefaults = function( sDefault, oDefault ) {
        if ( typeof sDefault === 'string' )
            _sDefault = sDefault;
        if ( typeof sDefault === 'boolean' )
            _sDefault = _oBackup.sDefault;
        if ( typeof oDefault === 'object' )
            _oDefault = $.extend({}, oDefault); // *
        if ( typeof oDefault === 'boolean' )
            _oDefault = $.extend({}, _oBackup.oDefault); // *
        return this; // make public methods chainable
    }

    this.updatePlayer = function() {
        return this.log('updatePlayer'); // make public methods chainable
    }

    this.log = function( sLog ) {
        console.log(sLog);
        return this; // make public methods chainable
    }

    this.init = function( sDefault, oDefault ) {
        _handleTeam(
            sDefault || _sDefault,
            oDefault || _oDefault
        );
        return this; // make public methods chainable
    }

    return this; // deploy everything that has public access

})( 'default', {default: true} ); // set default parameters on IIFE

// our public methods are chainable now
myTeam.init().log('initiated with default values')
      .init( 'custom', {default: false, custom: true} ).log('initiated with custom values')
      .setDefaults( false, false ).log('reseted to default values')
      .init().log('initiated reseted default values')
      .setDefaults( 'new default', {default: true, newDefault: true} ).log('set new default values')
      .init().log('initiated with new default values');

// *: if you don't know why I'm using  jQuery.extend for objects, feel free to leave a comment and I can explain...
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

Another question?

init: function(param1, param2) { handleTeam(param1, param2); }

This doesn't seem to be the best method as I won't be able to pass additional params later on unless I change code above to accept more params in the list above.

You can pass as many parameters/arguments as you like, without declaring them beforehand (use arguments instead):

init: function() {
   console.log(arguments);
   handleTeam(arguments[0], arguments[1], arguments[2]);
   // or you can do it like this as well:
   handleTeam.apply(this, arguments); // 
}
myTeam.init( 'yep', 'don't worry', 'works' )


When I read your question over and over again I guess the following mockup should be in your direction (or should at least be able to illustrate how things can work together). Working pseudocode | Closure but NO IIFE:

(function( $ ) { // sure this an IIFE again but thats not essentially to the question at this point

  var Team = function() {
    // private
    var _oThis = this,
        _oTeam = {},
        _privateHelper = function() {
          // this function can not be triggered directly from outer scope
          console.log('_privateHelper was called');
          return _oThis; // use _oThis instead of this here!!!
        },
        _get = function( sId, sIdSub ) {
          return _oTeam[sId] && _oTeam[sId][sIdSub] ? _oTeam[sId][sIdSub] : false;
        },
        _set = function( sId, sIdSub, val ) {
          _oTeam[sId][sIdSub] = val;
          return _privateHelper(); 
        };

    // public
    this.register = function() {
      for( var i = 0, iLen = arguments.length, sId; i < iLen; ++i ) {
        sId = arguments[i];
        _oTeam[ sId ] = {
          $: $('#' + sId), // #1 cache jQuery collection
          aPerson: [], // #2 cache names of each person
          sSelectedPerson: false // #3 cache name of selected person
        };
        _oTeam[ sId ].$.find('option').each(function( iEach ){
          _oTeam[ sId ].aPerson[ iEach ] = $(this).val(); // #2
        });
        this.updateSelectedPerson( sId ); // #3
      }
      return this; // for chaining | BTW: this === _oThis
    }

    this.updateSelectedPerson = function( sId ) {
      if ( _oTeam[ sId ] ) {
        _set(sId, 'sSelectedPerson', _oTeam[ sId ].$.val());
      }
      return this;
    }

    this.getSelectedPerson = function( sId ) {
      return _get(sId, 'sSelectedPerson');
    }

    this.getPersons = function( sId ) {
      return _get(sId, 'aPerson');
    }

    this.update = function( sId ) {
      if ( _oTeam[ sId ] ) {
        console.log(
          'old selected: ' + this.getSelectedPerson( sId ),
          'new selected: ' + this.updateSelectedPerson( sId ).getSelectedPerson( sId )
        );
      }
      return this;
    }

    arguments.length && this.register.apply( this, arguments );
    return this; // deploy public properties
  };

  $(function(){ // document ready

    var oTeam = new Team( 'coach', 'player' ); // would be the same as ...
    // var oTeam = new Team().register( 'coach', 'player' );
    console.log(oTeam.getPersons('coach'));
    console.log(oTeam.getPersons('player'));
    $('select').on('change.team', function(){
      oTeam.update( this.id )
    })

  });

})( jQuery ) // pass jQuery on IIFE for making save use of "$"
<h1 style="font-size:1em;display:inline">select coach and player: </h1>

<select name="players" id="player">
    <option>player Mark</option>
    <option>player Tom</option>
</select>
<select name="coaches" id="coach">
    <option>coach Mark</option>
    <option selected>coach Tom</option>
</select>

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>


Take care of parameters that have type of object

var oO = {prop: 'save???'},
    oA = [true],
    s  = 'save!',
    i  = 0,
    b  = true,
    fn = function( oO, oA, s, i, b ) {
      // every argument will get a new value
      // lets have a look if this effects the original variable that was passed
      oO.prop = 'nope!';
      oA[0]   = 'oh oh!';
      s       = 'yep save';
      i       = 999;
      b       = false;
    };

fn(oO, oA, s, i, b);

                        // initial   -> inner scope -> outer scope
console.log( oO.prop ); // 'save???' -> 'nope!'     -> 'nope!'
console.log( oA[0]   ); // true      -> 'oh oh!'    -> 'oh oh'
console.log( s       ); // 'save!'   -> 'yep save'  -> 'save!'
console.log( i       ); // 0         -> 999         -> 0
console.log( b       ); // true      -> false       -> true


Here is the best explanation about the WHY I ever found so far (short, precise, understandable, credits: @newacct):

"Objects" are not values in JavaScript, and cannot be "passed".
All the values that you are dealing with are references (pointers to objects).
Passing or assigning a reference gives another reference that points to the same object. Of course you can modify the same object through that other reference.

https://stackoverflow.com/a/16893072/3931192


So if you now have a closer look at the mockups above - thats exactly why I used jQuery utility method extend for objects that are somehow related with the outer scope (which is the case when passed as parameters - right?).

Keep it in mind - never ever forget it from now on! This can save you hours of headaches if you are a novice!

So how to circumvent this behavior:

var oO = {prop: 'make it save now'},
    oA = [true],
    fn = function( oO, oA ) {
      var o = jQuery.extend({}, oO, oA);
      console.log('log#1', o);
      o[0] = 'how save is this?';
      o.prop = 'so save now :)';
      console.log('log#2', o);
    };

fn( oO, oA );

console.log('log#3', oO, oA);
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

NOTE: Other libraries such as underscore or lodash also provide functions to achieve this.

Axel
  • 3,331
  • 11
  • 35
  • 58
  • That's great answer and I like your code comments and explanations. Thank you for taking time to share your knowledge and ideas. I will definitely be able to build upon this. Cheers! – NoNice Oct 23 '17 at 07:05
  • Appreciate to inspire you @NoNice :) – Axel Oct 23 '17 at 10:01
  • @NoNice BTW: The reason why I use `jQuery.extend` on objects is clear? – Axel Oct 23 '17 at 14:13
  • Yes I had a look at Jquery's creating plugins guide and I am sure I understand the concept. https://learn.jquery.com/plugins/basic-plugin-creation/ – NoNice Oct 23 '17 at 14:47
  • @NoNice the ressource is very very good indeed but does not catch what I mean. I'm talking about passing objects as paramaters. There is a very specific behavior in JS. Please take a look at the bottom of my edited answer. The edit starts with: "Take care of parameters that have `type of` `object`". I remember that this actually took me hours when I started to write plugins. Hope that I can save you from having headaches. Let me now if it was worth reading for you... – Axel Oct 23 '17 at 20:40
  • Great point - I would have definitely struggled with that.I will keep in mind to use extend when dealing with objects from outer scope. Thank you. – NoNice Oct 24 '17 at 08:18