[CS2113-T12-1] News on the Go #5
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JeffinsonDarmawan
pushed a commit
to JeffinsonDarmawan/tp
that referenced
this pull request
Mar 14, 2024
…utus Added Samuel Ong About Us
yeozongyao
added a commit
to yeozongyao/tp
that referenced
this pull request
Mar 15, 2024
Zong Yao Main master
kirangeofran
reviewed
Apr 3, 2024
docs/DeveloperGuide.md
Outdated
|
|
||
| #### Alternatives Considered | ||
| - **Cloud-Based Storage:** Providing cross-device synchronization was deemed unnecessary at this stage, given the application's primary focus on delivering a personalized news experience on individual devices. | ||
|
|
||
| ## Product scope |
There was a problem hiding this comment.
can add in more diagrams for a clearer explanation :)
|
|
||
| #### Alternatives Considered | ||
| - **Cloud-Based Storage:** Providing cross-device synchronization was deemed unnecessary at this stage, given the application's primary focus on delivering a personalized news experience on individual devices. | ||
|
|
||
| ## Product scope | ||
| ### Target user profile | ||
|
|
There was a problem hiding this comment.
The flow of control from UI to DailyNewsCommand and back to UI is not very clear. You could show explicit method calls from the UI to DailyNewsCommand and any data or control flow that returns to the UI.
yzhanglp
reviewed
Apr 4, 2024
docs/DeveloperGuide.md
Outdated
|
|
||
| Below is a snippit of the parser function that handles both of the operations above: | ||
| ```java | ||
| private static void saveDailyArticlesParser() { |
There was a problem hiding this comment.
Good for given code, may consider use pseudo-code to increase readibility
docs/DeveloperGuide.md
Outdated
| - `FilterNewsCommand#info()` — displays the importance, reliability and bias measure of the article to the user. | ||
| - `FilterNewsCommand#back()` — Exits the filter topic feature. | ||
|
|
||
| Given Below is an example usage scenario and how the filter and topic mechanism behaves at each step. |
docs/DeveloperGuide.md
Outdated
| The `Topic` function is complemented by the `Filter` Function which displays the list of articles related to the | ||
| specified topic. | ||
|
|
||
| #### Filter Function |
| } | ||
| ``` | ||
|
|
||
| #### Design Considerations |
There was a problem hiding this comment.
I think this part is good to write the consideration!
Vavinan
reviewed
Apr 4, 2024
docs/DeveloperGuide.md
Outdated
| .filter(article -> article.endsWith(";" + topic)) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| if (!articlesForTopic.isEmpty()) { |
There was a problem hiding this comment.
Consider using pseudo-code or algorithm to increase readability
| Alternative 1 (current choice): check for topicIndex in handleCommand | ||
| - Pros: easy to implement | ||
| - Con: duplicate checking of topicIndex for article commands | ||
|
|
There was a problem hiding this comment.
Its good that you provided some alternatives with their pros and cons
docs/DeveloperGuide.md
Outdated
| is used to retrieve the source of a news article. | ||
| The function takes in a string and a list of | ||
| `NewsArticle` objects. The string is split into an | ||
| array and the second element (index 1) is parsed as |
There was a problem hiding this comment.
Isn't it just index. The usage of "index 1" is not clear here.
docs/DeveloperGuide.md
Outdated
| topics of the news articles. | ||
|
|
||
| The following sequence diagram shows how the topic operation works. | ||
| <img src="UML Diagrams/topicFunctionSequence.png"> |
There was a problem hiding this comment.
Shouldn't it be "opt" instead of "alt" as it only has one path
Update User Guide
please accept my pr; added Ui stuff plus updated documentation
Final DG changes
…Branch * 'master' of https://github.com/BenjoBurger/tp: Final DG changes (Adjust line length and import Resolve conflicts Update DG, Add extract to DG, fix test case Update DG design sections, add extract to DG, Fix test cases
Update PPP
Update PPP links
quote fix
…benjoBranch * 'benjoBranch' of https://github.com/BenjoBurger/tp: quote fix quote and daily fix at hiteshri's request Update PPP links remove duplicate extract Update PPP
Update to UG and DG
Update PPP
edit UI for daily function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
News on the Go is a desktop command line app for getting your news on the go, i.e. quickly, conveniently, accurately, unbiased, and relevant to you. If you type fast, News on the Go will get your daily news to you faster than browsing Google.