Почему не удаляется событие?

Question

[fgehte]

Добрый день! С толкнулся с такой проблемой, когда программировал в функциональном стиле, проблем с удалением событий не было, сейчас практикуюсь в ООП стиле и почему-то при таком же алгоритме, событие не удаляется.
Подробнее написал в примере кода.
Примерный код(второстепенную информацию удалил):

// В функциональном стиле. В данном коде работает каждая функция и все открывается и закрывается как должно.
// При открытии модального окна навешивается обработчик на нажатие клавиши ESC и если окно открыто, то при
// нажатие ESC оно закрывается и вместе с закрытием удаляется обрабочик нажатия клавиши
	function bindModal() {
		modal.addEventListener(`click`, (evt) => {
			if (evt.target === modal) {
				closePopup()
			}
		})

		triggers.forEach(trigger => {
			trigger.addEventListener(`click`, () => openPopup())
		})
		
		closeModalBtn.addEventListener(`click`, () => closePopup())
	}

	function openPopup() {
		modal.style.display = `block`
		document.addEventListener(`keydown`, (evt) => onPopupCloseEscPress(evt))
	}
		
	function closePopup() {
		modal.style.display = ``
		document.removeEventListener(`keydown`, (evt) => onPopupCloseEscPress(evt))
	}

	function onPopupCloseEscPress(evt) {
		if (evt.key === `Escape`) {
			closePopup()
		}
	}

// В ООП стиле, код почти один в один
// В данном коде тоже все работает и даже при нажатие клавиши ESC всё закрывается, но если проверить
// и вывести в консоль event, то даже после закрытия overlay'я события продолжают выводиться в консоль.
	bindTriggers() {
		this.overlay.addEventListener(`click`, (evt) => {
			if (evt.target === this.overlay) {
				this.closeOverlay();
			}
		});

		this.close.addEventListener(`click`, () => this.closeOverlay())

		this.btns.forEach(btn => {
			btn.addEventListener(`click`, () => {
				this.openOverlay();
			});
		});
	}
	
	openOverlay() {
		this.overlay.style.display = `flex`;
		document.addEventListener(`keydown`, (evt) => this.onOverlayCloseEscPress(evt))
	}
	
	closeOverlay() {
		this.overlay.style.display = `none`;
		document.removeEventListener(`keydown`, (evt) => this.onOverlayCloseEscPress(evt))
	}

	onOverlayCloseEscPress(evt) {
		if (evt.key === `Escape`) {
			this.closeOverlay();
                        console.log(evt)
		}
	}

Answer 1

[Дмитрий Беляев]

проблем с удалением событий не было

Разве?
В обоих вариантах обработчики не будут удаляться.
В removeEventListener нужно передавать тот же самый listener, что и был передан в addEventListener.
Обратите внимание, что именно не такой же, а тот же самый.
ТО есть вот так удалится:

const listener = event => console.log(event);
element.addEventListener('click', listener);
element.removeEventListener('click', listener);

А так уже нет:

element.addEventListener('click', event => console.log(event));
element.removeEventListener('click', event => console.log(event));

ибо это 2 разные функции, не смотря на то, что делают одно и то же, и одинаковы до каждого символа.

P.S.

когда программировал в функциональном стиле

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

Comments

[Павел Корнилов]

Все по делу. Я бы только уточнил, что обработчик события, переданный в слушатель, должен быть ссылкой на функцию обработчика. Тогда эту ссылку можно будет удалить.
+1

[fgehte]

Разве?

Да ошибся, действительно не удаляется

Я правильно понимаю, что если передать как:
document.addEventListener(`click`, listener)
то в функцию/метод event попадает, т.е. его не обязательно передавать так как делал я?

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

А как должен выглядить код в таком случае?
P.s. При описание вопроса я указал, что удалил второстепенную информаци такую как конструктор, наследование и тд.

[Дмитрий Беляев]

Я правильно понимаю, что если передать как:document.addEventListener(`click`, listener)то в функцию/метод event попадает, т.е. его не обязательно передавать так как делал я?

fgehte, да, нет принципиальной разницы, сохранили Вы ссылку на функцию в переменную и потом передали аргументом, или сразу передали аргументом.

А как должен выглядить код в таком случае?
P.s. При описание вопроса я указал, что удалил второстепенную информаци такую как конструктор, наследование и тд.

ООП - это про композицию объектов и их взаимодействие через вызовы методов, наследование вторично.

ООП решение Вашего кейса выглядит примерно так

class EventSubscription {
    constructor(target, eventName, eventListener) {
        this.target = target;
        this.eventName = eventName;
        this.eventListener = eventListener;
    }

    subscribe() {
        this.target.addEventListener(this.eventName, this.eventListener);
    }

    unsubscribe() {
        this.target.removeEventListener(this.eventName, this.eventListener);
    }
}

class SubscriptionCollection {
    constructor(subscriptions) {
        this.subscriptions = subscriptions;
        this.subscription = this;
    }

    subscribe() {
        this.subscriptions.forEach(subscription => subscription.subscribe());
    }

    unsubscribe() {
        this.subscriptions.forEach(subscription => subscription.unsubscribe());
    }
}

class FilterEventListenerDecorator {
    constructor(filter, eventListener) {
        this.filter = filter;
        this.eventListener = eventListener;
    }

    handleEvent(event) {
        const {filter} = this;
        if(!filter(event)) { return; }
        return this.eventListener.handleEvent(event);
    }
}

class RunEventListener {
    constructor(runner) {
        this.runner = runner;
    }

    handleEvent() {
        this.runner.run();
    }
}

class RunnerCollection {
    constructor(...runners) {
        this.runners = runners;
    }

    run() {
        this.runners.forEach(runner => runner.run());
    }
}

class StyleSetter {
    constructor(target, property, value) {
        this.target = target;
        this.property = property;
        this.value = value;
    }

    set() {
        this.target.style.setProperty(this.property, this.value);
    }
}

class StyleSetterRunnerDecorator {
    constructor(styleSetter) {
        this.styleSetter = styleSetter;
    }

    run() {
        this.styleSetter.set();
    }
}

class SubscriberRunnerDecorator {
    constructor(subscriber) {
        this.subscriber = subscriber;
    }

    run() {
        this.subscriber.subscribe();
    }
}


class UnsubscriberRunnerDecorator {
    constructor(unsubscriber) {
        this.unsubscriber = unsubscriber;
    }

    run() {
        this.unsubscriber.unsubscribe();
    }
}

class NextTickApplyRunnerDecorator {
    constructor(runnerFactory) {
        this.runner = null;
        Promise.resolve().then(() => {
            this.runner = runnerFactory();
        });
    }

    run() {
        if(!this.runner) { return; }
        this.runner.run();
    }
}

class DocumentKeydownSubscriptionFactory {
    constructor(modal) {
        this.subscription = new EventSubscription(document, 'keydown', new FilterEventListenerDecorator(
            event => event.key === `Escape`,
            new RunEventListener(new RunnerCollection(
                new NextTickApplyRunnerDecorator(() => new UnsubscriberRunnerDecorator(this.subscription)),
                new StyleSetterRunnerDecorator(new StyleSetter(modal, 'display', ''))
            ))
        ));
    }
}

class ModalClickSubscriptionFactory {
    constructor(modal, documentKeydownSubscription) {
        this.subscription = new EventSubscription(modal, 'click', new FilterEventListenerDecorator(
            event => event.target === modal,
            new RunEventListener(new RunnerCollection(
                new UnsubscriberRunnerDecorator(documentKeydownSubscription.subscription),
                new StyleSetterRunnerDecorator(new StyleSetter(modal, 'display', ''))
            ))
        ));
    }
}

class CloseModalBtnSubscriptionFactory {
    constructor(closeModalBtn, modal, documentKeydownSubscription) {
        this.subscription = new EventSubscription(closeModalBtn, 'click', new RunEventListener(new RunnerCollection(
            new UnsubscriberRunnerDecorator(documentKeydownSubscription.subscription),
            new StyleSetterRunnerDecorator(new StyleSetter(modal, 'display', ''))
        )));
    }
}

class TriggerSubscriptionFactory {
    constructor(trigger, modal, documentKeydownSubscription) {
        this.subscription = new EventSubscription(trigger, 'click', new RunEventListener(new RunnerCollection(
            new SubscriberRunnerDecorator(documentKeydownSubscription.subscription),
            new StyleSetterRunnerDecorator(new StyleSetter(modal, 'display', 'block'))
        )));
    }
}

function bindModal(modal, triggers, closeModalBtn) {
    const documentKeydownSubscription = new DocumentKeydownSubscriptionFactory(modal);
    new SubscriptionCollection([
        new ModalClickSubscriptionFactory(modal, documentKeydownSubscription),
        new CloseModalBtnSubscriptionFactory(closeModalBtn, modal, documentKeydownSubscription),
        new SubscriptionCollection(triggers.map(trigger => new TriggerSubscriptionFactory(trigger, modal, documentKeydownSubscription)))
    ]).subscribe();
}

bindModal(modal, triggers, closeModalBtn);

ФП - это композиция чистых функций. Чистых значит, что они без побочных эффектов (вроде подписки на события или изменения стилей) и для одних и тех же аргументов всегда возвращающие один и тот же результат. Побочные эффекты выносятся в монадах (контейнерах) к краю программы и выполняются там.

[fgehte]

Дмитрий Беляев, Долго думал, что вам ответить)) В итоге, пришёл к следующему: За разъяснение вопроса спасибо большое! За предоставленный пример тоже. Но в следющей раз, когда какому-нибудь начинающему будете показывать пример кода, наподобие того что вы оставили, то велика вероятность что этот код отобъет все желание заниматься дальнейшем изучением, если не программированния то ООП так точно)))))

[Дмитрий Беляев]

fgehte, да, ООП весьма и весьма многословен, зато это обеспечивает поддерживаемость и расширяемость кода. ФП решает ту же задачу, кода будет меньше, но вот сложность освоения ФП людьми с "процедурным" мышлением гораздо выше. Однако мне лично нравится ФП стиль за свою предсказуемость и надежность, при этом сохраняя ту гибкость и расширяемость, что дает ООП.

Я набросал тот же самый пример в ФП стиле и снабдил комментариями

// Так как в ФП большая часть кода должны быть чистыми функциями
// все сайд эффекты придется выносить к краю и выполнять там
// таким "краем" будет такая обертка:
const withSideEffects = (runner) => (...args) => {
    const [returnValue, sideEffects] = runner(...args);
    sideEffects.forEach(se => se());
    return returnValue;
};

// А чтоб доносить сайд эффекты до этой функции
// их нужно держать в изменяемом состоянии
// чистая функция может менять только свое локальное состояние
// инкапсулируем эту логику в таком функторе:
const createSideEffectAccumulator = () => {
    const sideEffects = [];
    const acc = {
        with: (sideEffect) => (
            sideEffects.push(sideEffect),
            acc
        ),
        map: ([returnValue, resolvedSideEffects]) => (
            sideEffects.push(...resolvedSideEffects),
            returnValue
        ),
        resolve: (returnValue) => [returnValue, sideEffects]
    };
    return acc;
};

// подписка на событие (и отписка) - это сайд эффект
// а вот функция возвращающая парочку сайд эффектов
// созданных на основе своих аргументов - вполне себе чистая:
const subscriptionFactory = (target, eventName, listener) => [
    () => target.addEventListener(eventName, listener),
    () => target.removeEventListener(eventName, listener)
];

// изменение стиля - тоже сайд эффект,
// воспользуемся тем же приемом:
const setDisplayStyleFactory = (target, value) => () => target.style.setProperty('display', value);

// а еще для нашей задачи нужны условия
// абстрогируем их в функцию:
const condition = (predicate, truthyCallback, falsyCallback) => (...args) => (predicate(...args)
    ? truthyCallback(...args)
    : falsyCallback(...args)
);

// для falsyCallback в нашей задаче понадобится фабрика пустых аккумуляторов сайд эффектов:
const emptySideEffectAccumulatorFactory = () => createSideEffectAccumulator().resolve();

// наша логика в виде чистых функций:
const openPopup = (modal, subscribeDocumentKeydown) => () => (createSideEffectAccumulator()
    .with(setDisplayStyleFactory(modal, 'block'))
    .with(subscribeDocumentKeydown)
    .resolve()
);
const closePopup = (modal, unsubscribeDocumentKeydown) => () => (createSideEffectAccumulator()
    .with(setDisplayStyleFactory(modal, ''))
    .with(unsubscribeDocumentKeydown)
    .resolve()
);
const onPopupCloseEscPress = (modal, unsubscribeDocumentKeydown) => condition(
    event => event.key === 'Escape',
    closePopup(modal, unsubscribeDocumentKeydown),
    emptySideEffectAccumulatorFactory
);
const subscribeTriggers = (modal, triggers, subscribeDocumentKeydown) => () => triggers.reduce(
    (acc, trigger) => acc.with(subscriptionFactory(trigger, 'click', withSideEffects(openPopup(modal, subscribeDocumentKeydown)))[0]),
    createSideEffectAccumulator()
).resolve();

// соберем сайд эффекты произведенные нашим кодом
const bindModal = withSideEffects((modal, triggers, closeModalBtn) => {
    const acc = createSideEffectAccumulator();
    const [
        subscribeDocumentKeydown,
        unsubscribeDocumentKeydown
    ] = subscriptionFactory(document, 'keydown', withSideEffects(subscribeTriggers(modal, triggers, () => subscribeDocumentKeydown())))
    return (acc
        .with(subscriptionFactory(modal, 'click', withSideEffects(condition(
            event => event.target === modal,
            closePopup(modal, unsubscribeDocumentKeydown),
            emptySideEffectAccumulatorFactory
        )))[0])
        .with(subscribeTriggers(modal, triggers, subscribeDocumentKeydown))
        .with(subscriptionFactory(closeModalBtn, 'click', withSideEffects(closePopup(modal, unsubscribeDocumentKeydown)))[0])
        .resolve()
    );
});

// для выполнения нужно будет передать зависимости
bindModal(modal, triggers, closeModalBtn);

Если убрать комментарии, то в ФП варианте 75 строк кода, против 173 в ООП, но если в ООП могут разобраться многие, то вот с ФП поначалу приходится напрягать мозг.

[Дмитрий Беляев]

Bavashi, я тут на целую лекцию наговорить/написать могу... но постараюсь как то выжать основные тезисы своей позиции, надеюсь смысл останется понятен и мой посыл будет интерпретироваться с тем же смыслом, что я в него вкладываю.
Начну пожалуй с такого наблюдения, что когда говорят о 3 китах ООП (наследование, инкапсуляция и полиморфизм), неважно на собеседованиях, докладах или еще где-то, - для меня это верный признак, что ООП у людей нет и они его не особо то понимают. Я не знаю откуда пошла связь наследования, инкапсуляции и полиморфизма с ООП, но я абсолютно уверен, что это не связанные вещи, хоть и могут использоваться вместе. Я думаю, большую роль здесь играет, что люди в большинстве своем интерпретируют одни и те же термины по разному, более того многие из них способны вполне аргументировано доказать, что именно их интерпретация верная. И так как я тут обосновываю свою точку зрения "наследование в ООП вторично" (а первичны композиция объектов и их взаимодействие), пожалуй стоит, что я подразумеваю под терминами "наследование", "инкапсуляция" и "полиморфизм", начну пожалуй с последнего:

Полиморфизм - это инструмент уменьшения сложности программного кода и упрощения его поддержки путем создания абстракций переиспользуемых для разных типов данных. И я знаю как минимум 3 типа полиморфизма, хотя возможно их существует больше:
- Параметрический полиморфизм - это когда мы передаем в некоторую абстракцию свой тип данных в качестве параметра, а абстракция изменяется, подстраиваясь под этот тип данных. Иногда параметризуемый тип данных может быть ограничен некоторым контрактом. На выходе мы можем получать практически любые сущности языка - функции, классы, типы данных. В большинстве языков данный тип представлен дженериками, но могут быть и другие реализации (например темплейты в C++).
- Ad-Hoc полиморфизм - это когда мы можем представить несколько реализаций абстракции под одним именем, а компилятор сам решит какую из них использовать в каждом конкретном случае на основе некоторых признаков. Чаще всего таким признаком служат различные наборы параметров при обращении к сущности. Самый яркий пример тут - это перегрузка функций, но опять же могут быть другие варианты (например различные реализации типажа у разных типов в Rust).
- Полиморфизм подтипов - это когда мы, зная некий общий тип, можем использовать через него данные любого его подтипа. Это как раз тот тип, который подразумевают, когда говорят об ООП. Представлен как правило наследованием и/или реализацией интерфейсов, но так же может быть представлен и другими способами (например структурная типизация в TypeScript или утиная типизация в Go).
Полиморфизм встречается в большинстве парадигм - в ООП, в ФП, в процедурщине и т.д.

Наследование - это инструмент уменьшения сложности программного кода и упрощения его поддержки путем переиспользования сущностей в подтипе из супертипа. Уже из определения можно сделать вывод, что это всего лишь частная реализация полиморфизма подтипов. А еще в моем определении нет ни слова про ООП, и не просто так, многие, ошибочно (ИМХО), считают, что наследование - это детище ООП. Чтоб понять, что это не так, достаточно обратится к истории - ООП зародился в 1969 году с языком Smalltalk, а увидел свет и того позже, а наследование появилось на 2 года раньше в 1967 в языке Simula. И хотя в википедии приписывают Simula к ООП языку (сами понимаете, кто пишет википедию, там даже про JS с дуру пишут, что у него утиная типизация, не понимая ее сути), но Simula - типичный пример смеси процедурной и структурной парадигм. Хотя многие прекрасно пишут в стиле Simula на Java или C#, в процедурно-структурной парадигме, и свято верят, что это ООП, так что заблуждение не случайно.
Наследования обычно нет в ФП, ибо функции плохо наследуются друг от друга, но вот в процедурной парадигме оно вполне себе может быть, наравне с ООП.

Инкапсуляция - это инструмент уменьшения сложности программного кода и упрощения его поддержки путем (где-то я уже это писал...) сокрытия этой самой сложности. И опять таки, это не про private модификатор метода в ООП, хотя private и может преследовать своей целью инкапсуляцию, как и замыкания, как и разбиения на модули, которые экспортируют не все подряд, а только то, что явно укажет разработчик. Инкапсуляция - это про интерфейсы, есть интерфейсы доступные всем, но позволяющие сделать ограниченный набор действий с абстракцией, а есть внутренние интерфейсы, через которые абстракция управляет своим состоянием на все 100%, но использующему коду их знать не зачем, иначе он станет слишком сложным и подверженным багам. А еще инкапсуляция - это про гарантии единственного источника истины.
Инкапсуляция встречается в большинстве парадигм - в ООП, в ФП, в процедурщине и т.д.

Ну и на последок, выскажу немного своего мнения. Я считаю наследование плохой практикой. Те способы, которыми оно предлагает упростить поддержку кода, могут иметь обратный эффект, так как в неумелых руках наследование очень легко подводит разработчика к нарушению SRP из SOLID, скрывая это нарушение за цепочкой наследования. Кроме того нет адекватных способов унаследоваться от нескольких классов сразу или предоставить наследуемый класс через inversion of control и dependency injection. Вместо наследования я предлагаю использовать композицию, которая не имеет данных недостатков, но кроме того гораздо ближе по духу ООП, а так же будет понятна большему числу разработчиков, так как выглядит практически одинаково и в ООП и в ФП парадигме.