Как еще можно оптимизировать js код?

Question

Павел @KlinDev

Интересуют различные вопросы

Как еще можно оптимизировать js код?

Я новичок в javascript. Приступил к рефакторингу кода. Исправил синтаксические ошибки. Вроде бы получилось структурировать компоненты. Предполагаю, что можно переписать многие части программы более простым способом.
Первоначальный вид кода:

class dateInput { 
   constructor() { 
    this.input = document.querySelector(".input"); 
     
    this.input.onChange = this.onChange; 
   } 
    
    onChange(event) { 
    this.inputValue = event.srcElement.value; 
    this.updateTime = new Date(); 
    return this.inputValue; 
   } 
  } 
  
  class dateRange extends dataInput { 
   constructor() { 
    this.container = document.querySelector('.containerForLastUpdateRecordAndPeriodItems'); 
   } 
 
   createItems(period) { 
    let dates = []; 
    for (i = +period.start; i < +period.end; i+= 3600000 * 168) 
    dates.push(i); 
    let periods = []; 
    for(i=0;i<date.length;i++) { 
     let date = new Date(dates[i]); 
     if (date.getDay() == 1) period = `${date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`; 
     else if (date.getDay() == 2) period[i] = `${date.setHours(-24).toLocaleDateString()} - {date.setHours(144).toLocaleDateString()}`; 
     else if (date.getDay() == 3) period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; 
     else if (date.getDay() == 4) period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; 
     else if (date.getDay() == 5) period[i] = `${date.setHours(-72).toLocaleDateString()} - {date.setHours(96).toLocaleDateString()}`; 
     else if (date.getDay() == 6) period[i] = `${date.setHours(-96).toLocaleDateString()} - {date.setHours(72).toLocaleDateString()}`; 
     else if (date.getDay() == 0) period[i] = `${date.setHours(-120).toLocaleDateString()} - {date.setHours(48).toLocaleDateString()}`; 
 
     var n = periods.length, a = periods.length; 
     do { b = false; 
      a /= 1.3; 
      if (a == 9 || a == 10) a = 11; 
      if (a < 1) a = 1; 
      for (const i=0; i < n-a; i++) 
      { if (periods[ i ] > periods[i+a]) 
       { b = true; 
        var t = periods[i+a]; periods[i+a] = periods[ i ]; periods[ i ] = t; 
       } 
      } 
     } while (a > 1 || b); 
    } 
    return periods; 
   } 
    
   renderItems(items) { 
    this.container.appendChild(element = document.createElement('div')) 
    element.innerText = `Последнее изменение: ${this.updateTime.getDate() + '.' + this.updateTime.getMonth() > 9 ? ':' '0'+this.updateTime.getMonth() : this.updateTime.getMonth()}`      
    
    items.forEach(function(item) { 
     const element = document.createElement('div', {innerText: item}); 
         this.container.appendChild(element) 
    } 
   } 
    
   onChange() { 
    renderItems(this.createItems(this.createPeriod(this.inputValue))); 
   } 
 
   createPeriod(date) { 
    let newDate = date; 
    newDate.year = newDate.year + 1; 
    return { 
    start: date, 
    end: newDate 
    } 
   } 
  } 
   
  const range = Object.create(dateRange.prototype); 
  range.constructor = range.constructor.bind(range); 
  range.constructor();

Код после внесения изменений:

window.addEventListener('load', function() {

 class dateInput {

  constructor() { 
   this.input = document.querySelector(".input"); 
   this.input.onChange = this.onChange; 
 } 
  onChange(event) { 
   this.inputValue = event.srcElement.value; 
   this.updateTime = new Date(); 
    return this.inputValue; 
  } 
} 
// расчет диапазона времени
 class dateRange extends dataInput { 

  constructor() { 
   this.container = document.querySelector('.containerForLastUpdateRecordAndPeriodItems');
  } 

  createItems(period) {
   var dates = [], 
       periods = [];

    for (var i = +period.start; i < +period.end; i+= 3600000 * 168) {
      dates.push(i);
    } 

    for (var i = 0; i < date.length; i++) { 

     var date = new Date(dates[i]),
      n = periods.length,
      a = periods.length; 

    if (date.getDay() == 1) {
         period = `${date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`; 
      } else if (date.getDay() == 2) {
          period[i] = `${date.setHours(-24).toLocaleDateString()} - {date.setHours(144).toLocaleDateString()}`;
      } else if (date.getDay() == 3) {
          period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`;
      } else if (date.getDay() == 4) {
          period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`;
      } else if (date.getDay() == 5) {
          period[i] = `${date.setHours(-72).toLocaleDateString()} - {date.setHours(96).toLocaleDateString()}`;
      } else if (date.getDay() == 6) {
          period[i] = `${date.setHours(-96).toLocaleDateString()} - {date.setHours(72).toLocaleDateString()}`;
      } else (date.getDay() == 0) {
          period[i] = `${date.setHours(-120).toLocaleDateString()} - {date.setHours(48).toLocaleDateString()}`; 
      }
     
    do {  
     b = false; 
     a /= 1.3;

      if (a == 9 || a == 10) {
          a = 11; 
       } else (a < 1) {
               a = 1;
       } 

        for (const i = 0; i < n-a; i++) {

         if (periods[i] > periods[i+a]) { 
           b = true; 
           var t = periods[i+a]; 
           periods[i+a] = periods[i]; 
           periods[i] = t; 
          } 
         } 
        } while (a > 1 || b); 
     }
     return periods;
  }
 } 

  renderItems(items) { 
   this.container.appendChild( element = document.createElement('div') ); 
   element.textContent = `Последнее изменение: ${this.updateTime.getDate() + '.' + this.updateTime.getMonth() > 9 ? : '0' + this.updateTime.getMonth() : this.updateTime.getMonth()}`; 

   items.forEach(function(item) { 
    const element = document.createElement('div', {textContent: item}); 
    this.container.appendChild(element); 
  });
 } 

  onChange() { 
   renderItems(this.createItems(this.createPeriod(this.inputValue))); 
  } 

  createPeriod(date) { 
   var newDate = date; 
    newDate.year = newDate.year + 1; 
     return { 
     start: date, 
     end: newDate 
    } 
  } 

	const range = Object.create(dateRange.prototype); 
	range.constructor = range.constructor.bind(range); 
	range.constructor(); 

});

Извеняюсь за такую "портянку". Подскажите, как еще можно оптимизировать(зарефакторить) данный js код?

Вопрос задан более трёх лет назад
415 просмотров

2 комментария

Подписаться 1 Средний 2 комментария

Pavel Shvedov @mmmaaak

и символ доллара потерялся в шаблонных строках

Написано более трёх лет назад

Павел @KlinDev Автор вопроса

Егор Скороходов, со switch действительно меньше кода получилось:

switch(date.getDay()) {
		case 1: period = `{date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`; break;
		case 2: period[i] = `{date.setHours(-24).toLocaleDateString()} - {date.setHours(144).toLocaleDateString()}`; break;
		case 3: period[i] = `{date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; break;
		case 4: period[i] = `{date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; break;
		case 5: period[i] = `{date.setHours(-72).toLocaleDateString()} - {date.setHours(96).toLocaleDateString()}`; break;
		case 6: period[i] = `{date.setHours(-96).toLocaleDateString()} - {date.setHours(72).toLocaleDateString()}`; break;
		case 0: period[i] = `{date.setHours(-120).toLocaleDateString()} - {date.setHours(48).toLocaleDateString()}`; break;
	}

Спасибо за совет)

Написано более трёх лет назад

Решения вопроса 2

3 комментария

Роман Макаров @rmakarov

Стыдно хамить

Написано более трёх лет назад
0xD34F @0xD34F Куратор тега JavaScript

Роман Макаров, здесь нет никакого хамства. Есть свинская наглость автора вопроса - утверждение "исправил ошибки" сопровождается демонстрацией кода, этими самыми ошибками набитого. Подобие подобия адекватной реакции на данную ситуацию уже находится далеко за рамками дозволенного здесь поведения... Так что я высказался предельно мягко.

Ну и это, по существу вопроса есть что сказать? Если нет - буду весьма признателен, если вы в дальнейшем воздержитесь от написания малоосмысленных комментариев.

Написано более трёх лет назад
Павел @KlinDev Автор вопроса

Спасибо за конструктивную критику)

Написано более трёх лет назад

1 комментарий

marsdenden @marsdenden
и еще можно убрать лишний цикл, вместо
for (var i = +period.start; i < +period.end; i+= 3600000 * 168) { dates.push(i); } for (var i = 0; i < date.length; i++) { var date = new Date(dates[i]),

сделать
let period=[]; for (let dt = +period.start; dt < +period.end; dt+= 3600000 * 168) { let date = new Date(dt),

тогда надо вместо
period[i] = `${dateString1} - ${dateString2}`;
использовать
period.push(`${dateString1} - ${dateString2}`);

в общем - нет предела совершенству
Написано более трёх лет назад

Пригласить эксперта

Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы

JavaScript

+2 ещё

Простой
Почему не выводин информацию?
- 1 подписчик
- час назад
- 33 просмотра
0

ответов
JavaScript

Средний
Как добавить required к форме с выпадающим js календарём?
- 1 подписчик
- 2 часа назад
- 21 просмотр
1

ответ
JavaScript

Простой
Slick slider, как центрировать один слайд?
- 1 подписчик
- вчера
- 42 просмотра
1

ответ
JavaScript

Простой
Где найти датапикер такой?
- 1 подписчик
- вчера
- 109 просмотров
2

ответа
JavaScript

Простой
Fullcalendar. Как сменить тему?
- 1 подписчик
- вчера
- 66 просмотров
0

ответов
JavaScript

Простой
Как сделать валидацию очень сложной формы?
- 1 подписчик
- вчера
- 184 просмотра
2

ответа
JavaScript

+2 ещё

Простой
Как сделать анимацию атаки в игре на JS?
- 2 подписчика
- 06 февр.
- 403 просмотра
1

ответ
JavaScript

+2 ещё

Простой
Как можно продолжать обучение?
- 1 подписчик
- 06 февр.
- 1170 просмотров
3

ответа
JavaScript

Простой
Есть ли рабочая реализация алгоритма HDBSCAN на javascript?
- 1 подписчик
- 06 февр.
- 43 просмотра
0

ответов
JavaScript

+1 ещё

Простой
Есть ли адекватный способ расположить текст по фигуре?
- 1 подписчик
- 06 февр.
- 87 просмотров
0

ответов
Показать ещё Загружается…

Бэкэнд-разработчик JavaScript

Wanted. • Москва

от 250 000 до 400 000 ₽

JavaScript FullStack разработчик

Rocket • Смоленск

от 80 000 до 130 000 ₽

Senior Frontend (JavaScript) разработчик

Vital Partners

от 350 000 до 420 000 ₽

Добавить страницу в готовый WebApp

08 февр. 2025, в 18:10

2000 руб./за проект

Доработка мобильного приложения на flutter

08 февр. 2025, в 17:43

30000 руб./за проект

Генерация изображений в ИИ + дизайн

08 февр. 2025, в 17:41

4000 руб./за проект

и символ доллара потерялся в шаблонных строках
Егор Скороходов, со switch действительно меньше кода получилось:
switch(date.getDay()) { case 1: period = `{date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`; break; case 2: period[i] = `{date.setHours(-24).toLocaleDateString()} - {date.setHours(144).toLocaleDateString()}`; break; case 3: period[i] = `{date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; break; case 4: period[i] = `{date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`; break; case 5: period[i] = `{date.setHours(-72).toLocaleDateString()} - {date.setHours(96).toLocaleDateString()}`; break; case 6: period[i] = `{date.setHours(-96).toLocaleDateString()} - {date.setHours(72).toLocaleDateString()}`; break; case 0: period[i] = `{date.setHours(-120).toLocaleDateString()} - {date.setHours(48).toLocaleDateString()}`; break; }

Спасибо за совет)

Answer 1 · 2018-08-23 02:51:09

Исправил синтаксические ошибки.

Ага, на пять с плюсом.

Пропущен if или условие лишнее:

} else (date.getDay() == 0) {

} else (a < 1) {
Лишнее : после ?:

this.updateTime.getMonth() > 9 ? : '0'
Перед методом renderItems стоит лишняя фигурная скобка, а после метода createPeriod такой скобки не хватает.

И это только синтаксические. Есть ещё ворох иных.

Имена свойств, содержащих обработчики событий, состоят из букв только в нижнем регистре, тут должно было быть input.onchange:

this.input.onChange = this.onChange;
Какое там первое слово в имени базового класса должно быть - date или data? Вы бы определились:

class dateInput {

class dateRange extends dataInput {
В конструкторе dateRange отсутствует вызов конструктора базового класса - не хватает super(); перед this.container = ...:
```
constructor() {
 this.container = document.querySelector('.containerForLastUpdateRecordAndPeriodItems');
```
Это ещё что за на хрен:
```
period = `${date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`;
```
Во-первых - не указан индекс, и вместо собираемого массива используется объект, переданный в функцию, вместо period должно быть periods[i].
Во-вторых- метод setHours возвращает число, а не объект даты, так вызвать toLocaleDateString не получится.
В-третьих - отсутствует $ перед фигурной скобкой при подстановке второй даты.
Присваивание значений необъявленным переменным:

b = false;

element = document.createElement('div')

Никогда так больше не делайте. Ну и бегом гуглить, почему это плохо.
Неправильно пытаетесь вызывать метод renderItems - не хватает this.:
```
onChange() {
 renderItems(this.createItems(this.createPeriod(this.inputValue)));
```
Кроме того, this тут будет вовсе не экземпляром класса - надо привязывать контекст при установке этого метода в качестве обработчика события (this.onChange.bind(this) - это в конструкторе dateInput). А ещё, поскольку метод переопределён, надо не забыть вызвать метод базового класса, иначе не будет установлено значение свойства inputValue:
```
onChange(e) {
  super.onChange(e);
  this.renderItems(...
```
Можете хоть из кожи вон вылезти, но константа своего значения не изменит, а вот TypeError при попытке выполнить хотя бы одну итерацию подобного цикла получите обязательно:

for (const i = 0; i < n-a; i++) {
Что принимает document.createElement в качестве параметров? Откройте документацию и разберитесь, чтобы такую чушь больше не сочинять:
```
const element = document.createElement('div', {textContent: item});
```
Опять проблемы со значением this:

this.container.appendChild(element);

Поскольку данная строка находится внутри коллбека forEach, который представлен обычной функцией, то экземпляр класса оказывается недоступен. Или замените обычную функцию на стрелочную, или передайте this в forEach третьим параметром.
Это ещё что за на хрен №2:
```
createPeriod(date) {
 var newDate = date;
  newDate.year = newDate.year + 1;
```
Во-первых - в метод передаётся строка, а не дата.
Во-вторых - будь там дата, скопировать с помощью оператора присваивания её бы не получилось, для объектов копируются ссылки, а не значения.
В-третьих - год получается и устанавливается совсем не так, для этого есть методы:
```
createPeriod(dateStr) {
  const start = new Date(dateStr);
  const end = new Date(dateStr);
  end.setFullYear(end.getFullYear() + 1);
  return { start, end };
}
```
Это ещё что за на хрен №3:
```
const range = Object.create(dateRange.prototype); 
range.constructor = range.constructor.bind(range); 
range.constructor();
```
Во-первых, без new конструктор вызывать нельзя.
Во-вторых - к чему такие сложности? - достаточно new dateRange;.

А уж что касается общей логики данного кода и насколько он соответствует поставленной задаче (о которой вы почему-то решили умолчать, так что здесь можно было бы говорить только в предположительном ключе), то я даже пытаться думать в эту сторону не буду. Хотя несомненно, что и тут всё очень-очень плохо - раз уж вы при написании кода накосячили как только можно и как нельзя, то почему на этапе проектирования должно было быть что-то принципиально иное?

Подскажите, как еще можно оптимизировать(зарефакторить) данный js код?

А зачем? Лучшее, что тут можно сделать - выбросить всё на хрен и попробовать написать заново, предварительно подумав, на этот раз головой вместо задницы. Не стыдно подобный мусор показывать?

Но окей, окей - метод createItems, например:

Странные непонятные числа, встречающиеся в коде - такого не надо, определяем константы, имена которых будут указывать, что это за значения:
```
const MS_IN_HOUR = 3600000;
const HOURS = 168;
```
Вместо двух циклов сделайте один - промежуточный массив dates не нужен:
```
for (let i = +period.start; i < +period.end; i += MS_IN_HOUR * HOURS) {
  const date = new Date(i);
```
"Оптимизируйте" if-else-if-... - складываем нижние значения часов в массив (тоже бы неплохо определить его отдельно, дав осмысленное имя), извлекаем их используя день недели как индекс, верхнее значение получаем добавляя сколько там надо (я так понял, разница везде одинаковая между нижним и верхним):
```
const hours = [ -120, 0, -24, -48, -48, -72, -96 ][date.getDay()];
periods.push([ hours, hours + HOURS ]
  .map(n => (date.setHours(n), date.toLocaleDateString()))
  .join(' - ')
);
```
Для обмена значениями дополнительная переменная не нужна:
```
[ periods[i + a], periods[i] ] = [ periods[i], periods[i + a] ];
```

Или, renderItems:

Четыре обращения к свойству updateTime - как-то многовато, достаточно одного, чтобы запомнить значение под коротким именем:

const ut = this.updateTime;

Вместо отдельных элементов можно вставлять разметку:

this.container.insertAdjacentHTML('beforeend',
  `<div>Последнее изменение: ${ut.getDate()}.${`${ut.getMonth()}`.padStart(2, 0)}</div>` +
  items.map(n => `<div>${n}</div>`).join('')
);

Answer 2 · 2018-08-23 05:38:31

Берем портянку if-else

if (date.getDay() == 1) {
	period = `${date.toLocaleDateString()} - {date.setHours(168).toLocaleDateString()}`; 
} else if (date.getDay() == 2) {
	period[i] = `${date.setHours(-24).toLocaleDateString()} - {date.setHours(144).toLocaleDateString()}`;
} else if (date.getDay() == 3) {
	period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`;
} else if (date.getDay() == 4) {
	period[i] = `${date.setHours(-48).toLocaleDateString()} - {date.setHours(120).toLocaleDateString()}`;
} else if (date.getDay() == 5) {
	period[i] = `${date.setHours(-72).toLocaleDateString()} - {date.setHours(96).toLocaleDateString()}`;
} else if (date.getDay() == 6) {
	period[i] = `${date.setHours(-96).toLocaleDateString()} - {date.setHours(72).toLocaleDateString()}`;
} else (date.getDay() == 0) {
	period[i] = `${date.setHours(-120).toLocaleDateString()} - {date.setHours(48).toLocaleDateString()}`; 
}

и переписываем

let arr = [[-120,48], [null,168], [-24,144], [-48,120], [-48,120], [-72,96], [-96,72]];
let day = date.getDay();
let dateString1 = arr[day][0] === null ? date.toLocaleDateString() : date.setHours(arr[day][0]).toLocaleDateString();
let dateString2 = arr[day][1] === null ? date.toLocaleDateString() : date.setHours(arr[day][1]).toLocaleDateString();
period[i] = `${dateString1} - ${dateString2}`;

Плюс, если уж вы используете es6 синтаксис (классы у вас там, шаблонные строки), то объявляйте переменные через const/let, а не var

Как еще можно оптимизировать js код?

Войдите, чтобы написать ответ

Минуточку внимания

Войдите на сайт