Разделение кода на функции

Question

Вот фрагмент кода.

Правильно ли я сделала здесь то, что создала отдельные функции для показа и скрытия элементов?

Ещё как вариант я пробовавала с cозданием функций таких как я сделала в комментарии, а до этого вообще было без отдельных вынесенных функций, в общем как правильнее делать?

const hamburger = document.querySelector('.hamburger'),
menu = document.querySelector('.menu'),
closeElem = document.querySelector('.menu__close'),
overlay = document.querySelector('.menu__overlay');


// const showMenu = () => { 
//   menu.classList.add('active');
// }

hamburger.addEventListener('click', () => showMenu(menu));
closeElem.addEventListener('click', () => hideMenu(menu));
overlay.addEventListener('click', () => hideMenu(menu));

function hideMenu(element) {
    element.classList.remove('active');
}

function showMenu(element) {
    element.classList.add('active');
}

Answer 1

Можно по-всякому. Зависит от величичны задач, от масштабности, сложности и пр. Мне нравится идти путём как в ООП, где всё - объект (опять же, если имеет смысл в рамках задачи. Если это единственный код на странице, то зачем усложнять себе жизнь?)

В итоге может быть что-то такое (псевдокод):

class Menu {
    const hamburger = document.querySelector('.hamburger'),
    const menu = document.querySelector('.menu'),
    const closeElem = document.querySelector('.menu__close'),
    const overlay = document.querySelector('.menu__overlay');

    init() {
        this.initListeners();

        this.someFunctionsExecutingHere();
        this.additionalWork();
    }
    
    initListeners() {
        this.hamburger.addEventListener('click', this.show);
        this.closeElem.addEventListener('click', this.hide);
        this.overlay.addEventListener('click', this.hide);
    }

    show() {
        this.menu.classList.add('active');
    }

    hide() {
        this.menu.classList.remove('active');
    }
}

(new Menu).init();

либо так же, но чуть сложнее. Т.к. на один click или другое действие, может быть не одна, а несколько операций, то может быть и так:

class Menu {
    const hamburger = document.querySelector('.hamburger'),
    const menu = document.querySelector('.menu'),
    const closeElem = document.querySelector('.menu__close'),
    const overlay = document.querySelector('.menu__overlay');

    init() {
        this.initListeners();

        this.someFunctionsExecutingHere();
        this.additionalWork();
    }
    
    initListeners() {
        this.hamburger.addEventListener('click', this.onHamburgerClick);
        this.closeElem.addEventListener('click', this.onCloseClick);
        this.overlay.addEventListener('click', this.onOverlayClick);
    }

    onHamburgerClick() {
        this.getDataFromServer();
        this.parseData();
        this.fillTable();
        this.show();
    }

    onCloseClick() {
        this.removeDataFromTable();
        this.collapseAllRows();
        logger.log(this.operation);
        this.hide();
    }

    show() {
        this.menu.classList.add('active');
    }

    hide() {
        this.menu.classList.remove('active');
    }
}

(new Menu).init();

Answer 2

"Правильно ли я сделала здесь то, что создала отдельные функции" - мой ответ: ДА!

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

При написании много маленьких функций, важно давать им корректные имена. Например для меня такая ваша функция:

const showMenu = () => { 
   menu.classList.add('active');
}

намного понятнее чем эта:

function showMenu(element) {
    element.classList.add('active');
}

В первом случае вы назвали showMenu и функция ничего не принимает - что для меня будет означать, что функция showMenu соостветсвует названию и он просто покажет меню. И я скорее всего не буду лезть и читать код вашей функции
Во втором случае, вы ожидаете какой-то элемент - мне не ясно, вы ожидаете любой элемент или ожидаете только элементы, которые являются меню (например если по какой-то причине на сайте несколько меню)? И мне придётся залезть в исходники функции и/или связаться с вами, чтобы точно узнать для каких целей ожидалось использование этой функции. Вот если бы функция во втором случае называлась бы showElement, то это уже не вызывает никаких вопросов и сразу понятно, что можно отдавать любой скрытый элемент и он будет показан.

А так же функция должна выполнять ровно то что написано в названии. Но если вы будете давать названия типа operation_1, operation_2 и т.д., то несмотря на свою простоту такие названия вызовут больше путаницы, чем если бы весь их код был бы написан в одном месте

Не так важно, но всё же по опыту использования разных библиотек, увидев hide и show, я бы ожидал, что там ещё и есть toggle. Конечно это можно написать в одну строку самому, потому это не так важно, но было бы неплохо такое тоже например иметь

БЛОГ НА HUSL

Разделение кода на функции

Ответы (2 шт):