/*
Online cloud reading application
Amazon Kindle for short stories
Enums
=====
Status { ACTIVE, INACTIVE }
Structures
==========
Book
- id: &str
User
- id: &str
- library: BookState[]
(BookState
- book: Book
- status: Status
- last_read_page_number: u16)
Altrenative
===========
// How many active users are there for Book 'X' ?
// How many active books are there for User 'Y' ?
Book
- id: &str
User
- id: &str
- library: BookState[]
(BookState
- book: Book
- last_read_page_number: u16)
Features
========
- Add or remove books from a library
- Set to active any book in library
- render the currently reading page
- adjust the page number
*/
#[derive(Clone)]
enum Status {
Active,
Inactive
}
struct Page {
}
struct Book {
id: String,
pages: Vec<Page>
}
#[derive(Clone)]
struct BookState<'a> {
book: &'a Book,
status: Status,
last_read_page_number: usize,
}
struct User<'a> {
id: String,
library: Vec<BookState<'a>>
// library: HashMap<Book, BookState>
}
impl Book {
fn new(id: String) -> Book {
Book {
id,
pages: vec![]
}
}
}
impl<'a> User<'a> {
fn new(id: String) -> User<'a> {
User {
id,
library: vec![]
}
}
fn add_book_to_lib(&mut self, book: &'a Book) {
let new_book_state = BookState {
book: book,
status: Status::Inactive,
last_read_page_number: 0
};
self.library.push(new_book_state);
}
fn remove_book_from_lib<'b>(&mut self, book: &'b Book) {
self.library = self.library
.iter()
.filter(|book_state| book_state.book.id != book.id)
.map(|x| x.clone())
.collect();
}
fn make_active_book<'b>(&mut self, book: &'b Book) {
let mut made_active = false;
self.library = self.library
.iter()
.map(|book_state| {
if book_state.book.id == book.id {
let mut update = book_state.clone();
update.status = Status::Active;
made_active = true;
return update;
}
return book_state.clone();
})
.collect();
if !made_active {
panic!("Called make_active on a book that isn't in the library ! ")
}
}
fn next_page<'b>(&mut self, book: &'b Book) -> i32 {
let mut current_page: i32 = -1;
self.library = self.library
.iter()
.map(|book_state| {
if book_state.book.id == book.id {
let mut update = book_state.clone();
update.status = Status::Active;
current_page = update.last_read_page_number as i32;
update.last_read_page_number += 1;
return update;
}
return book_state.clone();
})
.collect();
return current_page;
}
}
fn main() {
let mut me = User::new("1".to_string());
let book1 = Book::new("811341".to_string());
let book2 = Book::new("485952".to_string());
let book3 = Book::new("247672".to_string());
me.add_book_to_lib(&book1);
me.add_book_to_lib(&book2);
me.add_book_to_lib(&book3);
me.remove_book_from_lib(&book2);
me.make_active_book(&book3);
let f = me.next_page(&book3);
let s = me.next_page(&book3);
println!("{}, {}", f, s);
}
First of all, welcome to the Rust user forums! I've got a couple suggestions for making your code more idiomatic or easier for others to follow.
-
Consider either removing dead code like the
Pagestruct andStatus::Inactivevariant or making sure your code will use them. -
To make your code more idiomatic, you can use
HashMapfrom thestd::collectionsmodule to store the user's library. It provides a more efficient way to handle lookups, insertions, and deletions. You can use the book's ID as the key andBookStateas the value. This way, you don't need to iterate through the whole library to find a book. For example:
use std::collections::HashMap;
struct User<'a> {
id: String,
library: HashMap<String, BookState<'a>>,
}
// When adding a book:
fn add_book_to_lib(&mut self, book: &'a Book) {
let new_book_state = BookState {
book,
status: Status::Inactive,
last_read_page_number: 0,
};
self.library.insert(book.id.clone(), new_book_state);
}
- In the
remove_book_from_libmethod, you can simply use theremovemethod provided byHashMapto delete a book from the user's library:
fn remove_book_from_lib(&mut self, book: &Book) {
self.library.remove(&book.id);
}
- For the make_active_book method, you can avoid unnecessary cloning and simplify the method using the get_mut method provided by HashMap:
fn make_active_book(&mut self, book: &Book) {
if let Some(book_state) = self.library.get_mut(&book.id) {
book_state.status = Status::Active;
} else {
panic!("Called make_active on a book that isn't in the library!");
}
}
- You can further improve the next_page method by using get_mut to avoid unnecessary cloning and make the code more readable:
fn next_page(&mut self, book: &Book) -> Option<usize> {
self.library.get_mut(&book.id).map(|book_state| {
book_state.status = Status::Active;
book_state.last_read_page_number += 1;
book_state.last_read_page_number
})
}
Note that the method now returns Option instead of i32. This change makes the function signature more idiomatic, as it indicates that the function may not return a value (e.g., when the book is not in the library). You should handle this case in your main` function:
fn main() {
// ... (previous code)
let f = me.next_page(&book3).expect("Book not found in the library");
let s = me.next_page(&book3).expect("Book not found in the library");
println!("{}, {}", f, s);
}
- Rust has great support for documentation comments. Use them to provide a high-level explanation of your types, methods, and functions. This will make it easier for others to understand your code:
/// A user of the online cloud reading application.
struct User<'a> {
// ...
}
impl<'a> User<'a> {
/// Adds a book to the user's library.
fn add_book_to_lib(&mut self, book: &'a Book) {
// ...
}
/// Removes a book from the user's library.
fn remove_book_from_lib(&mut self, book: &Book) {
// ...
}
/// Sets the specified book as active.
fn make_active_book(&mut self, book: &Book) {
// ...
}
/// Moves to the next page of the specified book and returns the new page number.
fn next_page(&mut self, book: &Book) -> Option<usize> {
// ...
}
}
- Regarding ownership and lifetimes, your current implementation works well for this specific use case, however a general rule of thumb is for structs to own all of their fields. It's completely fine for a struct to have references, but often people will get themselves tied up in knots ("fighting the borrow checker") because using lifetimes means your
UserandBookStatestructs can't outlive theBookobjects they refer to. This can be particularly annoying if aBookis a local variable because it means you won't be able to return yourUseror keep it around long-term.
For example, using your approach means it's impossible to write a function that creates a user and populates their library.
fn new_user() -> User<???> {
let mut me = User::new("1".to_string());
let book1 = Book::new("811341".to_string());
let book2 = Book::new("485952".to_string());
let book3 = Book::new("247672".to_string());
me.add_book_to_lib(&book1);
me.add_book_to_lib(&book2);
me.add_book_to_lib(&book3);
me
}
Compiling the above code will fail with this error message:
error[E0515]: cannot return value referencing local variable `book1`
--> src/main.rs:199:5
|
196 | me.add_book_to_lib(&book1);
| ------ `book1` is borrowed here
...
199 | me
| ^^ returns a value referencing data owned by the current function
The solution is to make sure a User owns the books in their library.
#[derive(Clone)]
struct BookState {
book: Book,
status: Status,
last_read_page_number: usize,
}
struct User {
id: String,
library: HashMap<String, BookState>,
}
If the same book may be in multiple people's libraries, you can wrap it in a std::sync::Arc.
4 Likes
This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.