alexbuki
@alexbuki
программист js

Что подразумевается под реафкторингом кода на js и насколько правильно я его сделал для предлагаемого кода?

Есть тестовое задание.
Необходимо сделать рефакторинг кода:
function Journal(date) {
  this.date = date;

  this.formatDate = function(date) {
    return date.getDate() + '.' + (date.getMonth() + 1) + '.' + date.getFullYear();
  };

  this.getTitle = function() {
    return "Выпуск от " + this.formatDate(this.date);
  };

}

Journal.compare = function(journalA, journalB) {
  return journalA.date - journalB.date;
};

// использование:
var journals = [
  new Journal(new Date(2012, 1, 1)),
  new Journal(new Date(2012, 0, 1)),
  new Journal(new Date(2011, 11, 1))
];

function findMin(journals) {
  var min = 0;
  for (var i = 0; i < journals.length; i++) {
    if (Journal.compare(journals[min], journals[i]) > 0) min = i;
  }
  return journals[min];
}

alert( findMin(journals).getTitle() );

Если я правильно понимаю рефакторинг-это оптимизация, поэтому я по максимуму перевел все на es6 и применил метод сортровки для массива, код сократился в два раза:
function Journal(date) {
  this.date = date;
  this.formatDate = date => `${date.getDate()}.${date.getMonth() + 1}.${date.getFullYear()}`;
  this.getTitle = () => `Выпуск от ${this.formatDate(this.date)}`;
}
const journals = [
  new Journal(new Date(2012, 1, 1)),
  new Journal(new Date(2012, 0, 1)),
  new Journal(new Date(2011, 11, 1)),
];
journals.sort((a, b) => a.date - b.date);
alert(journals[0].getTitle());
  • Вопрос задан
  • 220 просмотров
Решения вопроса 1
rockon404
@rockon404
Frontend Developer
1. Ну и где ES6 классы или хотя бы прототипы?
2. В массив следовало добавить строчные значения дат и работать с ними. Так вы избавитесь от дублирования: new Journal(new Date(/* ... */)),

3. У класса Journal есть метод compare, это значит, что this.date подробности реализации, которые могут быть изменены, поэтому вместо сортировки по датам, следует использовать метод compare. Либо реализовать метод Journal.min, который уже будет использовать в реализации сортировку.

4. Рефакторинг это не обязательно сокращение кода. Например для читаемости, стоило вынести минимальную дату в переменную:
const minJournal = findMin(journals);
или:
const minJournal = Journal.min(journals);

5. Для title можно использовать getter. Это позволит, например, использовать деструктуризацию:
const { title } = minJournal;
Ответ написан
Пригласить эксперта
Ответы на вопрос 1
alexbuki
@alexbuki Автор вопроса
программист js
С учетом замечаний
class Journal {
  constructor(date) {
    this.date = date;
  }

  formatDate(date) {
    return `${date.getDate()}.${date.getMonth() + 1}.${date.getFullYear()}`;
  }

  get title() {
    return `Выпуск от ${this.formatDate(this.date)}`;
  }
}
Journal.compare = (journalA, journalB) => journalA.date - journalB.date;
Journal.min = (journals) => {
  let min = 0;
  for (let i = 0; i < journals.length; i++) {
    if (Journal.compare(journals[min], journals[i]) > 0) min = i;
  }
  return journals[min];
};

const journals = [
  new Journal(new Date(2012, 1, 1)),
  new Journal(new Date(2012, 0, 1)),
  new Journal(new Date(2011, 11, 1)),
];

const minJournal = Journal.min(journals);
const { title } = minJournal;
Ответ написан
Комментировать
Ваш ответ на вопрос

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

Войти через центр авторизации
Похожие вопросы