2011-01-14 3 views
5

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

Вот пример (с помощью jQuery рамки):

$("#tbListing").delegate("a.btnEdit", "click", function(e) { 
    var storeId = $(this).closest("tr").attr("id").replace("store-", ""), 
     storeName = $(this).closest("tr").find("td:eq(1)").html(), 
     $currentRow = $(this).closest("tr"); 

    $currentRow.addClass("highlight"); 

    $("#dialogStore") 
     .data("mode", "edit") 
     .data("storeId", storeId) 
     .data("storeName", storeName) 
     .dialog("open"); 

    e.preventDefault(); 
}); 

Я, как правило, сделать это в PHP тоже. Правильно ли, если я считаю, что это не очень эффективная память?

Редактировать: Благодарим за ответы. Вы все дали хорошие ответы. Об этой оптимизации кода сейчас. Теперь это лучше?

  $("#tbListing").delegate("a.btnEdit", "click", function(e) { 
       var $currentRow = $(this).closest("tr"), 
        storeId = this.rel, /*storing the storeId in the edit button's rel attribute now*/ 
        storeName = $currentRow.find("td:eq(1)").html(); 

       $currentRow.addClass("highlight"); 

       $("#dialogStore") 
        .data("info", { 
         "mode" : "edit", 
         "storeId" : storeId, 
         "storeName" : storeName 
        }) /*can anyone confirm that overusing the data isn't very efficient*/ 
        .dialog("open"); 


       e.preventDefault(); 
      }); 
+1

повторять $ (это) .closest («tr») можно было бы избежать;) –

+0

Я не хочу, чтобы комментарии понравились: Эй, вы делаете это неправильно, потому что вы сохраняете информацию в атрибутах «id» или что-то в этом роде, или вы должны хранить информацию только один раз в данных с помощью объекта. Я только хочу знать, насколько это плохо для памяти, доступной памяти/браузера. – Cybrix

+0

@ Каспар, да, я знаю об этом. Я мог бы просто сохранить '$ (this) .closest (" tr ")' в переменной, поэтому jQuery не нужно запускать DOM каждый раз. : P – Cybrix

ответ

12

К сожалению, вы спрашиваете, нормально ли объявлять переменные, даже если вы используете их один раз?

Абсолютно! Это делает код в миллион раз более удобочитаемым, если вы правильно назовете что-либо с переменной. Читаемость должна быть вашей главной задачей. Эффективность памяти должна быть только проблемой, если она окажется проблематичной.

Как сказал Кнут

Мы должны забыть о небольших эффективности, скажем, около 97% времени: преждевременная оптимизация есть корень всех зол.

Если вы просите больше о объявлении переменных в начале функции, а не там, где они сначала используется, то Emmett has it right - Крокфорд рекомендует делать это в JavaScript, чтобы избежать области действия, связанные с путаницей. Стоит ли это на PHP - это сугубо субъективный вопрос, который я бы сказал, но нет ничего плохого в том, чтобы придерживаться ваших стилей кодирования PHP и JS. более


Один CS цитата (из Абельсоном и Сассмен в SICP):

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

+1

97% в 70-е годы. Теперь это больше 99.97% –

+0

, поэтому вы говорите, что вам нужно только беспокоиться об эффективности памяти, когда это происходит (потому что эффективность памяти всегда проблематична по своей природе), это плохая будущая проверка. Наверняка читаемость является второй функцией? – benhowdle89

+0

@Marco Очень хорошая точка. – Skilldrick

6

Неплохая практика.

Операторы var должны быть первыми утверждениями в теле функции.


JavaScript не имеет блок сферы, поэтому определение переменных в блоках могут запутать программистов, которые опыт работы с другими семейными языками C. Определите все переменные в верхней части функции .

http://javascript.crockford.com/code.html

+5

Не все согласны со всем, что говорит Крокфорд, не в последнюю очередь по этому конкретному вопросу. Я, конечно, этого не делаю. –

+0

@Tim, и где вы предлагаете определить переменные? – Emmett

+0

Э? В приведенном примере кода оператор 'var' * является * первой строкой в ​​теле функции. – Spudley

2

Объявление переменных на самом верху хорошая вещь, чтобы сделать. Это делает код более читаемым. В вашем конкретном примере вы можете заменить $ (this) .closest ('tr') на переменную, как предложено в комментариях, но в целом я нахожу код с описательными именами переменных в одном месте очень читаемым.

1

nah, я бы сказал, что вы делаете именно то, что нужно.

Как @Caspar говорит, вы можете упростить свой код, установив сначала $currentRow и используя это вместо $(this).closest("tr") в двух других строках. И может быть несколько других вещей, которые вы могли бы улучшить. Но установка варов в начале функции так, как вы это делали, - это очень хорошо.

Частично хорошо, потому что вы сделали это внутри функции, поэтому они являются локальными переменными, что означает, что они уходят в конце функции, поэтому проблемы с памятью не возникают.

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

+0

+1 Благодарим вас, вы указали, что они были выброшены после выполнения, потому что они являются локальной переменной. Я думал об этом, но я не был уверен. – Cybrix

+0

Я хотел бы узнать ** несколько других вещей, которые вы могли бы улучшить **. Я отредактировал свои вопросы и включил оптимизированную версию (верю). Ум взглянул на него? – Cybrix

Смежные вопросы