Skip to content

[Ryan Jee] iP#268

Open
rjeez wants to merge 193 commits into
nus-cs2103-AY2021S2:masterfrom
rjeez:master
Open

[Ryan Jee] iP#268
rjeez wants to merge 193 commits into
nus-cs2103-AY2021S2:masterfrom
rjeez:master

Conversation

@rjeez
Copy link
Copy Markdown

@rjeez rjeez commented Feb 6, 2021

No description provided.

Copy link
Copy Markdown

@danielonges danielonges left a comment

Choose a reason for hiding this comment

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

Generally code is quite well written! However, might want to be careful in leaving spaces where necessary in order not to violate coding standards. Other than that, LGTM! ☺️

Comment thread src/main/java/Duke.java Outdated


public Duke(String filePath){
ui = new Ui();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might want to consider removing extra space(s) between new Ui()

Comment thread src/main/java/Duke.java Outdated
Command command = parser.parse(input);
String reply = command.execute(ui,taskList, storage);
storage.writeFile();
assert(reply!=null);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of assertions!

Comment thread src/main/java/Storage.java Outdated
DukeList dukeList = new DukeList();
StringBufferTasks = new StringBuffer();
try{
BufferedReader reader = new BufferedReader(new InputStreamReader(new DataInputStream(new FileInputStream(this.filename))));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might have exceeded line limit

Comment thread src/main/java/Storage.java Outdated
Path filePath = Path(current,filename);
boolean directoryExist = Files.exists(direcPath);
boolean fileExist = Files.exists(filePath);
try{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might want to include spacing in between try {

Comment thread src/main/java/Ui.java Outdated

public String showTaskAdded(String task, List<Task> taskList){
assert(task != null && taskList != null);
return String.format( "Added the following task : \n" + "%s\n" + "You now have %d tasks in your list.\n", task, taskList.size());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might have exceeded line limit

Comment thread src/main/java/Storage.java Outdated

public Storage (String filename){
this.filename = new File(filename);
accessTaskListInFileSystem(getCurrentDirectory());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clear and concise naming of method

Comment thread src/main/java/tasktype/Task.java Outdated
return " [" + type + "] " + " [" + getStatusIcon() + "] " + getname();
}
else{
return " [" + type + "] " + " [" + getStatusIcon() + "] " + getname() + " " + dateTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd kkmm"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might have exceeded line limit

Add CBye class for command bye
Add CDeadline class for command deadline
Add CDelete class for command delete
Add CDone class for command done
Add CEvent class for command event
Add CList class for command list
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