это ключевое слово в шаблоне модуля?

Я только начал работать в новой компании и заметил что-то, что мне кажется совершенно неправильным во многих их JS. Я немного не решаюсь поднимать этот вопрос, не подтверждая, что это неправильно, так как я довольно молодой, я не эксперт JS, и это только мой второй день, и я не хочу выглядеть глупо.

Итак, обычно я ожидаю, что шаблон модуля будет выглядеть примерно так:

MODULENAME = MODULENAME || {};

MODULENAME.SUBMODULENAME = (function() {
    var bla = {};

    bla.somefunction = function() {
        //do stuff
    };

    //add more stuff to bla
    return bla;
}());

Что у них есть по всему коду:

MODULENAME = MODULENAME || {};

MODULENAME.SUBMODULENAME = (function() {
    var that = this;

    that.somefunction = function() {
        //do stuff
    };

    //add more stuff to that
    return that;
}());

Теперь, конечно, поскольку функция не вызывается как конструктор с ключевым словом new или как метод, this привязана к window, и они определяют that как this. Таким образом, они в основном сбрасывают все в глобальный объект, и все имена их подмодулей на самом деле являются псевдонимами для window. Есть ли причина, по которой кто-то хотел бы это сделать? Или это действительно так неправильно, как мне кажется?

Изменить:

Я сделал ошибку, поставив var перед определением подмодуля, изначально я написал что-то немного другое и забыл удалить var. Я также попытался сделать пример немного яснее, надеюсь, теперь более очевидно, что я имею в виду.

Редактировать 2:

Также я просмотрел сценарии, выполняемые в Firebug, и они определенно добавляют все к window, этот объект — полный беспорядок.


javascript design-patterns global-object
person Michal    schedule 09.05.2012    source источник
comment
Извините, я не понял вашего вопроса. Не могли бы вы объяснить это лучше?   -  person Danilo Valente    schedule 09.05.2012
comment
Вы уверены, что this не ссылается на класс или элемент? Я тоже не уверен, что понимаю ваш вопрос.   -  person Fabrício Matté    schedule 09.05.2012
comment
Вы только что начали работать в новой компании и обнаружили что-то, в чем вы не уверены, и вместо того, чтобы спросить своих коллег, которые знают код, почему вы написали об этом на SO? Кажется, это плохой способ начать новую работу.   -  person Jordan Running    schedule 09.05.2012
comment
Это реальный код? или он заключен в чем-то другом? this может быть чем угодно, в зависимости от окружения (но, насколько я понимаю, this действительно относится к глобальному объекту)   -  person Joseph    schedule 09.05.2012
comment
Есть еще кое-что, что выглядит неправильно: оператор var должен быть в первой строке, а не во второй.   -  person bfavaretto    schedule 09.05.2012
comment
@FabrícioMatté да, this относится к окну. Смотрите сами   -  person Danilo Valente    schedule 09.05.2012
comment
@бфаваретто. Да, я написал это пару минут назад.   -  person gdoron is supporting Monica    schedule 09.05.2012
comment
@gdoron Извините, я еще не видел вашего ответа, когда добавил этот комментарий.   -  person bfavaretto    schedule 09.05.2012
comment
Вы уверены, что код не заканчивается на }.call({}));?   -  person cliffs of insanity    schedule 09.05.2012
comment
Все добавляется в окно, я проверил с помощью Firebug. И нет, это не заканчивается на }.call({}));.   -  person Michal    schedule 09.05.2012
comment
вы уверены, что они добавляют все методы в this/that? Это восхитительно. Пожалуйста, опубликуйте вскрытие :) Единственная причина, по которой вы это сделаете, - поддержка небраузерных сред, добавление методов к глобальному объекту, который может не быть window.   -  person Ricardo Tomasi    schedule 09.05.2012
comment
Если зарплата того не стоит, предлагаю найти новую работу... Плохое программирование как болезнь.   -  person gdoron is supporting Monica    schedule 09.05.2012
comment
Итак... теперь, когда ты знаешь, что они отстой, что ты собираешься делать...? :)   -  person gdoron is supporting Monica    schedule 09.05.2012
comment
Глядя на историю репо, похоже, что один человек несет ответственность за весь код, в котором есть эта ошибка, поэтому я не думаю, что это системная проблема в компании. Думаю, я поговорю с этим человеком завтра.   -  person Michal    schedule 09.05.2012

Ответы (2)


arrow_upward
3
arrow_downward

Да выглядит неправильно.

MODULENAME = MODULENAME || {}; // missing var

var MODULENAME.SUBMODULENAME = (function() { // probably the missing var from above...
    var that = this;
    //add some stuff to that
    return that; // that is the WINDOW- wrong.
}());

DEMO за ущерб, который он может нанести:

var x = function() {
    alert('out');
}
var MODULENAME = MODULENAME || {};

MODULENAME.SUBMODULENAME = (function() {
    var that = this;
    that.x = function() {
        alert('DAMAGE');
    }
}());

x();​ // alert DAMAGE and not "out" - messed up with the global object!
person gdoron is supporting Monica    schedule 09.05.2012
comment
@МаркЛинус. Они хотят загрязнить глобальный объект?! Я считаю, что нет. В любом случае, если вы хотите сделать что-то не так, и вы сделали что-то не так - это все равно неправильно... :) - person gdoron is supporting Monica; 09.05.2012
comment
Объявление глобальных переменных является хорошей практикой, но здесь это не имеет значения. - person RobG; 09.05.2012
comment
@РобГ. Но этот код не обязательно должен быть в глобальном объекте... он может быть и во внутренней области видимости. - person gdoron is supporting Monica; 09.05.2012

arrow_upward
0
arrow_downward

Шаблон модуля используется неправильно, и это одна из причин, по которой не следует использовать функциональные выражения, если их использование ничего не дает по сравнению с объявлением функции. Если намерение состоит в том, чтобы создать глобальные функции (в чем я сомневаюсь), то они должны использовать:

function somefuncion() {
  ...
}

Если их намерение состоит в том, чтобы добавить свойства (в данном случае методы) к объекту, что более вероятно, тогда:

MODULENAME.SUBMODULENAME.somemethod = function() { /* do stuff */ };

Если есть необходимость условно создать методы, например. на основе обнаружения признаков может подойти следующее:

(function(global, undefined) {

  // In here global is the global object
  global.MODULENAME = global.MODULENAME || {};
  global.MODULENAME.SUBMODULENAME = global.MODULENAME.SUBMODULENAME || {};

  // and undefined is undefined, belt and braces approach
  undefined = void 0;

  // Direct assignment
  function somemethod() {
      //do stuff      
  };

  // Assign directly to the "namespace" object
  MODULENAME.SUBMODULENAME.somemethod = somemethod;

  // Conditional assignment
  if ( sometest ) {
    MODULENAME.SUBMODULENAME.anothermethod = function(){...};

  // Try another way...
  } else if (someOtherTest) {
    MODULENAME.SUBMODULENAME.anothermethod = function(){...};

  // Default
  } else {
    MODULENAME.SUBMODULENAME.anothermethod = function(){...};
  }

  // Clean up 
  global = null;

}(this));

Одна проблема с вышеизложенным заключается в том, что каждая функция, объявленная внутри внешней функции, имеет замыкание обратно к объекту функции и его среде, поэтому это немного расточительно для ресурсов. Гораздо эффективнее сохранять простоту и использовать шаблон модуля только там, где он действительно необходим, и просто использовать простые объявления функций и присваивания там, где это не так. Не такой забавный, но более практичный.

person RobG    schedule 09.05.2012