14

I have been following the Modular Design Pattern for quite some time now and find it extremely useful as it helps in the well maintenance of code & separation of blocks into modules.

Regular usage of the module structure with jQuery has led to most of my applications/code following the below structure:

(function() {
    var chat = {
        websocket: new WebSocket("ws://echo.websocket.org/"),
        that: this,
        init: function() {
            this.scrollToBottom();
            this.bindEvents();
            this.webSocketHandlers();
        },
        bindEvents: function() {
            this.toggleChat();
            this.filterPeople();
            this.compose();
        },
        elements: {
            indicator: $(".indicator"),
            statusText: $(".status-text"),
            chatHeadNames: $(".people li .name"),
            filterInput: $("#filter-input"),
            msgInput: $("#msg-input"),
            sendBtn: $(".send")
        },
        ...
        ...
        ...
        filterPeople: function() {
          var that = this;
          this.elements.chatHeadNames.each(function() {
              $(this).attr('data-search-term', $(this).text().toLowerCase());
          });
        },
        ...
        ...
        };

        chat.init();
})();

What I would like to know is whether referencing all my elements via jQuery as part of a single variable chat.elements is a good practice?

One part of me tells that it indeed is a good way to reference all your selectors at once and cache them in variables so that multiple usages of the same element can be done with the cached variables (instead of multiple DOM selections).

Another part of me tells that this might be an anti-pattern and specific elements should be selected and cached locally when required.

I have used similar structures throughout and have got mixed responses about the code, but nothing solid. Any help would be appreciated. Thanks!

Community
  • 1
  • 1
nashcheez
  • 5,067
  • 1
  • 27
  • 53
  • using element to select name and status or other things is not good idea whats happend if somebody change the name with inspect element? or tomorrow your teammate changing the theme everything will be broken! – masoud soroush Mar 03 '17 at 00:39
  • keep data in arry or object and for storing use localstorage – masoud soroush Mar 03 '17 at 00:45
  • The element properties get parsed at page load and events are bound directly to the node, so if somebody changes the name with inspect element, it won't cause an issue. And `localstorage` is a bad idea for this. – nashcheez Mar 03 '17 at 10:05

2 Answers2

6

Caching the selectors is a good thing. Hanging on to them is a good idea. It improves performance over repeatedly querying the DOM for the same thing. The code you have above looks very similar to BackboneJS and MarionetteJS code.

I do have some warnings for you though:

  1. This pattern could cause memory leaks. Consider the case where you destory a subview, but you keep a reference to something that selected it. This is called a dangling pointer. The view won't really disappear. All bindings will remain. Events will continue to fire behind the scenes.
  2. You will eventually run into a bug where you decided to re-render part of your screen. Cleaning up all your bindings is then required and you need to remember to delete and selectors. If you don't do this you will almost certainly run into issues where you wonder why an event is indeed firing but nothing appears to happen on screen.... (this will be because its happening off screen, to the element that you tried to delete, that still exists... sorta).
  3. The current way you are querying for elements causes searches against the entire page. Take a look at https://api.jquery.com/find/. If you cache one selector and then perform searches within that selector it may gain you a little performance bump.
Parris
  • 17,833
  • 17
  • 90
  • 133
  • 1. What if I am not using a framework like Backbone which usually is the case, then no subviews are destroyed & no scenarios of a dangling pointer. 2. Events unless delegated are bound directly to the DOM node, so if in case I delete an element node, the event bound to is automatically detached. 3. I do use `jQuery.find` on my cached selectors to search children elements. – nashcheez Mar 06 '17 at 08:55
  • @nashcheez you don't need to use a framework, you just need to build your own system that takes care of the clean-up (if needed). There are plenty of apps that don't need to handle any sort of clean-up. Anyways, none of that matters, it is still a good pattern. If you do need to clean-up, it makes it easier to find what to clean-up. – Parris Mar 07 '17 at 02:37
  • Sure thanks for your response. I will do my little bit of research on the same. :) – nashcheez Mar 07 '17 at 07:39
  • I would only add to above answer one thing: lazy select (about which part of the question was could be done through e.g. getter - and access would be still available through property), but it would introduce additional complexity under the hood (in scope of 2. is be even worse). IMHO potential benefit may not be worth it but your mileage may vary. – gaa Mar 08 '17 at 14:05
1
  • I think, If the chat module has selectors only for its children, then it's a good pattern. Like:
<div id="chat-module">
  <div class="indicator">...</div>
  <div class="status-text">...<div>
  ...
</div>
<script src="and your chat module.js"></script>
// your chat module selecting .indicator:
// $('#chat-module.indicator')
  • Also, add a shut-down function to your module. So, when you remove it from the view (as in a single-page-app), you can nullify your selectors and detaching event handlers, like: delete this.elements.indicator and event detaching code.

There are also other/better patterns for this, like, when a user types something, you fire an event, and catch that event in your module. To separate UI and the code.

Inanc Gumus
  • 25,195
  • 9
  • 85
  • 101