domlimm / ip Goto Github PK
View Code? Open in Web Editor NEWThis project forked from nus-cs2103-ay2122s2/ip
Here's a mascot, Abby. She keeps track of your tasks!
ip's Introduction
ip's People
Contributors
ip's Issues
Sharing iP code quality feedback [for @domlimm] - Round 2
@domlimm We did an automated analysis of your code to detect potential areas to improve the code quality. We are sharing the results below, so that you can avoid similar problems in your tP code (which will be graded more strictly for code quality).
IMPORTANT: Note that the script looked for just a few easy-to-detect problems only, and at-most three example are given i.e., there can be other areas/places to improve.
Aspect: Tab Usage
No easy-to-detect issues 👍
Aspect: Naming boolean variables/methods
Example from src/main/java/duke/task/Task.java lines 9-9:
private boolean completed;Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-sounding prefix).You may ignore the above if you think the name already follows the convention (the script can report false positives in some cases)
Aspect: Brace Style
No easy-to-detect issues 👍
Aspect: Package Name Style
No easy-to-detect issues 👍
Aspect: Class Name Style
No easy-to-detect issues 👍
Aspect: Dead Code
No easy-to-detect issues 👍
Aspect: Method Length
Example from src/main/java/duke/commands/AddCommand.java lines 41-108:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "AddCommand[execute] taskList cannot be null.";
assert storage != null : "AddCommand[execute] storage cannot be null.";
String[] taskArr = null;
String type = "";
Task task;
String response = "Got it. I've added this task:\n";
String[] taskData = null;
try {
taskArr = this.fullCommand.split(" ", 2);
type = taskArr[0];
if (type.equalsIgnoreCase("deadline")) {
taskData = taskArr[1].split(" /by ");
LocalDate by = LocalDate.parse(taskData[1]);
task = new Deadline(taskData[0], by);
} else if (type.equalsIgnoreCase("event")) {
taskData = taskArr[1].split(" /at ");
LocalDate at = LocalDate.parse(taskData[1]);
task = new Event(taskData[0], at);
} else if (type.equalsIgnoreCase("todo")) {
if (taskArr[1].trim().length() == 0) {
throw new IndexOutOfBoundsException();
}
task = new Todo(taskArr[1]);
} else {
throw new IndexOutOfBoundsException();
}
taskList.add(task);
assert taskData != null : "AddCommand[execute] taskData cannot be null.";
storage.writeToFile(formatTaskString(task, taskData));
int noOfTasks = taskList.size();
String pluralTask = (noOfTasks > 1) ? "tasks" : "task";
response += " " + task + "\n";
response += "Now you have " + noOfTasks + " " + pluralTask + " in the list.";
return response;
} catch (IndexOutOfBoundsException e) {
if (Utils.isValidType(type)) {
if (Utils.isMissingData(taskArr)) {
response = "OOPS!!! Some data of your " + type + " task is missing. :-(";
return response;
}
response = "OOPS!!! The description of a " + type + " cannot be empty. :-(";
} else {
response = Constants.UNKNOWN_MSG;
}
return response;
} catch (IOException e) {
throw new DukeException(Constants.STORAGE_ADD_MSG);
} catch (DateTimeParseException e) {
throw new DukeException(Constants.INVALID_DATE_MSG);
}
}Example from src/main/java/duke/commands/SearchCommand.java lines 37-94:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "SearchCommand[execute] taskList cannot be null.";
assert storage != null : "SearchCommand[execute] storage cannot be null.";
String response = "";
try {
if (this.args.length() == 0) {
throw new IllegalArgumentException();
}
LocalDate date = LocalDate.parse(this.args);
int index = 0;
ArrayList<Task> allTasks = taskList.getTasks();
int length = allTasks.size();
StringBuilder sb = new StringBuilder();
if (length == 0) {
response = "No tasks found based on given date! Also, quit lazing around!";
return response;
}
sb.append("Here are the tasks with date, "
+ date.format(DateTimeFormatter.ofPattern("MMM dd yyyy"))
+ ", in your list:\n");
for (int i = 0; i < length; ++i) {
boolean shouldAppend = false;
Task task = allTasks.get(i);
if (task.getType() == 'D') {
Deadline deadline = (Deadline) task;
if (deadline.getDate().isEqual(date)) {
sb.append(++index + ". " + deadline.toString());
shouldAppend = true;
}
} else if (task.getType() == 'E') {
Event event = (Event) task;
if (event.getDate().isEqual(date)) {
sb.append(++index + ". " + event.toString());
shouldAppend = true;
}
}
if (shouldAppend) {
sb.append("\n");
}
}
response = index > 0 ? sb.toString() : Constants.NO_TASK_SEARCH_MSG;
return response;
} catch (Exception e) {
throw new DukeException(Constants.INVALID_DATE_MSG);
}
}Example from src/main/java/duke/commands/ToggleCommand.java lines 40-103:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "ToggleCommand[execute] taskList cannot be null.";
assert storage != null : "ToggleCommand[execute] storage cannot be null.";
String response;
try {
if (this.args.length() == 0) {
throw new IllegalArgumentException();
}
if (!Utils.isNumeric(this.args)) {
throw new NumberFormatException();
}
int index = Integer.parseInt(this.args);
Task updatedTask = taskList.toggleCompleted(this.isMark, --index);
String taskString = updatedTask.toString();
String updateString;
if (taskString.charAt(1) == 'D') {
String date = ((Deadline) updatedTask)
.getDate()
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
updateString = String.format(
"D | %s | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription(),
date);
} else if (taskString.charAt(1) == 'E') {
String date = ((Event) updatedTask)
.getDate()
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
updateString = String.format(
"E | %s | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription(),
date);
} else {
updateString = String.format(
"T | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription());
}
storage.writeToFile(updateString, index, false);
String output = (isMark)
? "Nice! I've marked this task as done:\n"
: "OK, I've marked this task as not done yet:\n";
response = output + taskString;
return response;
} catch (IndexOutOfBoundsException e) {
throw new DukeException(Constants.INVALID_INDEX_MSG);
} catch (IOException e) {
throw new DukeException(Constants.STORAGE_UPDATE_MSG);
} catch (IllegalArgumentException e) {
throw new DukeException(Constants.INVALID_MARK_MSG);
}
}Suggestion: Consider applying SLAP (and other abstraction mechanisms) to shorten methods. You may ignore this suggestion if you think a longer method is justified in a particular case.
Aspect: Class size
No easy-to-detect issues 👍
Aspect: Header Comments
No easy-to-detect issues 👍
Aspect: Recent Git Commit Message (Subject Only)
No easy-to-detect issues 👍
Aspect: Binary files in repo
No easy-to-detect issues 👍
❗ You are not required to (but you are welcome to) fix the above problems in your iP, unless you have been separately asked to resubmit the iP due to code quality issues.
ℹ️ The bot account used to post this issue is un-manned. Do not reply to this post (as those replies will not be read). Instead, contact [email protected] if you want to follow up on this post.
Sharing iP code quality feedback [for @domlimm]
@domlimm We did an automated analysis of your code to detect potential areas to improve the code quality. We are sharing the results below, to help you improve the iP code further.
IMPORTANT: Note that the script looked for just a few easy-to-detect problems only, and at-most three example are given i.e., there can be other areas/places to improve.
Aspect: Tab Usage
No easy-to-detect issues 👍
Aspect: Naming boolean variables/methods
Example from src/main/java/duke/task/Task.java lines 9-9:
private boolean completed;Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-sounding prefix).You may ignore the above if you think the name already follows the convention (the script can report false positives in some cases)
Aspect: Brace Style
No easy-to-detect issues 👍
Aspect: Package Name Style
No easy-to-detect issues 👍
Aspect: Class Name Style
No easy-to-detect issues 👍
Aspect: Dead Code
No easy-to-detect issues 👍
Aspect: Method Length
Example from src/main/java/duke/commands/AddCommand.java lines 41-108:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "AddCommand[execute] taskList cannot be null.";
assert storage != null : "AddCommand[execute] storage cannot be null.";
String[] taskArr = null;
String type = "";
Task task;
String response = "Got it. I've added this task:\n";
String[] taskData = null;
try {
taskArr = this.fullCommand.split(" ", 2);
type = taskArr[0];
if (type.equalsIgnoreCase("deadline")) {
taskData = taskArr[1].split(" /by ");
LocalDate by = LocalDate.parse(taskData[1]);
task = new Deadline(taskData[0], by);
} else if (type.equalsIgnoreCase("event")) {
taskData = taskArr[1].split(" /at ");
LocalDate at = LocalDate.parse(taskData[1]);
task = new Event(taskData[0], at);
} else if (type.equalsIgnoreCase("todo")) {
if (taskArr[1].trim().length() == 0) {
throw new IndexOutOfBoundsException();
}
task = new Todo(taskArr[1]);
} else {
throw new IndexOutOfBoundsException();
}
taskList.add(task);
assert taskData != null : "AddCommand[execute] taskData cannot be null.";
storage.writeToFile(formatTaskString(task, taskData));
int noOfTasks = taskList.size();
String pluralTask = (noOfTasks > 1) ? "tasks" : "task";
response += " " + task + "\n";
response += "Now you have " + noOfTasks + " " + pluralTask + " in the list.";
return response;
} catch (IndexOutOfBoundsException e) {
if (Utils.isValidType(type)) {
if (Utils.isMissingData(taskArr)) {
response = "OOPS!!! Some data of your " + type + " task is missing. :-(";
return response;
}
response = "OOPS!!! The description of a " + type + " cannot be empty. :-(";
} else {
response = Constants.UNKNOWN_MSG;
}
return response;
} catch (IOException e) {
throw new DukeException(Constants.STORAGE_ADD_MSG);
} catch (DateTimeParseException e) {
throw new DukeException(Constants.INVALID_DATE_MSG);
}
}Example from src/main/java/duke/commands/SearchCommand.java lines 37-94:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "SearchCommand[execute] taskList cannot be null.";
assert storage != null : "SearchCommand[execute] storage cannot be null.";
String response = "";
try {
if (this.args.length() == 0) {
throw new IllegalArgumentException();
}
LocalDate date = LocalDate.parse(this.args);
int index = 0;
ArrayList<Task> allTasks = taskList.getTasks();
int length = allTasks.size();
StringBuilder sb = new StringBuilder();
if (length == 0) {
response = "No tasks found based on given date! Also, quit lazing around!";
return response;
}
sb.append("Here are the tasks with date, "
+ date.format(DateTimeFormatter.ofPattern("MMM dd yyyy"))
+ ", in your list:\n");
for (int i = 0; i < length; ++i) {
boolean shouldAppend = false;
Task task = allTasks.get(i);
if (task.getType() == 'D') {
Deadline deadline = (Deadline) task;
if (deadline.getDate().isEqual(date)) {
sb.append(++index + ". " + deadline.toString());
shouldAppend = true;
}
} else if (task.getType() == 'E') {
Event event = (Event) task;
if (event.getDate().isEqual(date)) {
sb.append(++index + ". " + event.toString());
shouldAppend = true;
}
}
if (shouldAppend) {
sb.append("\n");
}
}
response = index > 0 ? sb.toString() : Constants.NO_TASK_SEARCH_MSG;
return response;
} catch (Exception e) {
throw new DukeException(Constants.INVALID_DATE_MSG);
}
}Example from src/main/java/duke/commands/ToggleCommand.java lines 40-103:
public String execute(TaskList taskList, Storage storage) throws DukeException {
assert taskList != null : "ToggleCommand[execute] taskList cannot be null.";
assert storage != null : "ToggleCommand[execute] storage cannot be null.";
String response;
try {
if (this.args.length() == 0) {
throw new IllegalArgumentException();
}
if (!Utils.isNumeric(this.args)) {
throw new NumberFormatException();
}
int index = Integer.parseInt(this.args);
Task updatedTask = taskList.toggleCompleted(this.isMark, --index);
String taskString = updatedTask.toString();
String updateString;
if (taskString.charAt(1) == 'D') {
String date = ((Deadline) updatedTask)
.getDate()
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
updateString = String.format(
"D | %s | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription(),
date);
} else if (taskString.charAt(1) == 'E') {
String date = ((Event) updatedTask)
.getDate()
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd"));
updateString = String.format(
"E | %s | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription(),
date);
} else {
updateString = String.format(
"T | %s | %s",
updatedTask.getCompleted() ? "1" : "0",
updatedTask.getDescription());
}
storage.writeToFile(updateString, index, false);
String output = (isMark)
? "Nice! I've marked this task as done:\n"
: "OK, I've marked this task as not done yet:\n";
response = output + taskString;
return response;
} catch (IndexOutOfBoundsException e) {
throw new DukeException(Constants.INVALID_INDEX_MSG);
} catch (IOException e) {
throw new DukeException(Constants.STORAGE_UPDATE_MSG);
} catch (IllegalArgumentException e) {
throw new DukeException(Constants.INVALID_MARK_MSG);
}
}Suggestion: Consider applying SLAP (and other abstraction mechanisms) to shorten methods. You may ignore this suggestion if you think a longer method is justified in a particular case.
Aspect: Class size
No easy-to-detect issues 👍
Aspect: Header Comments
Example from src/main/java/duke/main/Duke.java lines 33-36:
/**
* You should have your own function to generate a response to user input.
* Replace this stub with your completed method.
*/Example from src/main/java/duke/ui/Ui.java lines 40-42:
/**
* Display a message when there is an issue loading from the text file that holds tasks.
*/Suggestion: Ensure method/class header comments follow the format specified in the coding standard, in particular, the phrasing of the overview statement
Aspect: Recent Git Commit Message (Subject Only)
No easy-to-detect issues 👍
ℹ️ The bot account @nus-se-bot used to post this issue is un-manned. Do not reply to this post (as those replies will not be read). Instead, contact [email protected] if you want to follow up on this post.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.