Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Фомин Денис #23

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
262 changes: 247 additions & 15 deletions lego.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,146 @@
'use strict';
var QUERY_TYPES = {
SELECT: 3,
FILTER: 1,
SORT: 2,
LIMIT: 99,
FORMAT: 99
};

/**
* Сделано задание на звездочку
* Реализованы методы or и and
*/
exports.isStar = true;
exports.isStar = false;

function cloneCollection(collection) {
return collection.reduce(function (previous, current) {
current = [cloneObject(current)];

return previous.concat(current);
}, []);
}
function cloneObject(obj) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему бы в этой функции тоже не использовать reduce?

var clonedObject = {};
Object.keys(obj).forEach(function (key) {
clonedObject[key] = obj[key];
});

return clonedObject;
}
function getIntersection(listOfArrays) {
if (listOfArrays.length === 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=== не нужно

return [];
}
var firstArray = listOfArrays[0];
var anyOtherArrayContains = function (item) {
var otherArrays = listOfArrays.splice(1);
if (!otherArrays.length) {
return true;
}

return otherArrays.some(function (otherArray) {
return otherArray.indexOf(item) !== -1;
});
};

return firstArray.filter(anyOtherArrayContains);
}
function getUnion(listOfArrays) {
if (!listOfArrays.length) {
return [];
}
var union = listOfArrays[0];
listOfArrays.splice(1).forEach(function (arrayItem) {
union.concat(arrayItem.filter(function (item) {
if (union.indexOf(item) === -1) {
return true;
}

return false;
}));
});

return union;
}
function applyFiltersToCollection(filters, collection) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хм, почему применение фильтров к коллекции возвращает массив массивов? Должна же быть просто отфильтрованная коллекция?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

фильтры применяются только к исходной коллекции. возвращается массив, элемент которого - исходная коллекция с одним примененным фильтром.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть какие-то особые причины не применять их сразу все?

var filteredCollections = [];
if (filters.length === 0) {
return [cloneCollection(collection)];
}
filters.forEach(function (filterQuery) {
var filtered = filterQuery.query(collection);
filteredCollections.push(filtered);
});

return filteredCollections;
}
function getExistingSelectArguments(collection, queryArguments) {
var compatibleArguments = [];
queryArguments.forEach(function (argument) {
var allCollectionItemsHaveProperty = true;
collection.forEach(function (item) {
if (!item.hasOwnProperty(argument)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Здесь также можно воспользоваться reduce и some

allCollectionItemsHaveProperty = false;
}
});
if (allCollectionItemsHaveProperty) {
compatibleArguments.push(argument);
}
});

return compatibleArguments;
}
function orderByDefault(defaultCollection, unorderedCollection) {
var orderedCollection = [];
unorderedCollection = unorderedCollection.map(function (item) {
var idx = defaultCollection.indexOf(item);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Названия переменных лучше не сокращать, даже если это просто index)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему? idx - общепринятое сокращение для index.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var clonedObject = cloneObject(item);
clonedObject.idx = idx;
var objectWithIdx = clonedObject;


return objectWithIdx;
});
orderedCollection = unorderedCollection.sort(function (a, b) {
return a.position - b.position;
}).map(function (item) {
delete item.position;

return item;
});

return orderedCollection;
}

/**
* Выбор полей
* @params {...String}
* @returns {Object} - query-object
*/
var select = function () {
var args = [].slice.call(arguments, 0);

return {
queryType: QUERY_TYPES.SELECT,
queryArguments: args,
query: function (collection) {
var selectedFromCollection = [];
collection.forEach(function (item) {
var selectedItem = {};
args.forEach(function (property) {
if (item.hasOwnProperty(property)) {
selectedItem[property] = item[property];
}
});
selectedFromCollection.push(selectedItem);
});

return selectedFromCollection;
}
};

};

/**
* Запрос к коллекции
Expand All @@ -13,58 +149,130 @@ exports.isStar = true;
* @returns {Array}
*/
exports.query = function (collection) {
return collection;
};
var collectionCopy = cloneCollection(collection);
var args = [].slice.call(arguments, 1);
var selectArguments = [];
var nonSelectArgs = [];
args.forEach(function (queryFunction) {
if (queryFunction.queryType === QUERY_TYPES.SELECT) {
var compatibleArguments = getExistingSelectArguments(
collection, queryFunction.queryArguments);
if (compatibleArguments.length > 0) {
selectArguments.push(compatibleArguments);
}
} else {
nonSelectArgs.push(queryFunction);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ В агрументы пушится функция?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

да, пушится функция для запроса (например, sortby, limit), но не ее результат.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь был тонкий намек на названия соответствующих переменных)

}
});
args = nonSelectArgs;
if (selectArguments.length > 0) {
var selectArgumentsIntersection = getIntersection(selectArguments);
args.push(select.apply(this, selectArgumentsIntersection));
}
args.sort(function (a, b) {
return a.queryType - b.queryType;
});
args.forEach(function (queryFunction) {
collectionCopy = queryFunction.query(collectionCopy);
});

/**
* Выбор полей
* @params {...String}
*/
exports.select = function () {
return;
return collectionCopy;
};
exports.select = select;

/**
* Фильтрация поля по массиву значений
* @param {String} property – Свойство для фильтрации
* @param {Array} values – Доступные значения
* @returns {Object} - query-object
*/
exports.filterIn = function (property, values) {
console.info(property, values);

return;
return {
queryType: QUERY_TYPES.FILTER,
query: function (collection) {
return collection.filter(function (item) {
if (values.length === 0 || !item.hasOwnProperty(property) ||
values.indexOf(item[property]) !== -1) {
return true;
}

return false;
});
}
};
};


/**
* Сортировка коллекции по полю
* @param {String} property – Свойство для фильтрации
* @param {String} order – Порядок сортировки (asc - по возрастанию; desc – по убыванию)
* @returns {Object} - query-object
*/
exports.sortBy = function (property, order) {
console.info(property, order);
var SORT_DIRECTION = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не понял, зачем здесь эта константа? Учитывая, что поле ASCENDING даже не используется. Я бы избавился от неё.

ASCENDING: 'asc',
DESCENDING: 'desc'
};

return {
queryType: QUERY_TYPES.SORT,
query: function (collection) {
var newCollection = cloneCollection(collection);
var orderMultiplyer = order === SORT_DIRECTION.DESCENDING ? -1 : 1;

return;
return newCollection.sort(function (a, b) {
return orderMultiplyer * (String(a[property])).localeCompare(String(b[property]));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Здесь можно учитывать, что значения, по которым происходит сортировка - числа, поэтому можно обойтись без сравнения строк

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему обязательно числа? в объектах и строки могут быть.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Какой у тебя в этом случае будет порядок для чисел 1, 2, 10, 100? Лексикографический порядок для них - 1,10,100,2.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обзательно ли здесь приведение к строке? Кажется, localeCompare сделаем всё за нас.

});
}
};
};

/**
* Форматирование поля
* @param {String} property – Свойство для фильтрации
* @param {Function} formatter – Функция для форматирования
* @returns {Object} - query-object
*/
exports.format = function (property, formatter) {
console.info(property, formatter);

return;
return {
queryType: QUERY_TYPES.FORMAT,
query: function (collection) {
var formatted = [];
collection.forEach (function (item) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лишний пробел. Плюс тут тоже можно reduce.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.assign() нельзя

var newItem = Object.assign({}, item);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Зачем столько копирований? Исходная коллекция и так уже не будет никак изменена, потому что она копируется у тебя ещё при выполнении query

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если бы format исполнялся перед select или sortBy, то элемент в коллекции, передаваемой в запрос был бы отформатирован.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Но ты ведь сортируешь запросы, как такое может произойти?

if (newItem.hasOwnProperty(property)) {
newItem[property] = formatter(item[property]);
}
formatted.push(newItem);
});

return formatted;
}
};
};

/**
* Ограничение количества элементов в коллекции
* @param {Number} count – Максимальное количество элементов
* @returns {Object} - query-object
*/
exports.limit = function (count) {
console.info(count);

return;
return {
queryType: QUERY_TYPES.LIMIT,
query: function (collection) {
count = Math.min(count, collection.length);

return collection.slice(0, count);
}
};
};

if (exports.isStar) {
Expand All @@ -73,17 +281,41 @@ if (exports.isStar) {
* Фильтрация, объединяющая фильтрующие функции
* @star
* @params {...Function} – Фильтрующие функции
* @returns {Object} - query-object
*/
exports.or = function () {
return;
var filters = [].slice.call(arguments, 0);

return {
queryType: QUERY_TYPES.FILTER,
query: function (collection) {
var filteredCollections = applyFiltersToCollection(filters, collection);
var united = getUnion(filteredCollections);
united = orderByDefault(collection, united);

return united;
}
};
};

/**
* Фильтрация, пересекающая фильтрующие функции
* @star
* @params {...Function} – Фильтрующие функции
* @returns {Object} - query-object
*/
exports.and = function () {
return;
var filters = [].slice.call(arguments, 0);

return {
queryType: QUERY_TYPES.FILTER,
query: function (collection) {
var filteredCollections = applyFiltersToCollection(filters, collection);
var intersection = getIntersection(filteredCollections);
intersection = orderByDefault(collection, intersection);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Функции js позволяют фильтровать так, чтобы не приходилось восстанавливать порядок. Подумай, как это можно сделать.


return intersection;
}
};
};
}