Skip to content

Commit

Permalink
Merge pull request AY2324S1-CS2103T-T09-2#223 from ncmathan/master
Browse files Browse the repository at this point in the history
Improve code quality, add javadocs, fixed build errors in test files
  • Loading branch information
mingyu-wan authored Nov 8, 2023
2 parents 6ef2183 + 6a1104a commit 8edc659
Show file tree
Hide file tree
Showing 39 changed files with 734 additions and 404 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ dependencies {
}

shadowJar {
archiveFileName = 'addressbook.jar'
archiveFileName = 'CodeContact.jar'
}

defaultTasks 'clean', 'test'
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/model/client/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public boolean isSameClient(Client otherClient) {
}

return otherClient != null
&& otherClient.getName().equals(getName());
&& otherClient.getName().fullName.toLowerCase().equals(getName().fullName.toLowerCase());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/model/developer/Developer.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public boolean isSameDeveloper(Developer otherDeveloper) {
}

return otherDeveloper != null
&& otherDeveloper.getName().equals(getName());
&& otherDeveloper.getName().fullName.toLowerCase().equals(getName().fullName.toLowerCase());
}

public GithubId getGithubId() {
Expand Down
33 changes: 17 additions & 16 deletions src/main/resources/view/DeveloperListCard.fxml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>

<?import org.controlsfx.control.Rating?>
<?import javafx.geometry.Insets?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.layout.ColumnConstraints?>
Expand All @@ -10,48 +9,50 @@
<?import javafx.scene.layout.Region?>
<?import javafx.scene.layout.RowConstraints?>
<?import javafx.scene.layout.VBox?>
<HBox xmlns:fx="http://javafx.com/fxml/1" id="cardPane" fx:id="cardPane" xmlns="http://javafx.com/javafx/20.0.1">
<?import org.controlsfx.control.Rating?>

<HBox id="cardPane" fx:id="cardPane" xmlns="http://javafx.com/javafx/20.0.1" xmlns:fx="http://javafx.com/fxml/1">
<GridPane HBox.hgrow="ALWAYS">
<columnConstraints>
<ColumnConstraints hgrow="SOMETIMES" minWidth="10" prefWidth="150"/>
<ColumnConstraints hgrow="SOMETIMES" minWidth="10" prefWidth="150" />
</columnConstraints>
<VBox alignment="CENTER_LEFT" minHeight="105" GridPane.columnIndex="0">
<padding>
<Insets bottom="5" left="15" right="5" top="5"/>
<Insets bottom="5" left="15" right="5" top="5" />
</padding>
<HBox alignment="CENTER_LEFT" spacing="5">
<Label fx:id="id" styleClass="cell_big_label">
<minWidth>
<!-- Ensures that the label text is never truncated -->
<Region fx:constant="USE_PREF_SIZE"/>
<Region fx:constant="USE_PREF_SIZE" />
</minWidth>
</Label>
<Label fx:id="name" styleClass="cell_big_label" text="\$first"/>
<Label fx:id="name" styleClass="cell_big_label" text="\$first" />
</HBox>
<FlowPane fx:id="tags"/>
<FlowPane fx:id="tags" />
<HBox alignment="CENTER_LEFT" layoutX="25.0" layoutY="15.0" spacing="5">
<children>
<VBox prefWidth="350.0">
<children>
<Label fx:id="phone" styleClass="cell_small_label" text="\$phone"/>
<Label fx:id="email" styleClass="cell_small_label" text="\$email"/>
<Label fx:id="address" styleClass="cell_small_label" text="\$address" wrapText="true"/>
<Label fx:id="dateJoined" styleClass="cell_small_label" text="\$dateJoined"/>
<Label fx:id="phone" styleClass="cell_small_label" text="\$phone" />
<Label fx:id="email" styleClass="cell_small_label" text="\$email" />
<Label fx:id="address" styleClass="cell_small_label" text="\$address" wrapText="true" />
<Label fx:id="dateJoined" styleClass="cell_small_label" text="\$dateJoined" />
</children>
</VBox>
<VBox prefWidth="300.0">
<children>
<Label fx:id="role" styleClass="cell_small_label" text="\$role"/>
<Label fx:id="salary" styleClass="cell_small_label" text="\$salary"/>
<Label fx:id="githubId" styleClass="cell_small_label" text="\$githubId"/>
<Rating fx:id="rating" prefHeight="20.0" prefWidth="100.0"/>
<Label fx:id="role" styleClass="cell_small_label" text="\$role" />
<Label fx:id="salary" styleClass="cell_small_label" text="\$salary" />
<Label fx:id="githubId" styleClass="cell_small_label" text="\$githubId" wrapText="true" />
<Rating fx:id="rating" prefHeight="20.0" prefWidth="100.0" />
</children>
</VBox>
</children>
</HBox>
</VBox>
<rowConstraints>
<RowConstraints/>
<RowConstraints />
</rowConstraints>
</GridPane>
</HBox>
3 changes: 1 addition & 2 deletions src/test/java/seedu/address/commons/core/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;

Expand All @@ -20,7 +19,7 @@ public void toStringMethod() {
public void equalsMethod() {
Config defaultConfig = new Config();
assertNotNull(defaultConfig);
assertTrue(defaultConfig.equals(defaultConfig));
assertEquals(defaultConfig, defaultConfig);
}


Expand Down
10 changes: 5 additions & 5 deletions src/test/java/seedu/address/commons/core/VersionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void versionConstructor_correctParameter_valueAsExpected() {
assertEquals(19, version.getMajor());
assertEquals(10, version.getMinor());
assertEquals(20, version.getPatch());
assertEquals(true, version.isEarlyAccess());
assertTrue(version.isEarlyAccess());
}

@Test
Expand All @@ -53,11 +53,11 @@ public void versionComparable_validVersion_compareToIsCorrect() {
// Tests equality
one = new Version(0, 0, 0, true);
another = new Version(0, 0, 0, true);
assertTrue(one.compareTo(another) == 0);
assertEquals(0, one.compareTo(another));

one = new Version(11, 12, 13, false);
another = new Version(11, 12, 13, false);
assertTrue(one.compareTo(another) == 0);
assertEquals(0, one.compareTo(another));

// Tests different patch
one = new Version(0, 0, 5, false);
Expand Down Expand Up @@ -121,11 +121,11 @@ public void versionComparable_validVersion_equalIsCorrect() {

one = new Version(0, 0, 0, false);
another = new Version(0, 0, 0, false);
assertTrue(one.equals(another));
assertEquals(one, another);

one = new Version(100, 191, 275, true);
another = new Version(100, 191, 275, true);
assertTrue(one.equals(another));
assertEquals(one, another);
}

private void verifyVersionParsedCorrectly(String versionString,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ public void requireAllNonNullVarargs() {
assertNullPointerExceptionThrown((Object[]) null);

// confirms nulls inside lists in the argument list are not considered
List<Object> containingNull = Arrays.asList((Object) null);
List<Object> containingNull = Collections.singletonList((Object) null);
assertNullPointerExceptionNotThrown(containingNull, new Object());
}

@Test
public void requireAllNonNullCollection() {
// lists containing nulls in the front
assertNullPointerExceptionThrown(Arrays.asList((Object) null));
assertNullPointerExceptionThrown(Collections.singletonList((Object) null));
assertNullPointerExceptionThrown(Arrays.asList(null, new Object(), ""));

// lists containing nulls in the middle
Expand All @@ -66,10 +66,10 @@ public void requireAllNonNullCollection() {

// list with all non-null elements
assertNullPointerExceptionNotThrown(Arrays.asList(new Object(), "ham", Integer.valueOf(1)));
assertNullPointerExceptionNotThrown(Arrays.asList(new Object()));
assertNullPointerExceptionNotThrown(List.of(new Object()));

// confirms nulls inside nested lists are not considered
List<Object> containingNull = Arrays.asList((Object) null);
List<Object> containingNull = Collections.singletonList((Object) null);
assertNullPointerExceptionNotThrown(Arrays.asList(containingNull, new Object()));
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/seedu/address/logic/LogicManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class LogicManagerTest {
@TempDir
public Path temporaryFolder;

private Model model = new ModelManager();
private final Model model = new ModelManager();
private Logic logic;

@BeforeEach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

import static java.util.Objects.requireNonNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static seedu.address.testutil.Assert.assertThrows;
import static seedu.address.testutil.TypicalDevelopers.ALICE;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.function.Predicate;

import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -46,7 +45,7 @@ public void execute_personAcceptedByModel_addSuccessful() throws Exception {

assertEquals(String.format(AddDeveloperCommand.MESSAGE_SUCCESS, Messages.format(validDeveloper)),
commandResult.getFeedbackToUser());
assertEquals(Arrays.asList(validDeveloper), modelStub.developersAdded);
assertEquals(List.of(validDeveloper), modelStub.developersAdded);
}

@Test
Expand All @@ -55,7 +54,8 @@ public void execute_duplicatePerson_throwsCommandException() {
AddDeveloperCommand addDeveloperCommand = new AddDeveloperCommand(validDeveloper);
ModelStub modelStub = new ModelStubWithDeveloper(validDeveloper);

assertThrows(CommandException.class, AddDeveloperCommand.MESSAGE_DUPLICATE_DEVELOPER, () -> addDeveloperCommand.execute(modelStub));
assertThrows(CommandException.class, AddDeveloperCommand.MESSAGE_DUPLICATE_DEVELOPER, ()
-> addDeveloperCommand.execute(modelStub));
}

@Test
Expand All @@ -66,20 +66,20 @@ public void equals() {
AddDeveloperCommand addBobCommand = new AddDeveloperCommand(bob);

// same object -> returns true
assertTrue(addAliceCommand.equals(addAliceCommand));
assertEquals(addAliceCommand, addAliceCommand);

// same values -> returns true
AddDeveloperCommand addAliceCommandCopy = new AddDeveloperCommand(alice);
assertTrue(addAliceCommand.equals(addAliceCommandCopy));
assertEquals(addAliceCommand, addAliceCommandCopy);

// different types -> returns false
assertFalse(addAliceCommand.equals(1));
assertNotEquals(1, addAliceCommand);

// null -> returns false
assertFalse(addAliceCommand.equals(null));
assertNotEquals(null, addAliceCommand);

// different developer -> returns false
assertFalse(addAliceCommand.equals(addBobCommand));
assertNotEquals(addAliceCommand, addBobCommand);
}

@Test
Expand Down
29 changes: 17 additions & 12 deletions src/test/java/seedu/address/logic/commands/CommandResultTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
public class CommandResultTest {
@Test
public void equals() {
CommandResult commandResult = new CommandResult("feedback");
CommandResult commandResult = new CommandResult("feedback", TabIndex.Developer);

// same values -> returns true
assertTrue(commandResult.equals(new CommandResult("feedback")));
assertTrue(commandResult.equals(new CommandResult("feedback", false, false)));
assertTrue(commandResult.equals(new CommandResult("feedback", TabIndex.Developer)));
assertTrue(commandResult.equals(new CommandResult("feedback", false, false, TabIndex.Developer)));

// same object -> returns true
assertTrue(commandResult.equals(commandResult));
Expand All @@ -26,35 +26,40 @@ public void equals() {
assertFalse(commandResult.equals(0.5f));

// different feedbackToUser value -> returns false
assertFalse(commandResult.equals(new CommandResult("different")));
assertFalse(commandResult.equals(new CommandResult("different", TabIndex.Developer)));

// different showHelp value -> returns false
assertFalse(commandResult.equals(new CommandResult("feedback", true, false)));
assertFalse(commandResult.equals(new CommandResult("feedback", true, false, TabIndex.Developer)));

// different exit value -> returns false
assertFalse(commandResult.equals(new CommandResult("feedback", false, true)));
assertFalse(commandResult.equals(new CommandResult("feedback", false, true, TabIndex.Developer)));
}

@Test
public void hashcode() {
CommandResult commandResult = new CommandResult("feedback");
CommandResult commandResult = new CommandResult("feedback", TabIndex.Developer);

// same values -> returns same hashcode
assertEquals(commandResult.hashCode(), new CommandResult("feedback").hashCode());
assertEquals(commandResult.hashCode(), new CommandResult("feedback", TabIndex.Developer)
.hashCode());

// different feedbackToUser value -> returns different hashcode
assertNotEquals(commandResult.hashCode(), new CommandResult("different").hashCode());
assertNotEquals(commandResult.hashCode(),
new CommandResult("different", TabIndex.Developer)
.hashCode());

// different showHelp value -> returns different hashcode
assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", true, false).hashCode());
assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", true, false,
TabIndex.Developer).hashCode());

// different exit value -> returns different hashcode
assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", false, true).hashCode());
assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", false, true,
TabIndex.Developer).hashCode());
}

@Test
public void toStringMethod() {
CommandResult commandResult = new CommandResult("feedback");
CommandResult commandResult = new CommandResult("feedback", TabIndex.Developer);
String expected = CommandResult.class.getCanonicalName() + "{feedbackToUser="
+ commandResult.getFeedbackToUser() + ", showHelp=" + commandResult.isShowHelp()
+ ", exit=" + commandResult.isExit() + "}";
Expand Down
18 changes: 9 additions & 9 deletions src/test/java/seedu/address/logic/commands/CommandTestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_DATEJOINED;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
import static seedu.address.logic.parser.CliSyntax.PREFIX_GITHUBID;
import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PROJECT;
import static seedu.address.logic.parser.CliSyntax.PREFIX_RATING;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ROLE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_SALARY;
import static seedu.address.logic.parser.CliSyntax.PREFIX_DATEJOINED;
import static seedu.address.logic.parser.CliSyntax.PREFIX_GITHUBID;
import static seedu.address.logic.parser.CliSyntax.PREFIX_RATING;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PROJECT;
import static seedu.address.testutil.Assert.assertThrows;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import seedu.address.commons.core.index.Index;
Expand Down Expand Up @@ -79,12 +79,11 @@ public class CommandTestUtil {
public static final String RATING_DEC_BOB = " " + PREFIX_RATING + VALID_RATING_BOB;



public static final String INVALID_NAME_DESC = " " + PREFIX_NAME + "James&"; // '&' not allowed in names
public static final String INVALID_PHONE_DESC = " " + PREFIX_PHONE + "911a"; // 'a' not allowed in phones
public static final String INVALID_EMAIL_DESC = " " + PREFIX_EMAIL + "bob!yahoo"; // missing '@' symbol
public static final String INVALID_ADDRESS_DESC = " " + PREFIX_ADDRESS; // empty string not allowed for addresses
public static final String INVALID_ROLE_DESC = " " + PREFIX_ROLE; //
public static final String INVALID_ROLE_DESC = " " + PREFIX_ROLE; //
public static final String INVALID_SALARY_DESC = "abc" + PREFIX_SALARY; //
public static final String INVALID_DATEJOINED_DESC = "2003-23" + PREFIX_DATEJOINED; //
public static final String INVALID_GITHUBID_DESC = " " + PREFIX_GITHUBID; //
Expand Down Expand Up @@ -158,12 +157,13 @@ public static void assertCommandFailure(Command command, Model actualModel, Stri
* Updates {@code model}'s filtered list to show only the developer at the given {@code targetIndex} in the
* {@code model}'s address book.
*/
public static void showPersonAtIndex(Model model, Index targetIndex) {
public static void showDeveloperAtIndex(Model model, Index targetIndex) {
assertTrue(targetIndex.getZeroBased() < model.getFilteredDeveloperList().size());

Developer developer = model.getFilteredDeveloperList().get(targetIndex.getZeroBased());
final String[] splitName = developer.getName().fullName.split("\\s+");
model.updateFilteredDeveloperList(new NameDeveloperContainsKeywordsPredicate(Arrays.asList(splitName[0])));
model.updateFilteredDeveloperList(new NameDeveloperContainsKeywordsPredicate(Collections
.singletonList(splitName[0])));

assertEquals(1, model.getFilteredDeveloperList().size());
}
Expand Down
Loading

0 comments on commit 8edc659

Please sign in to comment.