1
\$\begingroup\$

Here is my bootstrap pagination code. Maybe it can be simplified.

const pagination = {
  pre: 0,
  curr: 1,
  max: 6,
  list: null,
  cn: '',
  swap() {
    pagination.list.children[pagination.pre].classList.remove(pagination.cn);
    pagination.list.children[pagination.curr].classList.add(pagination.cn);
  },
  changeBy(n) {
    pagination.pre = pagination.curr;
    pagination.curr += n;
    if (pagination.curr < 1) {
      pagination.pre = 0;
      pagination.curr = 1;
    } else if (pagination.curr > pagination.max) {
      pagination.pre = pagination.max - 1;
      pagination.curr = pagination.max;
    }
    pagination.swap();
  },
  prev() {
    pagination.changeBy(-1);
  },
  next() {
    pagination.changeBy(1);
  },
  activate(i) {
    return () => {
      pagination.pre = pagination.curr;
      pagination.curr = i;
      pagination.swap();
    }
  },
  setHandler() {
    pagination.list.children[0].addEventListener("click", pagination.prev);
    pagination.list.children[pagination.max + 1].addEventListener("click", pagination.next);
    for (let i = 1; i < (pagination.max + 1); i += 1) {
      pagination.list.children[i].onclick = pagination.activate(i);
    }
  },
  init(list, cn) {
    pagination.list = list;
    pagination.cn = cn;
    pagination.setHandler();
  }
}

let list = document.querySelector(".pagination"),
  cn = "active",
  init = pagination.init(list, cn);
<link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" rel="stylesheet">
<nav aria-label="Page navigation">
  <ul class="pagination">
    <li>
      <a href="#" aria-label="Previous">
        <span aria-hidden="true">&laquo;</span>
      </a>
    </li>
    <li class="active"><a href="#">1</a></li>
    <li><a href="#">2</a></li>
    <li><a href="#">3</a></li>
    <li><a href="#">4</a></li>
    <li><a href="#">5</a></li>
    <li><a href="#">6</a></li>
    <li>
      <a href="#" aria-label="Next">
        <span aria-hidden="true">&raquo;</span>
      </a>
    </li>
  </ul>
</nav>

\$\endgroup\$
2
\$\begingroup\$

Could you give more information on what the point of this is? All it does is change the class of the links. What about the actual pagination itself, which i assume would use AJAX? This code should probably know about that in order to react to errors.


Wrapping the code in an object like this isn't very useful like this. For one, the functions inside the containing object all refer to its variable name, so it breaks if it's changed. Also it looks like a reusable class allowing to have multiple paginations in one HTML document, however it's not.


You have the "max size" redundantly hard coded twice - once in the JavaScript and once as the number of elements in the HTML. How do you know that the pagination has exactly 6 pages?

The same basic problem with the original "current" element and the "active" class: It's hard coded in both the JS and the HTML. With the class name it even looks as if you could change it, because it's an initialization parameter, but changing it breaks the design, because its hard coded in the HTML.


Generally you have some unnecessarily abbreviated variable names, cn being the worst.

\$\endgroup\$

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.