wrong commit by sallysohyun · Pull Request #94 · nusCS2113-AY1920S1/PersonalAssistant-Duke

sallysohyun · 2019-10-17T15:02:02Z

fixed comments

nus-se-pr-bot · 2019-10-17T15:02:08Z

Hi @sallysohyun, your pull request title is not as specified. Please rectify the PR title to match the following requirement.

The PR name should be in the format of [Semester-Module Code-Tutorial Slot-Team ID] Product Name. For example:

If you are from CS2113, [AY1920S1-CS2113-T13-1] Contact List Pro;
If you are from CS2113T, [AY1920S1-CS2113T-W13-1] Contact List Pro.

Please follow the above format strictly and edit your title for reprocessing.

Note: this comment is posted by a bot. If you believe this is done in error, please create an issue at nus-se-pr-bot and add a link to this PR.

Rishav1

@sallysohyun Good PR! I have added some suggestions and scope for improvements to reduce redundancy of several functions.

Overall, you did a good Job!

Rishav1 · 2019-10-22T05:55:39Z

src/main/java/Duke.java

@@ -1,10 +1,21 @@
+import executor.task.TaskList;


First line should be package name.

Rishav1 · 2019-10-22T05:58:10Z

src/main/java/Duke.java

+    protected static TaskList taskList;
+
+    /**
+     * The Main method by which Duke will be launched.


Probably this can be deleted as it is not adding any value. Main methods are always unique and obvious to identify.

Rishav1 · 2019-10-22T06:01:11Z

src/main/java/duke/exception/DukeException.java

@@ -0,0 +1,11 @@
+package duke.exception;
+
+public class DukeException extends Exception {


DukeException should the base exception class for other, more specific exception types related to the application. If you get DukeException error, it gives you no more information about the type other than the message string.

Rishav1 · 2019-10-22T06:02:00Z

src/main/java/executor/command/Command.java

+
+public abstract class Command {
+    protected Boolean exitRequest = false;
+    protected CommandType commandType;


Maybe initialize the command type as a no-op type.

Rishav1 · 2019-10-22T06:09:51Z

src/main/java/executor/command/CommandAddIncomeReceipt.java

+    public CommandAddIncomeReceipt(String userInput) {
+        this.commandType = CommandType.IN;
+        this.userInput = userInput;
+        this.description = "You can add a new income receipt in format of 'In $5.00 /tags tag'.";


This line seems too long. Maybe create a private static final string that stores this default description and assign it inside the constructor.

Rishav1 · 2019-10-22T06:30:01Z

src/main/java/executor/task/Task.java

+            generatedStr += " (";
+            if (this.detailDesc != null && !this.detailDesc.isEmpty()) {
+                generatedStr += this.detailDesc + ": ";
+            }
+            if (this.taskDetails != null && !this.taskDetails.isEmpty()) {
+                generatedStr += this.taskDetails;
+            }
+            generatedStr += ")";


Violation of Single Layer of Abstraction Principle (SLAP).

Rishav1 · 2019-10-22T06:30:15Z

src/main/java/executor/task/Task.java

+            int indexMsg = userInput.indexOf(' ', indexBackslash);
+            if (indexMsg >= 0) {
+                this.detailDesc = userInput.substring(indexBackslash + 1, indexMsg);
+            }
+            String[] splitDetails = userInput.split('/' + this.detailDesc, 2);
+            this.taskName = splitDetails[0].trim();
+            if (splitDetails.length > 1) {
+                this.taskDetails = splitDetails[1].trim();
+            }


Violation of Single Layer of Abstraction Principle (SLAP).

Rishav1 · 2019-10-22T06:31:08Z

src/main/java/executor/task/TaskType.java

+     * @return A String Array that contains all the types of this enum
+     */
+    public static String[] getNames() {
+        TaskType[] holder = TaskType.values();


Maybe taskTypes is a better name.

Rishav1 · 2019-10-22T06:32:06Z

src/main/java/storage/Storage.java

+            while (scanner.hasNextLine()) {
+                try {
+                    String loadedInput = scanner.nextLine();
+                    newTask = loadTaskFromStorageString(loadedInput);
+                    taskList.addTask(newTask);
+                } catch (Exception e) {
+                    System.out.println(e);
+                }
+            }


Violation of Single Layer of Abstraction Principle (SLAP).

Rishav1 · 2019-10-22T06:32:20Z

src/main/java/storage/Storage.java

+        for (String taskString : taskStrings) {
+            if (newTask == null) {
+                newTask = TaskList.createTaskFromString(taskString);
+            } else {
+                queuedTask = TaskList.createTaskFromString(taskString);
+                queuedTasks.getList().add(queuedTask);
+            }


Violation of Single Layer of Abstraction Principle (SLAP).

f

…xceptions-currencyconverter Handled exceptions for Currency Converter to resolve issues AY1920S1-CS2113T-F09-1#199 , AY1920S1-CS2113T-F09-1#189 , AY1920S1-CS2113T-F09-1#183 , AY1920S1-CS2113T-F09-1#155 , AY1920S1-CS2113T-F09-1#130

…ws errors

…ser and prevent invalid input by user

…me formatting

…n-for-expenditure Added exceptions to expenditure

…k-change-help-formatting Added function to help command .

…ator Added simple arithmetic command line based functionalities

…r-exporter Tester exporter

Removed taglist

…CommandEdit

New feature CommandEdit added with Junit testing

Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

…CS2113T-F09-1#315) * Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Moved Classes to their proper places. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

* Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Fixed height issue. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

sf

Code optimised and added some testing

…tor-again-export Done with refactor of export command

* Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Fixed height issue. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * LAST BUGFIX Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

…ll now be ignored. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

ad

nus-se-pr-bot added tutor.Rishav1 team.1 FormatCheckRequested tutorial.CS2113T-F09 labels Oct 17, 2019

sallysohyun changed the title ~~Add CommandAddIncomeReceipt and CommandAddSpendingReceipt~~ wrong commit Oct 17, 2019

Rishav1 reviewed Oct 22, 2019

View reviewed changes

Araavinds and others added 23 commits November 4, 2019 20:40

Update CommandDiv.java

94963b6

Update CommandMul.java

fedc7c8

Update CommandSub.java

7008ab4

trying to make setbalance a one time implementation

971e533

Made changes after reviewing comments

390d770

Merge pull request #4 from AY1920S1-CS2113T-F09-1/master

e4ea9c9

f

now setbalance can only be executed once

617c178

refactored Date to LocalDate in Task Class and Command Reminder

085265d

Added expendday & checkstyle done

d09ad55

refactoring commandschedule

80974a7

refactored command schedule

c117f34

refactored command schedule and command reminder but now deadline sho…

e8eb5b8

…ws errors

refactored deadline

71a5598

refactored todo

3b7f6fc

Added function to help to find explanation for specific commands by u…

2df26d0

…ser and prevent invalid input by user

refactored reminder, schedule, todo, deadline to make the date and ti…

2b447b7

…me formatting

Merge pull request AY1920S1-CS2113T-F09-1#217 from AjeyAshok/exceptio…

e162e86

…n-for-expenditure Added exceptions to expenditure

Commiting new help

c006bf8

Merge branch 'master' into AjeyAshok-change-help-formatting

0f5ffaf

Merge pull request AY1920S1-CS2113T-F09-1#225 from AjeyAshok/AjeyAsho…

9f4b234

…k-change-help-formatting Added function to help command .

Merge pull request AY1920S1-CS2113T-F09-1#218 from Araavinds/b-calcul…

b8307e7

…ator Added simple arithmetic command line based functionalities

Merge branch 'master' into reminder-fix

037d462

priyan-coder and others added 30 commits November 11, 2019 16:35

Update testWalletDataSave.txt

34067d9

Update savedWallet.txt

3096230

Merge pull request AY1920S1-CS2113T-F09-1#311 from priyan-coder/teste…

893899d

…r-exporter Tester exporter

Removed taglist

fd46310

Added more extensive testing to Command Stats

ea4ca0b

Merge pull request AY1920S1-CS2113T-F09-1#313 from Araavinds/b-RemoveTag

b29f09c

Removed taglist

Merge branch 'master' into CommandEdit

3e6640c

Final changes

adb45c3

Merge branch 'CommandEdit' of https://github.com/AjeyAshok/main into …

411eac4

…CommandEdit

All test cases rechecked

3fbf638

Merge pull request AY1920S1-CS2113T-F09-1#277 from AjeyAshok/CommandEdit

c9aece6

New feature CommandEdit added with Junit testing

Made some minor bugfixes. (AY1920S1-CS2113T-F09-1#314)

3d847b5

Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

Moves Java Classes to the layers they're supposed to be in (AY1920S1-…

518bec4

…CS2113T-F09-1#315) * Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Moved Classes to their proper places. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

Moved Stats Window up (AY1920S1-CS2113T-F09-1#316)

ae9e91f

* Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Fixed height issue. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

Merge pull request #8 from AY1920S1-CS2113T-F09-1/master

64a511b

sf

Update README.adoc

998e08a

Update README.adoc

21e799e

Delete README.md

4351219

Update AboutUs.adoc

7355b9c

Update AboutUs.adoc

136314c

code optimised and added some testing

1b915f1

Merge pull request AY1920S1-CS2113T-F09-1#317 from Araavinds/optimise

deb95f3

Code optimised and added some testing

done

baf0f2a

Merge branch 'master' into refactor-again-export

a37ac7e

Merge pull request AY1920S1-CS2113T-F09-1#318 from priyan-coder/refac…

d925546

…tor-again-export Done with refactor of export command

Last branch (AY1920S1-CS2113T-F09-1#319)

217c261

* Made some minor bugfixes. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * Fixed height issue. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com> * LAST BUGFIX Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

User inputs not corresponding to a particular CommandType/TaskType wi…

25fc3d4

…ll now be ignored. Signed-off-by: Mudaafi <SleepyUndergrad@gmail.com>

Added User Guide and Developer Guide

cedc941

Updated Docs

568cf15

Merge pull request #9 from AY1920S1-CS2113T-F09-1/master

4596d9a

ad

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wrong commit#94

wrong commit#94
sallysohyun wants to merge 506 commits intonusCS2113-AY1920S1:masterfrom
sallysohyun:master

sallysohyun commented Oct 17, 2019

Uh oh!

nus-se-pr-bot commented Oct 17, 2019

Uh oh!

Rishav1 left a comment

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Rishav1 Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		@@ -0,0 +1,11 @@
		package duke.exception;

		public class DukeException extends Exception {

Conversation

sallysohyun commented Oct 17, 2019

Uh oh!

nus-se-pr-bot commented Oct 17, 2019

Uh oh!

Rishav1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants