flor-master / kottans-homework

0 stars 0 forks source link

Фидбек на домашку №2 #5

Closed tiny-kottan closed 8 years ago

tiny-kottan commented 8 years ago

"Однострочные" arrow function могут быть упрощены. Например

let bootstrap = fs.readFileSync('./bootstrap.txt').toString().split('.').map((el)=>{
    return el.replace('\n', '');
});

Можно упустить скобки у аргумента, фигурные скобки у тела функции и return и прийти к такому:

let bootstrap = fs.readFileSync('./bootstrap.txt').toString().split('.').map( el => el.replace('\n', '') );

Опять таки, многократные вызовы функций часто принято писать с новой строки, так увеличивается наглядность chaining. Пример можно привести к такому:

let bootstrap = fs.readFileSync('./bootstrap.txt')
  .toString()
  .split('.')
  .map( el => el.replace('\n', '') );

Используй всегда где возможно const вместо let.

Следи за кодом :)

const plugin = (tree) => {
  tree.match({ content: true }, node =>
...

Можно упустить скобки у tree или добавить к node. В первой половине строк в файле есть ;, а потом они начали пропадать ;)

Вот эту конструкцию необходимо упростить до одной строчки и уйти от тернарного оператора.

classList = classList.filter((el) => {
  return (bootstrapSet.has(el) ?  false : true)
});

Не лучшая идея мутировать объекты в .filter( ... ), всё таки для фильтрации его сделали

    classList = classList.filter((el) => {
      if (el.indexOf('js-') > -1){
        if (!node.attrs['data-js']) node.attrs['data-js'] = ''
        node.attrs['data-js'] += el.substr(3, el.length) + ' '
        return false
      } else{
        return true
      }
    })

Я рекомендую пересмотреть алгоритм и складировать js-* классы в отдельный массив и присваивать после обхода.

flor-master commented 8 years ago

https://github.com/flor-master/kottans-homework/commit/4bbecff9f6c0b764fefd82f7b4d942429011edf4

Наконец то доделал верстку формы) и пофиксил баги в плагине. Можно глянуть.