Skip to content

Commit

Permalink
Merge branch 'master' of github.com:AY2425S1-CS2113-W13-4/tp into Zih…
Browse files Browse the repository at this point in the history
…an-Test-Storage

* 'master' of github.com:AY2425S1-CS2113-W13-4/tp:
  Edit DG to fix issues
  Update GradeStorage uml
  Fix codestyle violatons
  Update AddStudentCommandTest for both null
  Update tests
  Add more descriptive NULL_PARAMETER_ERROR messages
  • Loading branch information
jinzihan2002 committed Nov 11, 2024
2 parents d6878a2 + 30418f7 commit f7e6adb
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 42 deletions.
62 changes: 32 additions & 30 deletions docs/DeveloperGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ All commands follow the sequence as described in the diagram below:

![ArchitectureSequenceGrouped.png](diagrams/ArchitectureSequenceGrouped.png)

Where <code>ref</code> frame is a placeholder for each command's specific operations.
Where <code>ref</code> frame is a placeholder for each command's specific operations:
- [setup](#setup)
- [specific command execution](#adddelete-studentcomponent-feature)

#### Setup:
### Setup:

During the setup phase of `TutorLink`, the following operations are performed:
1. `StudentStorage`, `ComponentStorage` and `GradeStorage` objects are instantiated
Expand All @@ -70,6 +72,34 @@ During the setup phase of `TutorLink`, the following operations are performed:

The specific implementation of noteworthy operations are presented below:

### Storage Load feature

#### Implementation Details

The `StudentStorage`, `GradeStorage` and `ComponentStorage` classes implement the feature to load data from the
data `.txt` files into their respective List objects at the start of the program.

The load list methods for the Storage classes have largely similar logic flows. To avoid repetition,
only the implementation for `GradeStorage` is shown.

The following section and sequence diagram elaborate on the implementation of the `loadGradeList` method in `GradeStorage`,
as referenced in [Setup](#setup):

![GradeStorage.png](diagrams/GradeStorage.png)

1. TutorLink constructs a new `GradeStorage`.
2. `GradeStorage` creates a new `ArrayList` of `String`s for discarded entries.
3. TutorLink calls `loadGradeList`.
4. `GradeStorage` creates a new `ArrayList` of `Grade`s.
5. While there are next lines in the data file:
- FileScanner returns the current file line as a String and moves to the next file line.
- `GradeStorage` calls its `getGradeFromFileLine` method with the file line.
- If the file line references a valid `Component` and a valid `Student`, a `Grade` is returned and added to the `ArrayList`.
- If not (e.g. file line was corrupted), the file line is added to `discardedEntries`,
and the loop continues to the next iteration.
6. The `ArrayList` of `Grade`s is returned to TutorLink.
7. TutorLink calls `getDiscardedEntries`, and the discarded entries are displayed by UI.

### Add/Delete Student/Component Feature

#### Implementation Details
Expand Down Expand Up @@ -158,34 +188,6 @@ The sequence diagram of the DeleteGradeCommand is shown below.

![DeleteGradeCommand.png](diagrams/DeleteGradeCommand.png)

### Storage Load feature

#### Implementation Details

The `StudentStorage`, `GradeStorage` and `ComponentStorage` classes implement the feature to load data from the
data `.txt` files into their respective List objects at the start of the program.

The load list methods for the Storage classes have largely similar logic flows. To avoid repetition,
only the implementation for `GradeStorage` is shown.

The following section and sequence diagram elaborate on the implementation of the `loadGradeList` method in `GradeStorage`,
as referenced in [Setup](#setup):

![GradeStorage.png](diagrams/GradeStorage.png)

1. TutorLink constructs a new `GradeStorage`.
2. `GradeStorage` creates a new `ArrayList` of `String`s for discarded entries.
3. TutorLink calls `loadGradeList`.
4. `GradeStorage` creates a new `ArrayList` of `Grade`s.
5. While there are next lines in the data file:
- FileScanner returns the current file line as a String and moves to the next file line.
- `GradeStorage` calls its `getGradeFromFileLine` method with the file line.
- If the file line references a valid `Component` and a valid `Student`, a `Grade` is returned and added to the `ArrayList`.
- If not (e.g. file line was corrupted), the file line is added to `discardedEntries`,
and the loop continues to the next iteration.
6. The `ArrayList` of `Grade`s is returned to TutorLink.
7. TutorLink calls `getDiscardedEntries`, and the discarded entries are displayed by UI.

## Appendix A: Product Scope

### Target User Profile
Expand Down
Binary file modified docs/diagrams/GradeStorage.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion docs/diagrams/GradeStorage.puml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ GS <-- AL: grades: Grade
deactivate AL

loop file has next line
GS -> GS: getGradeFromFileLine(currentLine: String)
GS -> GS: getGradeFromFileLine(currentLine: String, grades: Grade)
activate GS
GS --> GS: newGrade: Grade
deactivate GS
Expand All @@ -53,6 +53,8 @@ end
TL <-- GS: grades: Grade
deactivate GS

destroy AL

TL -> GS: getDiscardedEntries()
activate GS
TL <-- GS: discardedGrades: String
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/tutorlink/command/AddComponentCommand.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package tutorlink.command;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;

import tutorlink.appstate.AppState;
import tutorlink.commons.Commons;
Expand Down Expand Up @@ -57,7 +59,18 @@ public CommandResult execute(AppState appState, HashMap<String, String> hashmap)
private void validateArguments(String componentName, String weightageNumber, String maxScoreNumber)
throws IllegalValueException {
if (componentName == null || weightageNumber == null || maxScoreNumber == null) {
throw new IllegalValueException(Commons.ERROR_NULL);
List<String> nullParameters = new ArrayList<>();
if (componentName == null) {
nullParameters.add(ARGUMENT_PREFIXES[0]);
}
if (weightageNumber == null) {
nullParameters.add(ARGUMENT_PREFIXES[1]);
}
if (maxScoreNumber == null) {
nullParameters.add(ARGUMENT_PREFIXES[2]);
}
throw new IllegalValueException(String.format(Commons.ERROR_NULL,
String.join(", ", nullParameters)));
}
}

Expand Down
13 changes: 12 additions & 1 deletion src/main/java/tutorlink/command/AddStudentCommand.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package tutorlink.command;

import java.util.ArrayList;
import java.util.HashMap;

import tutorlink.appstate.AppState;
import tutorlink.commons.Commons;
import tutorlink.exceptions.IllegalValueException;
import tutorlink.exceptions.TutorLinkException;
import tutorlink.result.CommandResult;

import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -20,7 +23,15 @@ public CommandResult execute(AppState appState, HashMap<String, String> hashmap)
String matricNumber = hashmap.get(ARGUMENT_PREFIXES[0]);
String name = hashmap.get(ARGUMENT_PREFIXES[1]);
if (matricNumber == null || name == null) {
throw new IllegalValueException(Commons.ERROR_NULL);
List<String> nullParameters = new ArrayList<>();
if (matricNumber == null) {
nullParameters.add(ARGUMENT_PREFIXES[0]);
}
if (name == null) {
nullParameters.add(ARGUMENT_PREFIXES[1]);
}
throw new IllegalValueException(String.format(Commons.ERROR_NULL,
String.join(", ", nullParameters)));
}
Pattern pattern = Pattern.compile(Commons.MATRIC_NUMBER_REGEX);
Matcher matcher = pattern.matcher(matricNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class DeleteComponentCommand extends Command{
public CommandResult execute(AppState appState, HashMap<String, String> hashmap) throws TutorLinkException {
String componentName = hashmap.get(ARGUMENT_PREFIXES[0]);
if(componentName == null) {
throw new IllegalValueException(Commons.ERROR_NULL);
throw new IllegalValueException(String.format(Commons.ERROR_NULL,ARGUMENT_PREFIXES[0]));
}
ComponentList componentsToDelete = appState.components.findComponent(componentName);
if(componentsToDelete.size() == 0) {
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/tutorlink/command/DeleteGradeCommand.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package tutorlink.command;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -59,7 +61,15 @@ public CommandResult execute(AppState appState, HashMap<String, String> hashmap)
*/
private void validateArguments(String matricNumber, String componentDescription) throws IllegalValueException {
if (matricNumber == null || componentDescription == null) {
throw new IllegalValueException(Commons.ERROR_NULL);
List<String> nullParameters = new ArrayList<>();
if (matricNumber == null) {
nullParameters.add(ARGUMENT_PREFIXES[0]);
}
if (componentDescription == null) {
nullParameters.add(ARGUMENT_PREFIXES[1]);
}
throw new IllegalValueException(String.format(Commons.ERROR_NULL,
String.join(", ", nullParameters)));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tutorlink/command/DeleteStudentCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class DeleteStudentCommand extends Command {
public CommandResult execute(AppState appState, HashMap<String, String> hashmap) throws TutorLinkException {
String matricNumber = hashmap.get(ARGUMENT_PREFIXES[0]);
if (matricNumber == null) {
throw new IllegalValueException(Commons.ERROR_NULL);
throw new IllegalValueException(String.format(Commons.ERROR_NULL, ARGUMENT_PREFIXES[0]));
}
Pattern pattern = Pattern.compile(Commons.MATRIC_NUMBER_REGEX);
Matcher matcher = pattern.matcher(matricNumber);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tutorlink/commons/Commons.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

public class Commons {

public static final String ERROR_NULL = "Error! Null parameter passed!";
public static final String ERROR_NULL = "Error! Null parameters %s passed!";

//@@author RCPilot1604

Expand Down
4 changes: 2 additions & 2 deletions src/test/java/tutorlink/command/AddComponentCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void execute_nullArguments_throwsIllegalValueException() {
IllegalValueException exception = assertThrows(IllegalValueException.class, () -> {
command.execute(appState, arguments);
});
assertEquals("Error! Null parameter passed!", exception.getMessage());
assertEquals("Error! Null parameters c/, w/, m/ passed!", exception.getMessage());
assertEquals(initialWeighting, appState.components.getTotalWeighting());
}

Expand All @@ -63,7 +63,7 @@ void execute_missingWeightageArgument_throwsIllegalValueException() {
IllegalValueException exception = assertThrows(IllegalValueException.class, () -> {
command.execute(appState, arguments);
});
assertEquals("Error! Null parameter passed!", exception.getMessage());
assertEquals("Error! Null parameters w/ passed!", exception.getMessage());
assertEquals(initialWeighting, appState.components.getTotalWeighting());
}

Expand Down
16 changes: 15 additions & 1 deletion src/test/java/tutorlink/command/AddStudentCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,21 @@ void execute_emptyInput_exceptionThrown() {
fail("Expected an exception to be thrown due to empty input");
} catch (IllegalValueException e) {
// Assert that the exception message matches the expected outcome
assertEquals(Commons.ERROR_NULL, e.getMessage());
assertEquals(String.format(Commons.ERROR_NULL, command.getArgumentPrefixes()[1]), e.getMessage());
} catch (Exception e) {
// If any other type of exception is thrown, fail the test
fail("Expected IllegalArgumentException, but got: " + e.getClass().getSimpleName());
}
}

@Test
void execute_bothNullInputs_exceptionThrown() {
try {
CommandResult result = command.execute(appState, arguments);
fail("Expected an exception to be thrown due to empty input");
} catch (IllegalValueException e) {
// Assert that the exception message matches the expected outcome
assertEquals("Error! Null parameters i/, n/ passed!", e.getMessage());
} catch (Exception e) {
// If any other type of exception is thrown, fail the test
fail("Expected IllegalArgumentException, but got: " + e.getClass().getSimpleName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void deleteComponent_emptyParam_fail() {
try {
command.execute(appState, arguments);
} catch (IllegalValueException e) {
assertEquals(e.getMessage(), Commons.ERROR_NULL);
assertEquals(e.getMessage(), String.format(Commons.ERROR_NULL, command.getArgumentPrefixes()[0]));
} catch (Exception e) {
fail("Expected: ComponentNotFoundException, actual: " + e.getMessage());
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void execute_delete_matricNull() {
try {
result = deleteCommand.execute(appState, arguments);
} catch (IllegalValueException e) {
assertEquals(e.getMessage(), Commons.ERROR_NULL);
assertEquals(e.getMessage(), String.format(Commons.ERROR_NULL, deleteCommand.getArgumentPrefixes()[0]));
} catch (Exception e) {
fail("Expected: IllegalValueException, Actual: " + e.getMessage());
}
Expand Down

0 comments on commit f7e6adb

Please sign in to comment.