Что не так с модулем?

Question

Задавала вопрос о том, что лучше использовать: простой код или модули при написании простого скрипта для страницы. На тостере ответили, что модули, но мой модуль ужасен.

О модулях больше всего понравилось описание тут.

Вопрос: что не так с моим модулем?

Пример для простого аккордеона:

"use strict";
$(document).ready(function(){
  if($('.accordion').length){
    accordion.init();
  }
});
var accordion = (function(){
var _openContent = function(el){
var
    wrap = el.closest('.accordion'),
    item = el.closest('.accordion__item'),
    items = wrap.find('.accordion__item'),
    content = item.find('.accordion__content'),
    contents = wrap.find('.accordion__content'),
    dur = 500;

if(!item.hasClass('on')){
  items.siblings().removeClass('on');
  item.addClass('on');    

  contents.stop(true,true).slideUp(dur);
  content.stop(true,true).slideDown(dur);
} else {
  item.removeClass('on');
  content.stop(true,true).slideUp(dur);
}

}
// Инициализация

  return {
    init: function(){
  $('.accordion__trigger').on('click', function(e){
    e.preventDefault();

    var
    $this = $(this);

    _openContent($this);
  });
}

}

})();

*,
*:before,
*:after {
  box-sizing: border-box;
}

html,
body {
  margin: 0;
  padding: 0;
}

a {
  text-decoration: none;
  color: inherit;
  transition: all .27s ease-in-out;
}

a:hover {
  color: tomato;
}

ul {
  padding: 0;
  margin: 0;
  list-style-type: none;
}

body {
  font-family: 'Raleway', sans-serif;
  font-size: 1rem;
  line-height: 1.5rem;
  color: #727272;
}

.accordion {
  width: 100%;
  margin: 2rem auto;
  padding: 0 2rem;
}

.accordion__item {
  box-shadow: 0 2px 7px rgba(0,0,0,0.17);
}

.accordion__title {
  text-transform: uppercase;
  margin: 0;
}

.accordion__trigger {
  display: block;
  color: #fff;
  padding: .5rem 1rem;
  background: #9c27b0;
  box-shadow: 0 2px 7px rgba(0,0,0,0.27);  
  padding-left: 3.5rem;
  position: relative;
}

.accordion__trigger:before {
  content: '+';
  position: absolute;
  left: 0;
  top: 0;
  bottom: 0;
  width: 2rem;
  text-align: center;
  line-height: 2.5rem;
  background: #ff4051;
  font-weight: 800;
  transition: all .34s ease-in-out;
}

.on .accordion__trigger:before {
  content: '-';
}

.accordion__trigger:after {
  content: ' ';
  position: absolute;
  left: 2rem;
  top: 0;
  bottom: 0;
  width: 0;
  height: 0;
  border-top: 1.25rem solid transparent;
  border-bottom: 1.25rem solid transparent;
  border-left: .5rem solid #ff4051;
}

.accordion__trigger:hover {
  color: #fff;
  background: #7b1fa2;
}

.accordion__content {
  padding: 1.5rem 3.5rem;
  display: none;
}

.accordion__content span {
  display: block;
  text-transform: uppercase;
  color: #212121;
  font-weight: 700;
  margin-bottom: 1rem;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<div class="accordion">
  <ul class="accordion__list">
    <li class="accordion__item">
      <a href="#" class="accordion__trigger">
        <h5 class="accordion__title">Lorem ipsum 1.</h5>
      </a>

      <div class="accordion__content">
        <p>
          <span>Content 1. </span>
          Lorem ipsum dolor sit amet, consectetur adipisicing elit. Inventore saepe nulla iure cupiditate libero accusantium deleniti in, quisquam quae quidem.
        </p>
      </div>
    </li>
    <li class="accordion__item">
      <a href="#" class="accordion__trigger">
        <h5 class="accordion__title">Lorem ipsum 2.</h5>
      </a>

      <div class="accordion__content">
        <p>
          <span>Content 2. </span>
          Lorem ipsum dolor sit amet, consectetur adipisicing elit. Inventore saepe nulla iure cupiditate libero accusantium deleniti in, quisquam quae quidem.
        </p>
      </div>
    </li>
    <li class="accordion__item">
      <a href="#" class="accordion__trigger">
        <h5 class="accordion__title">Lorem ipsum 3.</h5>
      </a>

      <div class="accordion__content">
        <p>
          <span>Content 3. </span>
          Lorem ipsum dolor sit amet, consectetur adipisicing elit. Inventore saepe nulla iure cupiditate libero accusantium deleniti in, quisquam quae quidem.
        </p>
      </div>
    </li>
  </ul>
</div>

Answer 1

Пару небольших замечаний:

1. Использовать один стиль для кавычек (либо одинарные, либо двойные). "use strict"; у вас в двойных, а остальных местах - одинарные
2. Для ; тоже выбрать один стиль. Если ставите везде, то у вас пропущена после закрытия функции (Function Expression) var _openContent = function(el){ /* ... */ }; и тут return { /* ... */ };

3. Вы используете в модуле $ - внешнюю библиотеку, её лучше передавать в сам модуль как параметр:

   var accordion = (function($){ /* ... */ })($);

Сразу понятно, что для использования вашей библиотеки нужна библиотека jQuery. При использовании других зависимых библиотек, их так же можно передавать как параметр в модуль.

4. "use strict"; лучше перенести непосредственно в модуль, что бы он распространялся именно на него var accordion = (function($){ 'use strict'; /* ... */ })($);

5. Думаю, было бы неплохо, если бы в модуль можно было передавать параметры (wrap, item, dur и т.д.) А значения, которые у вас есть сейчас, оставить по умолчанию. Выглядело бы это как то так:

---

// в init передаем объект с параметрами
accordion.init({
  dur: 1000
});
// ...
_openContent($this, setting); // передаем эти параметры в _openContent
// ...
var _openContent = function(el, setting){
  setting = setting || {}; // по умолчанию пустой объект
  var dur = setting.dur || 500; // -> 1000
  var wrapSelector = setting.wrapSelector ||'.accordion';  // -> '.accordion'

6. Рекомендация (как и все остальные пункты), но тут уже дело вкуса. Если значение переменной является jQuery-объектом, то имя переменной можно начинать с $ - var $container = $('.container'). Тогда в коде сразу понятно (например, по вашему примеру, тут: contents.stop(true,true)), что мы работаем в данном месте с jQuery-объектом, а не с обычным js-объектом, у которого есть метод stop. В процессе написания кода, рефакторинга или поиска ошибок становится ясно, с чем работаем (возможно и не сразу, но я думаю вы поняли о чем я)

Для первых двух пунктов я рекомендую использовать eslint (или другие синтаксические анализаторы кода), которые сразу будут подчеркивать (выводить) такие ошибки.

И если ещё не сформировался свой стиль написания кода или когда работаете в команде, удобно использовать единый стиль, например Google JavaScript Style Guide или StandartJS (по которому не нужно ставить точку с запятой). Сам придерживаюсь Airbnb JavaScript Style Guide (на русском) - очень хорошо написан.