Вопрос по абстрагированию JavaScript

#javascript #инспекция_кода

Всем привет. Изучаю JavaScript, читаю учебник "Выразительный JavaScript" и дошел
до интересной задачи в конце пятой главы.

Задание выглядит так:


  В качестве призовой игры напишите функцию groupBy, абстрагирующую операцию группировки.
Она должна принимать массив и функцию, которая вычисляет группу для элементов массива,
и возвращать объект, который сопоставляет названия групп массивам членов этих групп.


Я решил что буду писать прототипированный метод для массивов, чтобы в функцию массив
передавался как массив.метод()

В общем вот код, он работает прекрасно, всё как нужно отрабатывает. Принёс Вам его
на суд, что Вы скажете? Есть какие-то явные огрехи и говнокод?

Array.prototype.groupBy = function(acto) {

    var tempObj = {}; //Создаем пустой объект
    for (var i=0; i

Ответы
Ответ 1

Во-первых, добавлять перечисляемое свойство в массив - ужасная идея! Как минимум,
его следует сделать неперечисляемым. Для этого можно использовать Object.defineProperty:

Object.defineProperty(Array.prototype, "groupBy", {
  value: function (acto) {
    // ...
  },
});


Во-вторых, лучше давать переменным понятные названия. Не acto, temp, tempObj - а
keySelector, key, result.

В-третьих, у вас будут баги с такими ключами, как toString. Чтобы создаваемый пустой
объект был реально пустым - надо использовать не {}, а Object.create(null).

В-четвертых, большинство методов массива передают в callback вторым аргументом индекс
элемента в массиве, а третьим - сам массив. Возможно, вам стоит ради красоты сделать
так же - это же ничего не стоит.


Ответ 2

Забавная задачка.
Но простая, поэтому код тоже простой, оптимизировать нечего особо (разве что добавить
проверок на входные данные).
Изменять/добавлять прототипы встроенных объектов - плохо, не делайте этого.



const groupBy = (a, fn, full = {}) => a.forEach((e, _) => (_ = fn(e) + '') && (full.hasOwnProperty(_)
? full[_].push(e) : full[_] = [e])) || full;

var arrayWithTrash = [1, 2, 3, "A", "B", "C", 4, 5, 6, true, [1, 2, 3], {a: 250}],
    fn = function(elemArray) {
  if (typeof elemArray === "number") return "Number";
  else if (typeof elemArray === "string") return "String";
  else if (typeof elemArray === "boolean") return "Boolean";
  else return "Other";
};

console.info(groupBy(arrayWithTrash, fn));
.as-console-wrapper {
  min-height: 100% !important;
  top: 0;
}