Skip to content

[Wang-Jingwei] iP#102

Open
Wang-Jingwei wants to merge 31 commits into
nus-cs2113-AY2122S2:masterfrom
Wang-Jingwei:master
Open

[Wang-Jingwei] iP#102
Wang-Jingwei wants to merge 31 commits into
nus-cs2113-AY2122S2:masterfrom
Wang-Jingwei:master

Conversation

@Wang-Jingwei

Copy link
Copy Markdown

No description provided.

Wang-Jingwei and others added 30 commits February 13, 2022 23:39
* branch-Level-5:
  Add some error handlings.
* branch-Level-6:
  Add the delete functionality.
* branch-Level-7:
  Add the functionality to load data from the file and save data into the file.
* master:
  Add the find functionality.

@Mick609 Mick609 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Readable code with some coding standard and naming convention issue.

Comment thread src/main/java/Duke.java
public void run() throws IOException {
String task;
ui.greetings();
Scanner in = new Scanner(System.in);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you should consider a more descriptive name for the Scanner object like "scanner"?

Comment thread src/main/java/Duke.java
String task;
ui.greetings();
Scanner in = new Scanner(System.in);
task=in.nextLine();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please make sure the spaces between operators are consistent.

Comment thread src/main/java/Duke.java
else{
try{
int index = Integer.parseInt(words[1]);
if(index>todolist.size()||index<=0) System.out.println("No task found.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you should consider having the if block in two lines.

Comment thread src/main/java/Duke.java
break;
}
case "find": {
if (words.length == 1) ui.incompleteMessage("find");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you could place the if and else blocks in more than a line.

*/

public static void save(String textToAdd,String filePath) throws IOException {
FileWriter fw = new FileWriter(filePath,false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you should consider a more descriptive name for the object like fileWriter.

*/
public ArrayList<Task> load() throws IOException {
ArrayList<Task> todolist = new ArrayList<Task>();
BufferedReader br = new BufferedReader(new FileReader(this.filePath));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Name like "bufferedReader" would be better in this case.

Comment thread src/main/java/Ui.java
* @param deadline Description of the task which will be added as Deadline.
*/
public void addDeadline(ArrayList<Task> taskList, String deadline){
String[] deadline1 = deadline.split("/by", 2);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you should use the plural form for the name of a String array.

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