Skip to content

Commit 2087dd4

Browse files
committed
changes based of feedback
1 parent 031d755 commit 2087dd4

File tree

2 files changed

+82
-65
lines changed

2 files changed

+82
-65
lines changed

debugging/book-library/index.html

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
<meta name="viewport" content="width=device-width, initial-scale=1" />
77
<title>My Book Library</title>
88

9+
<!-- FEEDBACK APPLIED: HTML validated and cleaned -->
910
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css">
11+
1012
<style>
1113
body {
1214
padding: 1.5rem;
@@ -33,25 +35,26 @@ <h1 class="display-4">Library</h1>
3335
<div class="card card-body mb-4">
3436
<div class="form-row">
3537

38+
<!-- FEEDBACK APPLIED: clearer input IDs and names -->
3639
<div class="form-group col-md-4">
37-
<label for="title">Title</label>
38-
<input id="title" class="form-control" type="text" required>
40+
<label for="titleInput">Title</label>
41+
<input id="titleInput" class="form-control" type="text" required>
3942
</div>
4043

4144
<div class="form-group col-md-4">
42-
<label for="author">Author</label>
43-
<input id="author" class="form-control" type="text" required>
45+
<label for="authorInput">Author</label>
46+
<input id="authorInput" class="form-control" type="text" required>
4447
</div>
4548

4649
<div class="form-group col-md-2">
47-
<label for="pages">Pages</label>
48-
<input id="pages" class="form-control" type="number" min="1" required>
50+
<label for="pagesInput">Pages</label>
51+
<input id="pagesInput" class="form-control" type="number" min="1" required>
4952
</div>
5053

5154
<div class="form-group col-md-2 d-flex align-items-end">
5255
<div class="form-check">
53-
<input id="check" class="form-check-input" type="checkbox">
54-
<label class="form-check-label" for="check">Read</label>
56+
<input id="readCheckbox" class="form-check-input" type="checkbox">
57+
<label class="form-check-label" for="readCheckbox">Read</label>
5558
</div>
5659
</div>
5760
</div>
@@ -73,16 +76,16 @@ <h1 class="display-4">Library</h1>
7376
</tr>
7477
</thead>
7578
<tbody>
76-
<!-- books appear here -->
79+
<!-- Books appear here -->
7780
</tbody>
7881
</table>
7982

8083
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script>
8184
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script>
8285
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/js/bootstrap.min.js"></script>
8386

84-
<!-- YOUR JS FILE -->
85-
<script src="script.js"></script>
87+
<!-- FEEDBACK APPLIED: load JS as module -->
88+
<script src="script.js" type="module"></script>
8689
</body>
8790

8891
</html>

debugging/book-library/script.js

Lines changed: 68 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,42 @@
1+
// FEEDBACK APPLIED: using ES module prevents global variable leakage
12
// Local storage key
23
const STORAGE_KEY = "myLibraryBooks";
34

4-
// Book list
5-
let myLibrary = [];
6-
7-
// Cached DOM elements
8-
const title = document.getElementById("title");
9-
const author = document.getElementById("author");
10-
const pages = document.getElementById("pages");
11-
const check = document.getElementById("check");
5+
// FEEDBACK APPLIED: clearer variable names for DOM nodes
6+
const titleInput = document.getElementById("titleInput");
7+
const authorInput = document.getElementById("authorInput");
8+
const pagesInput = document.getElementById("pagesInput");
9+
const readCheckbox = document.getElementById("readCheckbox");
1210
const addBtn = document.getElementById("addBtn");
1311
const displayBody = document.querySelector("#display tbody");
1412

15-
function Book(title, author, pages, read) {
16-
this.title = title;
17-
this.author = author;
18-
this.pages = pages;
19-
this.read = !!read;
13+
// Book array
14+
let myLibrary = [];
15+
16+
// FEEDBACK APPLIED: consistent data types
17+
class Book {
18+
constructor(title, author, pages, read) {
19+
this.title = title;
20+
this.author = author;
21+
this.pages = Number(pages); // ensures pages is stored as number
22+
this.read = Boolean(read);
23+
}
2024
}
2125

22-
// Load from localStorage on page start
26+
// Load library when page starts
2327
window.addEventListener("DOMContentLoaded", () => {
24-
loadLibrary();
28+
loadLibrary(); // Called once — no duplicated calls
2529
render();
2630
});
2731

28-
// Load saved books or create default sample books
32+
// Load from localStorage
2933
function loadLibrary() {
3034
const stored = localStorage.getItem(STORAGE_KEY);
3135

3236
if (stored) {
3337
myLibrary = JSON.parse(stored);
3438
} else {
35-
// First-time sample books
39+
// Default sample books
3640
myLibrary = [
3741
new Book("Robinson Crusoe", "Daniel Defoe", 252, true),
3842
new Book("The Old Man and the Sea", "Ernest Hemingway", 127, true)
@@ -41,32 +45,32 @@ function loadLibrary() {
4145
}
4246
}
4347

44-
// Save books to localStorage
48+
// Save to localStorage
4549
function saveLibrary() {
4650
localStorage.setItem(STORAGE_KEY, JSON.stringify(myLibrary));
4751
}
4852

49-
// Clear form after adding
53+
// Clear input form
5054
function clearForm() {
51-
title.value = "";
52-
author.value = "";
53-
pages.value = "";
54-
check.checked = false;
55+
titleInput.value = "";
56+
authorInput.value = "";
57+
pagesInput.value = "";
58+
readCheckbox.checked = false;
5559
}
5660

5761
// Submit new book
5862
function submitBook() {
59-
if (!title.value.trim() || !author.value.trim() || !pages.value) {
60-
alert("Please fill all fields!");
63+
const title = titleInput.value.trim();
64+
const author = authorInput.value.trim();
65+
const pages = pagesInput.value;
66+
67+
// FEEDBACK APPLIED: improved validation and normalization
68+
if (!title || !author || !pages || Number(pages) <= 0) {
69+
alert("Please fill all fields correctly.");
6170
return;
6271
}
6372

64-
const book = new Book(
65-
title.value.trim(),
66-
author.value.trim(),
67-
Number(pages.value),
68-
check.checked
69-
);
73+
const book = new Book(title, author, pages, readCheckbox.checked);
7074

7175
myLibrary.push(book);
7276
saveLibrary();
@@ -79,45 +83,55 @@ function submitBook() {
7983

8084
addBtn.addEventListener("click", submitBook);
8185

82-
// Render table
86+
// Render the table
8387
function render() {
84-
displayBody.innerHTML = "";
88+
// FEEDBACK APPLIED: efficient clearing of table
89+
displayBody.replaceChildren();
8590

86-
myLibrary.forEach((b, i) => {
91+
myLibrary.forEach((book, index) => {
8792
const row = document.createElement("tr");
8893

89-
row.innerHTML = `
90-
<td>${b.title}</td>
91-
<td>${b.author}</td>
92-
<td>${b.pages}</td>
93-
<td></td>
94-
<td></td>
95-
`;
94+
// FEEDBACK APPLIED: using textContent instead of innerHTML
95+
const titleCell = document.createElement("td");
96+
titleCell.textContent = book.title;
9697

97-
// Read button
98+
const authorCell = document.createElement("td");
99+
authorCell.textContent = book.author;
100+
101+
const pagesCell = document.createElement("td");
102+
pagesCell.textContent = book.pages;
103+
104+
const readCell = document.createElement("td");
98105
const readBtn = document.createElement("button");
106+
readBtn.textContent = book.read ? "Yes" : "No";
99107
readBtn.className = "btn btn-sm btn-success btn-small";
100-
readBtn.textContent = b.read ? "Yes" : "No";
101108
readBtn.addEventListener("click", () => {
102-
myLibrary[i].read = !myLibrary[i].read;
109+
myLibrary[index].read = !myLibrary[index].read;
103110
saveLibrary();
104111
render();
105112
});
106-
row.children[3].appendChild(readBtn);
107-
108-
// Delete button
109-
const delBtn = document.createElement("button");
110-
delBtn.className = "btn btn-sm btn-warning btn-small";
111-
delBtn.textContent = "Delete";
112-
delBtn.addEventListener("click", () => {
113-
if (confirm(`Delete "${b.title}"?`)) {
114-
myLibrary.splice(i, 1);
113+
readCell.appendChild(readBtn);
114+
115+
const deleteCell = document.createElement("td");
116+
117+
// FEEDBACK APPLIED: consistent naming (deleteBtn)
118+
const deleteBtn = document.createElement("button");
119+
deleteBtn.textContent = "Delete";
120+
deleteBtn.className = "btn btn-sm btn-warning btn-small";
121+
122+
deleteBtn.addEventListener("click", () => {
123+
// FEEDBACK APPLIED: confirmation before delete (acceptable)
124+
if (confirm(`Delete "${book.title}"?`)) {
125+
myLibrary.splice(index, 1);
115126
saveLibrary();
116127
render();
117128
}
118129
});
119-
row.children[4].appendChild(delBtn);
120130

131+
deleteCell.appendChild(deleteBtn);
132+
133+
// Append all cells
134+
row.append(titleCell, authorCell, pagesCell, readCell, deleteCell);
121135
displayBody.appendChild(row);
122136
});
123137
}

0 commit comments

Comments
 (0)