Skip to content

Кулешова Ирина#53

Open
trtz wants to merge 6 commits intourfu-2016:masterfrom
trtz:master
Open

Кулешова Ирина#53
trtz wants to merge 6 commits intourfu-2016:masterfrom
trtz:master

Conversation

@trtz
Copy link
Copy Markdown

@trtz trtz commented Nov 10, 2016

No description provided.

@honest-hrundel
Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link
Copy Markdown

🍏 Пройдено тестов 9 из 9

Comment thread emitter.js
off: function (event, context) {
console.info(event, context);
var eventsToOff = Object.keys(this._events).filter(
function (key) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

давай переименуем key

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

и еще эту строку можно к предыдущей приписать, так обычно пишут

Comment thread emitter.js
function (key) {
return key === event ||
(key.length > event.length &&
key.substr(0, event.length + 1) === event + '.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

вроде из условий

(key.length > event.length && key.substr(0, event.length + 1) === event + '.')

достаточно написать последнее

Comment thread emitter.js
key.substr(0, event.length + 1) === event + '.');
}
);
for (var i = 0; i < eventsToOff.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

почему бы не использовать forEach?

Comment thread emitter.js
emit: function (event) {
console.info(event);
var events = getAllEvents(event).reverse();
for (var i = 0; i < events.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

и тут вроде forEach хорошо впишется, тогда не придется создавать новую переменную строчкой ниже

Comment thread emitter.js


function getAllEvents(event) {
var splitted = event.split('.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Название не отражает, что лежит в переменной

Comment thread emitter.js
*/
emit: function (event) {
console.info(event);
var events = getAllEvents(event).reverse();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

если ты сплитишь event, а потом реверсишь, то почему бы сразу не пойти с конца?

Comment thread emitter.js
);
for (var i = 0; i < eventsToOff.length; i++) {
this._events[eventsToOff[i]] = this._events[eventsToOff[i]].filter(
function (item) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

тут item переименовать надо

@bazhenova
Copy link
Copy Markdown

🍅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants