-1

I'm trying to make better use of OOP, but I don't have good instincts for this yet and I sometimes worry I'm making things more complicated and/or less efficient. I'm wondering what is the best way to handle this situation working with groups of related objects that essentially map to DOM elements...

I've written a class to represent expanding elements (like dropdown/mobile menus, Q&As, etc.) which takes some arguments to set up, and has methods to expand and collapse. Here's a simplified version for the purposes of this question (methods here just add/remove CSS classes to handle transitions, but there are more things I'm doing in my actual code with animations, keyboard input, etc. that seem worth building something modular and reusable):

class Expandable {

  constructor({ toggleButton, toggleButtonClass = "toggle-expand", expandableClass = "expandable" }) { 
    this._toggleButton = toggleButton;
    this._toggleButtonClass = toggleButtonClass;
    this._expandableClass = expandableClass; 

    this._expandableId = this._toggleButton.getAttribute('aria-controls');
    this._expandable = document.querySelector(`#${this._expandableId}`);
    this._expandable.classList.add(expandableClass);

    this._setEventListeners();
  }

  _setEventListeners() {
    this._toggleButton.addEventListener('click', (e) => {
      this._handleClick();
    });
  }

  _handleClick() {
    if(this._toggleButton.getAttribute('aria-expanded')  === 'true') this.collapse();
    else this.expand();
  } 

  expand() {
    this._toggleButton.setAttribute('aria-expanded', 'true');
    this._expandable.classList.add(`${this._expandableClass}_open`);
    this._toggleButton.classList.add(`${this._toggleButtonClass}_on`);
  }

  collapse() {
    this._toggleButton.setAttribute('aria-expanded', 'false');
    this._expandable.classList.remove(`${this._expandableClass}_open`);
    this._toggleButton.classList.remove(`${this._toggleButtonClass}_on`);
  }
}

Now I'm trying to create a class to represent a group of related Expandable objects, with a method to close everything at once. So far I have:

class ExpandableGroup {

  constructor({ toggleButtonClass = "toggle-expand", expandableClass = "expandable" }) { 
    this._buttonSet = document.querySelectorAll(`.${toggleButtonClass}`);
    this._expandables = [];

    this._buttonSet.forEach((button) => {
      this._expandables.push( 
        new Expandable({ toggleButton: button, toggleButtonClass, expandableClass })
      );
    });
  }

  closeAll() {
    // ???
  }

}

What is the best (or a non-terrible) way to implement the closeAll method using an OOP approach? I used to do this (without OOP Classes, just a mess of different functions that were redundant and hyperspecific) by using querySelectorAll() with my "open" class to get all the "open" elements, then loop through and remove that class from each. One thought I had is to have an isOpen property of the Expandable class, update it on expand/collapse, then the closeAll method of the ExpandableGroup could filter the _expandables array for any Expandable objects that have isOpen === true, and then with the results run their collapse method to close them. But is that a good approach? And how does it compare in terms of efficiency to the "old way" where I'm querying the DOM vs. filtering an array of objects?

2
  • please don't cross-post: stackoverflow.com/questions/68264750/… "Cross-posting is frowned upon as it leads to fragmented answers splattered all over the network..."
    – gnat
    Jul 6, 2021 at 5:51
  • I'm sorry, the only answer I got suggested I post here instead. I have deleted the other post. Was not intending to spam!
    – jamescodes
    Jul 6, 2021 at 6:13

2 Answers 2

3

One adagio in OOP design is

Tell, don't ask

If you follow that, then you don't ask an Expandable if they are open/expanded before telling them to close/collapse. Instead, you would just blindly tell each Expandable in the group to close/collapse and make sure in Expandable itself that collapsing an expandable that is already collapsed does nothing.

1
  • Thanks, that is a simple way to put it! I'll try to rework things with that in mind.
    – jamescodes
    Jul 6, 2021 at 17:46
0

The DOM itself applies far better to object oriented structures than most people think, at is is stateful itself. So, if you let JS-Objects create your DOM elements, you can access the DOM state directly without any overhead. But this means: one JS object for one DOM object.

Currently, you are using a class, but inside this is more or less functional programming. It would be more OO style to define the interactions in a class, but let every button be represented by a class instance (or "object" if you like).

Maybe you have a look on the DML-project, that implements a fully class based DOM framework.

BR Eckehard

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.