0
\$\begingroup\$

My Approach

The code for the library project is all over the place, how would you code this project using object-oriented programming. For example, would you use

document.addEventListener()

and just set if statements for elements that are dynamically created?

Also, there is not a lot of comments in this code, I thought my naming for each variable made sense and so comments were unneeded. Do you disagree?

I used a closure in my render function but that's actually my first time using a closure so i don't know if that is correct or would it just involve unexpected bugs.

I did this project for The Odin Project Here's the link the project and steps to take https://www.theodinproject.com/courses/javascript/lessons/library

/*------------------------------------------ 
                Variables
-------------------------------------------*/
let myLibrary = [];
let bookIndex = 0;
let mainRender = render();

/*------------------------------------------ 
                Constructor
-------------------------------------------*/
function Book(title, author, pages, status) {
    this.title = title;
    this.author = author;
    this.pages = pages;
    this.status = status;
}

/*------------------------------------------ 
                Functions
-------------------------------------------*/
function addBookToLibrary() {
    const title = document.querySelector('#title').value;
    const author = document.querySelector('#author').value;
    const pages = document.querySelector('#pages').value;
    const status = document.querySelector('#status').value;
    const book = new Book(title, author, pages, status);
    myLibrary.push(book);
}

function render() {
    let i = 0;

    return function() {
        const bookshelf = document.querySelector('.bookshelf__creation');
        for (; i < myLibrary.length; i++) {
            bookshelf.insertAdjacentHTML('beforeend', bookTemplate(myLibrary[i]));
        }
    }
}

function bookTemplate(book) {
    const statusBtnColor = book.status == 'unread' ? 'status__btn status__btn--change' : 'status__btn';
    const HTML = `
            <tr class="bookshelf__book" data-index="${bookIndex++}">
                <td class="title">${book.title}</td>
                <td class="author">${book.author}</td>
                <td class="pages">${book.pages}</td>
                <td class="status"><button id="status-btn" class="${statusBtnColor}">${book.status}</button></td>
                <td class="delete"><button class="btn btn--primary">x</button></td>
            </tr>`;

    return HTML;
}

function toggleStatus(event) {
    const index = event.target.parentElement.parentElement.dataset.index;

    if (event.target.textContent == 'read') {
        event.target.textContent = 'unread';
        myLibrary[index]['status'] = 'unread';
        event.target.classList.add('status__btn--change');
    } else if (event.target.textContent == 'unread') {
        event.target.textContent = 'read';
        myLibrary[index]['status'] = 'read';
        event.target.classList.remove('status__btn--change');
    }    
}

function removeBook(event) {
    const parentOfBookInfo = event.target.parentNode.parentNode;
    parentOfBookInfo.parentNode.remove();

    myLibrary = myLibrary.filter( book => {
        console.log(`${book.title}\n${parentOfBookInfo.querySelector(".title").textContent}`);
        return book.title !== parentOfBookInfo.querySelector(".title").textContent;
    });
}

function emptyInputs() {
    const modalInputs = document.querySelectorAll('.modal__input');
    modalInputs.forEach(input => input.value = '');
}

/*------------------------------------------ 
                Event Listeners
-------------------------------------------*/
document.addEventListener('click', event => {
    const modal = document.querySelector('.modal');

    if (event.target.className == 'btn btn--primary') removeBook(event);
    if (event.target.id == 'status-btn') toggleStatus(event);
    if (event.target.className == 'bookshelf__open-modal') modal.style.display = 'block';

    if (event.target.className == 'modal__add') {
        modal.style.display = 'none';
        addBookToLibrary();
        emptyInputs();
        mainRender();
    }

    if (event.target.className == 'modal__cancel') {
        modal.style.display = 'none';
        emptyInputs();
    }
});
* {
  margin: 0;
  padding: 0;
  -webkit-box-sizing: border-box;
          box-sizing: border-box;
  font-family: 'Roboto', sans-serif;
}

.background {
  padding: 2% 4%;
  margin-bottom: 2em;
  height: 12rem;
  width: 100%;
  color: #fff;
  background: linear-gradient(304deg, #020024 0%, #3a3d3d 0%, #282828 94%);
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  -webkit-box-orient: vertical;
  -webkit-box-direction: normal;
      -ms-flex-direction: column;
          flex-direction: column;
  -webkit-box-pack: center;
      -ms-flex-pack: center;
          justify-content: center;
}

.background__heading {
  margin-bottom: 0.2em;
  font-size: 2.5rem;
}

.bookshelf {
  max-width: 90%;
  margin: 0 auto;
}

.bookshelf__heading {
  font-size: 2rem;
  font-weight: 700;
}

.bookshelf__creation {
  text-align: left;
}

.bookshelf table {
  border-collapse: collapse;
  margin: 2em 0;
}

.bookshelf table th {
  padding: 0.6em 1em;
}

.bookshelf__book {
  border-top: 2px solid #ececec;
}

.bookshelf__book td {
  padding: 1em;
  text-align: center;
}

.bookshelf__open-modal {
  padding: 0.7rem;
  border: none;
  background-color: #383838;
  color: white;
  cursor: pointer;
}

.status__btn {
  padding: 0.4rem 2rem;
  border: none;
  background-color: #ececec;
  cursor: pointer;
}

.status__btn--change {
  background-color: #383838;
  color: white;
}

.delete .btn {
  width: 1.5rem;
  height: 1.5rem;
  color: white;
  border-radius: 50%;
  text-align: center;
  border: none;
  font-weight: 700;
}

.modal {
  padding: 1.25rem;
  width: 100%;
  position: absolute;
  top: 0;
  background: white;
  max-width: 50%;
  margin: 0 auto;
  -webkit-transform: translate(50%, 20%);
          transform: translate(50%, 20%);
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  -webkit-box-align: center;
      -ms-flex-align: center;
          align-items: center;
  -webkit-box-pack: center;
      -ms-flex-pack: center;
          justify-content: center;
  -webkit-box-orient: vertical;
  -webkit-box-direction: normal;
      -ms-flex-direction: column;
          flex-direction: column;
  border-radius: 6px;
  -webkit-box-shadow: 0 0.5em 1em -0.125em rgba(10, 10, 10, 0.1), 0 0 0 1px rgba(10, 10, 10, 0.02);
          box-shadow: 0 0.5em 1em -0.125em rgba(10, 10, 10, 0.1), 0 0 0 1px rgba(10, 10, 10, 0.02);
  color: #4a4a4a;
}

.modal .modal__add {
  padding: 0.7rem;
  border: none;
  background-color: #383838;
  color: white;
  cursor: pointer;
}

.modal .modal__cancel {
  padding: 0.7rem;
  border: 1.5px solid lightgray;
  border-radius: 5px;
  background-color: #fff;
  cursor: pointer;
}

.modal h2 {
  margin-bottom: 1em;
}

.modal * {
  -ms-flex-item-align: self-start;
      -ms-grid-row-align: self-start;
      align-self: self-start;
}

.modal label {
  font-size: 1rem;
  font-weight: 600;
  margin-bottom: 0.4em;
}

.modal input {
  width: 100%;
  margin-bottom: 1em;
  height: 3em;
  border: 1.5px solid lightgray;
  border-radius: 5px;
}

.modal select {
  width: 6rem;
  height: 2rem;
  border: 1.5px solid lightgray;
  border-radius: 5px;
  background-color: #fff;
  margin-bottom: 1em;
  cursor: pointer;
}

tbody:nth-child(even) {
  background-color: whitesmoke;
  border-top: 1px solid #a0a0a0;
}
<!DOCTYPE html>
<html lang="en">

    <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <meta http-equiv="X-UA-Compatible" content="ie=edge">
        <title>Librario</title>

        <!-- links -->
        <link href="https://fonts.googleapis.com/css?family=Roboto:400,700&display=swap" rel="stylesheet">
        <link rel="stylesheet" href="./index.css">
    </head>

    <body>

        <div class="container">
            <header class="background">
                <h1 class="background__heading">Librario</h1>
                <p class="background__intro-text">Your Pocket Bookshelf</p>
            </header>

            <div class="bookshelf">
                <h2 class="bookshelf__heading">Books</h2>

                <table class="bookshelf__creation">
                    
                    <tr class="table">
                        <th class="table__heading">Title</th>
                        <th class="table__heading">Author</th>
                        <th class="table__heading">Pages</th>
                        <th class="table__heading">Status</th>
                    </tr>

                    <!-- JavaScript make this -->
                    <!-- <tr class="bookshelf__book" data-newBook="">
                        <td class="title">Harry Potter and the Sorceror Stone</td>
                        <td class="author">J.K Rowling</td>
                        <td class="pages">890</td>
                        <td class="status"><button class="status__btn">Read</button></td>
                        <td class="delete"><button class="btn btn--primary">x</button></td>
                    </tr>

                    <tr class="bookshelf__book" data-newBook="">
                        <td class="title">Harry Potter and the Sorceror Stone</td>
                        <td class="author">J.K Rowling</td>
                        <td class="pages">890</td>
                        <td class="status"><button class="status__btn">Read</button></td>
                        <td class="delete"><button class="btn btn--primary">x</button></td>
                    </tr> -->
                    
                </table>

                <button class="bookshelf__open-modal">Add New Book</button>
            </div>

            <div class="modal" style="display: none;">
                <h2>Add New Book</h2>
                <label for="title">Title</label>
                <input class="modal__input" id="title" type="text">
                
                <label for="author">Author</label>
                <input class="modal__input" id="author" type="text">
                
                <label for="pages">Number of Pages</label>
                <input class="modal__input" id="pages" type="text">

                <label for="status">Read Status</label>
                <select  id="status">
                    <option value="read">Read</option>
                    <option value="unread">Unread</option>
                </select>

                <div class="modal__btn-container">
                    <button class="modal__add">Add Book</button>
                    <button class="modal__cancel">Cancel</button>
                </div>
            </div>

        </div>

        <!-- Scripts -->
        <script src="./index.js"></script>
    </body>

</html>

\$\endgroup\$

2 Answers 2

Reset to default
2
\$\begingroup\$

Because this question is tagged with , I will focus on some improvements based on the oop-design.

Currently your code uses objects, but this does not mean that it is object-oriented.

When we take a look to your code all methods are global. With objects we can couple the methods to an object. For example:
The methods addBookToLibrary and removeBook have a strong relationship to myLibary and toggleStatus to Book.

function Book(title, author, pages, status) {
  this.title = title;
  this.author = author;
  this.pages = pages;
  this.status = status;
}

Book.prototype.toggleStatus = function() {
  if (this.status === 'read') {
    this.status = 'unread';
  } else {
    this.status = 'read';
  }
}

const zhivago = new Book('Doctor Zhivago', 'Boris Pasternak', '592', 'unread');

console.log(zhivago.status);

zhivago.toggleStatus();

console.log(zhivago.status);

Or the ES6-Version below which uses class to abstract the prototype change.

class Book {
  constructor(title, author, pages, status) {
    this.title = title;
    this.author = author;
    this.pages = pages;
    this.status = status;
  }

  toggleStatus() {
    if (this.status === 'read') {
      this.status = 'unread';
    } else {
      this.status = 'read';
    }
  }
}

const zhivago = new Book('Doctor Zhivago', 'Boris Pasternak', '592', 'unread');

console.log(zhivago.status);

zhivago.toggleStatus();

console.log(zhivago.status);

Currently the program heavy rely on strings. Actually the status is an abstraction in Book and could be represent with an own datatype with to concrete types Unread and Read. The following example just shows how composition could work and is not a recommendation:

class Book {
  constructor(title, author, pages, status) {
    this.title = title;
    this.author = author;
    this.pages = pages;
    this.status = status;
  }

  toggleStatus() {
    this.status = this.status.toggle();
  }
}

class Unread {
  constructor() {
    this.status = 'unread';
  }

  toggle() {
    return new Read()
  }
}

class Read {
  constructor() {
    this.status = 'read';
  }

  toggle() {
    return new Unread()
  }
}

const zhivago = new Book('Doctor Zhivago', 'Boris Pasternak', '592', new Unread());
console.log(zhivago.status)
zhivago.toggleStatus();
console.log(zhivago.status)

Possible Object Structure

class Library {
  constructor() {
    this.books = [];
  }

  add(book) {
    this.books.push(book);
  }

  removeBookWithTitle(title) {
    this.books = this.books.filter(book => book.hasNot(title))
  }

  get(index) {
    return this.books[index];
  }

  numberOfBooks() {
    return this.books.length;
  }
}

class Book {
  constructor(title, author, pages, status) {
    this.title = title;
    this.author = author;
    this.pages = pages;
    this.status = status;
  }

  toggleStatus() {
    this.status = this.status.toggle();
  }
  
  has(title) {
    return this.title === title;
  }

  hasNot(title) {
    return !this.has(title);
  }
  
}

class Unread {
  constructor() {
    this.value = 'unread';
  }

  toggle() {
    return new Read()
  }
}

class Read {
  constructor() {
    this.value = 'read';
  }

  toggle() {
    return new Unread()
  }
}

const myLibrary = new Library();
const unread = new Unread();

myLibrary.add(new Book('title', 'author', 123, unread));
myLibrary.add(new Book('other title', 'other author', 321, unread));

myLibrary.removeBookWithTitle('title')

const aBook = myLibrary.get(0)
aBook.toggleStatus();

console.log(JSON.stringify(myLibrary, null, 2))

MVC

@K.H. mentions in his answer already that your Model is tightly coupled to the HTML. The Model-View-Controller is one of many patterns to logic is decoupled from how it gets represent.

\$\endgroup\$
3
\$\begingroup\$

I agree, that your naming is mostly alright, no point to document these variables. Except for event variables.

What I seriously don't like about your approach is how tightly it is bound to html. For example removeBook takes as parameter event, which you use to extract element and use that one to extract data that you use to remove. That is bad from many reasons.

Example:

If you are removing book, I expect parameter to be "book", not some kind of event. What if I wanted to add keyboard shortcut to remove book? Or maybe drag & drop? First step is to pass element, that represents the book parentOfBookInfo. But still, that element is both data and both it's html representation. Even that change makes your code cleaner and more flexible - you can remove that book/element based on different actions or events. Separating html and data is a bit harder and may be not worth it. It's better to keep "database" or "memory" of your Book objects and that then triggers UI update (or similar approach, depending on your pattern). But digging deeper and deeper into making this "clean" usually ends up making your own framework and then it is usually better to choose one, that is close to your mindset and what you are trying to do :)

Having "global" click listener can be fine, but then again you end up making some kind of routing to make sure clicking on element, that you wanted will trigger correct function and in the end it may be easier to just attach event to element in the first place :) I don't like it in this case though. Again very much bound to css (and looks like visually-depended css) so if you change css for design, you break your functionality. Also theoretically it is possible with your code to trigger multiple events if your html allows it. Ex. if element has both modal__add and modal_cancel, there will be 2 methods triggered. That seems bad and may lead to bugs hard to find in case someone messes up html. Better switch or explicit return after passing event to something to make sure it does not propagate further.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you! Also what do you think about my way of using a constructor? Should I use more of prototype inheritance or just keep it as it is \$\endgroup\$ Commented Dec 29, 2019 at 18:43
  • \$\begingroup\$ That depends on your targeting platform and compatibility + if you want to use babel. Imho best approach is to use modern javascript and then use babel if you need backwards compatibility. In that case you can even go further and use class, constructor keywords and other newer features. Another interesting alternative is Typescript. \$\endgroup\$ Commented Dec 30, 2019 at 7:56

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.