1

Добрый вечер!

Изучаю JS... и в связи с этим есть несколько вопросов:

  1. Есть функция, которая берет из объекта некоторые значения и использует их в качестве CSS свойств. Почему при установке цвета границы(border) блока, не получается использовать x-1, при

    var x = box.bg.length; ?

Приходится использовать следующий код -

block.style.border = box.border + 'px' + ' solid ' + box.bg[5];

а это, как мне кажется, не очень оптимизированный подход.

  1. Есть ли более оптимизированный вариант задания стилей для элементов, а то, на мой взгляд, пять строчек кода для пяти свойств - это грубо...

И еще, хотелось бы услышать Ваше мнение и критику если необходимо, по коду в целом.

Вот то, что я понаписал:

window.onload = function() {

    var box = {
        width: 100,
        height: 100,
        bg: ['red', 'green', 'blue', 'orange', 'aqua', 'black'],
        border: 2
    };

    function blockShow(css) {
        for(i = 0; i < box.bg.length - 1; i++) {
            var x = box.bg[i],
                a = box.bg.length,
                block = document.createElement('div');

            block.style.backgroundColor = x;
            block.style.width = box.width + 'px';
            block.style.height = box.height + 'px';
            block.style.height = box.height + 'px';
            block.style.border = box.border + 'px' + ' solid ' + box.bg[5]; // 1 вопрос
            block.style.marginBottom = box.border + 'px';

            document.getElementById('wrapper').appendChild(block);
        }
    }
    blockShow(box);

}

HTML, соответственно:

<div id="wrapper"></div>

А вот живьем - http://jsfiddle.net/Alex83/5jYmC/

2
  • 2
    я конечно понимаю что ты учишь js и всё такое, но было бы проще закинуть все стили в css с привязкой по классам, а затем в цикле создавать элементы с заведёнными классами
    – LEroyGGG
    Commented 11 янв. 2013 в 10:46
  • @Astor оба комментария перевел в один ответ.
    – lampa
    Commented 11 янв. 2013 в 11:57

4 ответа 4

2
 function blockShow(box) {
    for(var/*не забываем*/ i = 0, l = box.bg.length - 1/*кэшируем*/; i < l; i++) {
        var x = box.bg[i],
            a = box.bg.length,
            block = document.createElement('div'),
            style = block.style, // кэшируем
            wrapper = document.getElementById('wrapper'); // кэшируем

        style.backgroundColor = x;
        style.width = box.width + 'px';
        // ...

        wrapper.appendChild(block);
    }
}

первый вопрос не до конца понятен, какого результата хотите добиться?

2
  • @Spectre, спасибо. Имелось ввиду уменьшение объема CSS в JS. Хотя конечно лучшим вариантом, я думаю, будет задание стилей в CSS а не в js, делал так для тренировки и пополнения багажа знаний...
    – Astor
    Commented 11 янв. 2013 в 7:01
  • 1
    естественно, создать блок с определённым классом куда правильнее
    – Spectre
    Commented 11 янв. 2013 в 7:33
1
var box = {
   'width': '100px',
   'height': '100px',
   'border': '2px solid',
   'background': ['red', 'green', 'blue', 'orange', 'aqua', 'black'],
   'margin-bottom': '2px'
};

function blockShow(css, bg) {

   var block = document.createElement('div'),
       toStyles = 'background: '+css.background[bg]+';';

   for(var i in css) {
      if(i == 'background')
         continue;

      toStyles += i+':'+css[i]+';';
   }

   block.style.cssText  = toStyles;

   document.getElementById('wrapper').appendChild(block);

}

for(var i=0; i < 5; i++) {
   blockShow(box, i);
}

http://jsfiddle.net/5jYmC/1/ ну как-то так. По моему значение удобней хранить в объекте, т.к. их в будущем можно будет легко поменять, не затрагивая функцию. Да и функция становиться менее заточена под определенную работу и становится более свободной.

@Astor Плюсы используются для соединения строк. Про цикл: цикл перебирает все элементы массива box. По-этому нужно исключить background из перебора, т.е. если ключ равен background, то пропускаем эту итерацию цикла. И последнее: цикл я добавил просто так, просто я отделил функцию создания блока и она стала универсальней, чем ежели одна функция была бы предназначена только для создания этих пяти блоков.

Вот еще одно улучшение придумал.

Можно вместо

  if(i == 'background')
     continue;

добавить вот это в начало функции:

   var block = document.createElement('div'),
       toStyles = 'background: '+css.background[bg]+';';

   delete css.background;

Тогда получится чуть короче и менее затратнее:

var box = {
   'width': '100px',
   'height': '100px',
   'border': '2px solid',
   'background': ['red', 'green', 'blue', 'orange', 'aqua', 'black'],
   'margin-bottom': '2px'
};

function blockShow(css, bg, wrap) {

   var block = document.createElement('div'),
       toStyles = 'background: '+css.background[bg]+';';

   delete css.background;

   for(var i in css) {
      toStyles += i+':'+css[i]+';';
   }

   block.style.cssText  = toStyles;

   wrap.appendChild(block);      
}

var wrapper = document.getElementById('wrapper');

for(var i=0; i < 5; i++) {
   blockShow(box, i, wrapper);
}
2
  • 2
    обращаться в цикле к document.getElementById('wrapper') в цикле не самая лучшая практика, тем более, вы говорите >Да и функция становиться менее заточена под определенную работу и становится более свободной может wrapper тоже передавать как аргумент
    – Spectre
    Commented 11 янв. 2013 в 12:10
  • @Spectre так и document можно закешировать. Надо подправить.
    – lampa
    Commented 11 янв. 2013 в 12:13
1

Я как то привык работать с объектами в js если box у вас примитив блока я бы описал так

var box = function(){
    this.height = 100;
    this.width = 100;
    ....
    this.show = function(){
    .... 
    } 
}

//когда нужно создать блок такой
var bx1 = new box();
bx1.show();

ну и для прописания стилей отедльно свойство класса, а так в js объектDOM.setAttribute('style','все стили как в html когда на элементе прописываешь'); задать можно все стили 1 строчкой. Например

var md = document.body;
md.setAttribute('style','background-color: #ff00ff; color: #000000;');

Преобразовал в ответ по просьбе задавшего вопрос демка

4
  • @ilyas, комментарий "о преобразовании в ответ" был обращен к @lampa. К Вам был обращен следующий... На сколько я могу понять, Вы используете конструктор... Я в своем изучении до них пока не дошел, это будет следующим шагом. В Вашей демке выводится один блок, а необходимо вывести его в цикле, как показано в примере.
    – Astor
    Commented 11 янв. 2013 в 8:02
  • Я показл как вывести 1 по циклу все просто внутри цикла создаешь объект выводишь и все
    – Ильяс
    Commented 11 янв. 2013 в 8:08
  • 1
    jsfiddle.net/wr3F7 выводит 101 блоков с разными бекграундами (:
    – Ильяс
    Commented 11 янв. 2013 в 11:59
  • 1
    jsfiddle.net/ETPkX для полного счастья (:
    – Ильяс
    Commented 11 янв. 2013 в 12:10
1
window.onload = function() {

    var box = {
        width: 100,
        height: 100,
        bg: ['red', 'green', 'blue', 'orange', 'aqua', 'black'],
        border: 2
    };

    function blockShow(css) { // объявление функции здесь излишнее
    // так же как и переменной css, которая не используется совсем
    // объект box в данном случае будет виден изнутри функции 
    // нет необходимости передавать его как аргумент
        for(i = 0; i < box.bg.length - 1; i++) { 
        // здесь цикл считает длину массива на каждой итерации            
        // кроме того там происходит операция вычитания
        // это влияет на скорость, кэшируйте в переменную            
            var x = box.bg[i], // всегда давайте переменным осмысленные имена
                a = box.bg.length, // хотя бы старайтесь
                block = document.createElement('div');

            block.style.backgroundColor = x;
            block.style.width = box.width + 'px'; 
            block.style.height = box.height + 'px'; 
            block.style.height = px(box.height); 
            block.style.border = box.border + 'px' + ' solid ' + box.bg[5]; // 1 вопрос
            block.style.marginBottom = box.border + 'px';

            document.getElementById('wrapper').appendChild(block);
        }
    }
    blockShow(box);

}

Ваш ответ

Нажимая «Отправить ответ», вы соглашаетесь с условиями пользования и подтверждаете, что прочитали политику конфиденциальности.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.