Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CS2113-F15-4] BookBuddy #54

Open
wants to merge 494 commits into
base: master
Choose a base branch
from

Conversation

yeozongyao
Copy link

BookBuddy helps users track and manage the list of the books they are reading. It is optimized for CLI users so that the tracking and management objectives can be achieved most efficiently.

JeffinsonDarmawan added a commit to JeffinsonDarmawan/tp that referenced this pull request Mar 22, 2024
…dler

Successfully set up file handler. Florizz.xml file will appear if for…

By abstracting out the parsing functionality of BookBuddy into a separate `Parser` class, the complexity of parsing user input is removed
from the main code. It is instead replaced by a simple interface for the user to work with, adhering to the abstraction concept of
object-oriented programming.

## Product scope
Copy link

Choose a reason for hiding this comment

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

image
Could you give more elaborations on user stories?

Copy link

@JingHaoooo JingHaoooo left a comment

Choose a reason for hiding this comment

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

Overall good job. Diagrams were mostly clear and easy to understand.

- An embedded approach, where genre setting logic is part of a larger class managing all book attributes, was
considered. However, this was rejected due to potential scalability issues and difficulty in maintaining code.

![SetGenreSequenceDiagram.png](UML_diagrams/SetGenreSequenceDiagram.png)

Choose a reason for hiding this comment

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

image

Is this correct? Should it be returning any value(eg genres)?

By importing predefined string constants from `CommandList` class representing the valid commands, the `ParserMain` class
parses the input from a user using the `parseCommand` method. The class diagram below shows how `ParserMain` interacts with other
classes.
![ParserMain.png](UML_diagrams/ParserMain.png)

Choose a reason for hiding this comment

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

image
Perhaps you could remove the icons in the class diagrams, as well as the attributes icons.

Comment on lines 179 to 180
| v1.0 | user | add books to a list | |
| v1.0 | user | remove books from a list | |

Choose a reason for hiding this comment

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

Perhaps you could add the part for "so that I can" to make it complete.

Copy link

@LowTL LowTL left a comment

Choose a reason for hiding this comment

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

Well done on the DG! There are a few small placeholder texts to delete, but generally well done!


Reference to AB-3 diagrams code
* [Source URL](https://github.com/se-edu/addressbook-level3/tree/master/docs/diagrams)
* Used as reference to understand PlantUML syntax
Copy link

Choose a reason for hiding this comment

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

I like how you give credit to the sources you referenced!

enhancing maintainability and scalability.

#### Alternatives Considered:

Copy link

Choose a reason for hiding this comment

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

Good display of alternatives considered!


By abstracting out the parsing functionality of BookBuddy into a separate `ParserMain` class, the complexity of parsing
user input is removed from the main code. It is instead replaced by a simple interface for the user to work with, adhering to the abstraction
concept of object-oriented programming.
Copy link

Choose a reason for hiding this comment

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

Good application of OOP concepts!

Progress for each book can be recorded according to the number of pages read.
Users will be able to sort books according to genre.
Users can sort books according to Read or Unread.
Users will also be able to search for books via keywords in book titles

### Value proposition

{Describe the value proposition: what problem does it solve?}
Copy link

Choose a reason for hiding this comment

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

Perhaps you could add a value proposition so that developers can understand at a glance what your program can offer.

Reference to AB-3 diagrams code
* [Source URL](https://github.com/se-edu/addressbook-level3/tree/master/docs/diagrams)
* Used as reference to understand PlantUML syntax

## Design & implementation

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
Copy link

Choose a reason for hiding this comment

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

Perhaps you could remove the placeholder text so that it's clearer where the Design and Implementation section begins

@@ -1,29 +1,194 @@
# Developer Guide

## Table of Contents
Copy link

Choose a reason for hiding this comment

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

Good use of a Table of Contents to allow easy navigation of the DG!

liuzehui03 and others added 26 commits April 4, 2024 17:02
find based on read and unread status implemented
updated UG and list of commands in help
Add single step operation of set-genre method for pro users
fix bug such that capitalization of letters do not matter when using …
Fix bugs in genre and label commands
yeozongyao and others added 30 commits April 15, 2024 20:56
Bug fix - replace split criteria as \\s+ instead of space
Refactor code for set-genre feature for code readability
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.

None yet

7 participants