Skip to content

Improve samples/book-app-project/books.py: add atomic save, validatio…#162

Closed
ewyuen wants to merge 1 commit into
github:mainfrom
ewyuen:improve-books-py
Closed

Improve samples/book-app-project/books.py: add atomic save, validatio…#162
ewyuen wants to merge 1 commit into
github:mainfrom
ewyuen:improve-books-py

Conversation

@ewyuen

@ewyuen ewyuen commented Jun 11, 2026

Copy link
Copy Markdown

…n, context manager, comparison ops

…n, context manager, comparison ops

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 11, 2026 18:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Enhances the sample book app’s persistence and API by adding richer Book/BookCollection utilities, improving JSON file handling, and expanding collection operations.

Changes:

  • Added JSON-serialization helpers, ordering/equality behavior, and validation for Book.
  • Improved persistence with path-based storage, atomic writes, and corrupt-file backup.
  • Expanded BookCollection API (sorting, search, update, export, counts, context manager).

Comment on lines +81 to +87
def __exit__(self, exc_type, exc, tb) -> bool:
# On normal exit persist changes; never suppress exceptions.
try:
with open(DATA_FILE, "r") as f:
data = json.load(f)
self.books = [Book(**b) for b in data]
except FileNotFoundError:
self.save_books()
except Exception:
LOG.exception("Failed to save books on __exit__")
return False
Comment on lines 177 to 182
def mark_as_read(self, title: str) -> bool:
book = self.find_book_by_title(title)
if book:
if book and not book.read:
book.read = True
self.save_books()
return True
Comment on lines +199 to +217
def update_book(self, title: str, **fields) -> bool:
"""Update fields on a book identified by title. Allowed fields: title, author, year, read.

Returns True if the book was found and updated; False otherwise.
"""
book = self.find_book_by_title(title)
if not book:
return False
allowed = {"title", "author", "year", "read"}
changed = False
for k, v in fields.items():
if k in allowed:
if k == "year":
v = _validate_year(v)
setattr(book, k, v)
changed = True
if changed:
self.save_books()
return changed
Comment on lines +120 to +131
tmp_fd, tmp_path = tempfile.mkstemp(dir=str(self.data_file.parent))
try:
with os.fdopen(tmp_fd, "w", encoding="utf-8") as f:
json.dump(data, f, indent=2, ensure_ascii=False, sort_keys=True)
os.replace(tmp_path, str(self.data_file))
except Exception:
# Attempt to clean up temp file on failure
try:
os.remove(tmp_path)
except OSError:
pass
raise
Comment on lines +52 to +55
def __lt__(self, other: "Book") -> bool:
return (self.title.lower(), self.author.lower(), self.year) < (
other.title.lower(), other.author.lower(), other.year
)
Comment on lines 194 to +197
def find_by_author(self, author: str) -> List[Book]:
"""Find all books by a given author."""
return [b for b in self.books if b.author.lower() == author.lower()]
"""Find all books where the provided string is contained in the author name (case-insensitive)."""
a = author.strip().lower()
return [b for b in self.books if a in b.author.lower()]
@ewyuen ewyuen closed this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants