Hello. I study JavaScript, read the textbook "Expressive JavaScript" and reached an interesting task at the end of the fifth chapter.

The task looks like this:

As a bonus game, write the groupBy function, which abstracts the grouping operation. It should take an array and a function that computes the group for the elements of the array, and return an object that matches the names of the groups to the arrays of the members of these groups.

I decided that I would write a prototyped method for arrays so that the array would be passed to the function as an array. массив.метод()

In general, here's the code, it works fine, everything works as it should. Brought you to the court, what do you say? Are there any obvious flaws and govnokod?

 Array.prototype.groupBy = function(acto) { var tempObj = {}; //Создаем пустой объект for (var i=0; i<this.length; i++){ //Запускаем проход по массиву var temp = acto(this[i]); //Создаём переменную, куда помещаем результат вызова функции переданной аргументом. Условимся о том, что функция, передаваемая аргументом, должна возвращать строку if (!tempObj[temp]) { //Проверяем наш объект и если в нём нет свойства с таким именем, то tempObj[temp] = [this[i]]; //Создаём свойство с таким именем куда помещаем массив, в который, в свою очередь, помещаем элемент итерации } else { //Иначе tempObj[temp].push(this[i]); //в массив-свойство объекта добавляем результат операции } } return tempObj; //Возвращаем объект } 

Work example:

 Array.prototype.groupBy = function(acto) { var tempObj = {}; //Создаем пустой объект for (var i = 0; i < this.length; i++) { //Запускаем проход по массиву var temp = acto(this[i]); //Создаём переменную, куда помещаем результат вызова функции переданной аргументом. Условимся о том, что функция, передаваемая аргументом, должна возвращать строку if (!tempObj[temp]) { //Проверяем наш объект и если в нём нет свойства с таким именем, то tempObj[temp] = [this[i]]; //Создаём свойство с таким именем куда помещаем массив, в который, в свою очередь, помещаем элемент итерации } else { //Иначе tempObj[temp].push(this[i]); //в массив-свойство объекта добавляем результат операции } } return tempObj; //Возвращаем объект } var arrayWithTrash = [1, 2, 3, "A", "B", "C", 4, 5, 6, true, [1, 2, 3], { "a": 250 }]; console.log( arrayWithTrash.groupBy(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"; }) ); 
 .as-console-wrapper { min-height: 100% !important; top: 0; } 

    2 answers 2

    First, adding an enumerable property to an array is a terrible idea! At a minimum, it should be made untransferable. You can use Object.defineProperty for this:

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

    Secondly, it is better to give variable clear names. Not acto , temp , tempObj - but keySelector , key , result .

    Thirdly, you will have bugs with keys such as toString . In order to create an empty object to be really empty, you must use not Object {} , but Object.create(null) .

    Fourthly, most of the array methods pass in the callback with the second argument the index of the element in the array, and the third - the array itself. Perhaps you should do the same for the sake of beauty - it does not cost anything.

    • Interestingly, creating an object through Object.create (null) it does not have a proto property, but it does have {}. What does it mean? - Denis Ivanov
    • 1) The built-in object prototype should not be touched at all. This is the patrimony of the creators of the engine. 2) If they are in two lines, then optional. Although, of course, the thought is worthwhile. 3) What are the bugs? Own properties do not contain them, and may need a method of the prototype object. 4) Good idea :) - user207618
    • @DenisIvanov, Object.create(null) is a rare way to make an empty prototype so that even Object not in ancestors. {} - the usual creation of an object that inherits Object . - user207618
    • @Other will fail when trying to do this: tempObj["toString"].push(this[i]) - the function does not have a push method! - Pavel Mayorov
    • I do not agree. The key must be added with the value of the array. - user207618

    Funny task.
    But simple, so the code is also simple, there is nothing much to optimize (except to add input checks).
    Modifying / adding prototypes of embedded objects is bad, don't do that.

     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; } 

    • one
      huge arrays are better for chunks to process - what is better in javascript? and then make an example - Grundy
    • one
      @Grundy, processing large data may take a long time process and the browser may offer to kill it. And pieces can and will go. Example ... Well, why not? - user207618
    • @Grundy, will you? - user207618
    • Now it only remains to make a huge array and check whether there will really be a benefit from this function - Grundy
    • @Grundy, let's check it if there is a desire. - user207618