Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove description from Module & Add tests #107

Merged
merged 3 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/checkstyle/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
<suppress checks="MissingJavadocMethodCheck" files=".*Test\.java"/>
<suppress checks="CommentsIndentation" files=".*\.java"/>
<suppress checks="DeclarationOrder" files=".*\.java"/>

<suppress checks="OperatorWrap" files=".*\.java"/>

Choose a reason for hiding this comment

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

What does this do?

Copy link
Author

Choose a reason for hiding this comment

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

There is a warning that the operator (like + for string concatenation) must be on a new line, but I haven't found a way to make IntelliJ auto-format for that, and it's really minor so I will just disable it for now

Choose a reason for hiding this comment

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

Hmm I think we should enable it before merging right?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm because I haven't been able to get the IntelliJ to conform to this rule, and this is very minor (it's just the position of the operator) so I think it's best to disable it for now (if not forever)

Choose a reason for hiding this comment

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

Ok, but I think before we submit v1.3 on Friday we should remove this

</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static java.util.Objects.requireNonNull;
import static trackitnus.logic.parser.CliSyntax.PREFIX_CODE;
import static trackitnus.logic.parser.CliSyntax.PREFIX_DESC;
import static trackitnus.logic.parser.CliSyntax.PREFIX_NAME;

import trackitnus.logic.commands.Command;
Expand All @@ -17,8 +16,7 @@ public class AddModuleCommand extends Command {
public static final String MESSAGE_USAGE = Module.TYPE + " " + COMMAND_WORD + ": Adds a module to the app "
+ "Parameters: "
+ PREFIX_CODE + "CODE "
+ PREFIX_NAME + "NAME "
+ PREFIX_DESC + "DESCRIPTION ";
+ PREFIX_NAME + "NAME ";

public static final String MESSAGE_SUCCESS = "New module added: %1$s";
public static final String MESSAGE_DUPLICATE_MODULE = "This module already exists";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static java.util.Objects.requireNonNull;
import static trackitnus.logic.parser.CliSyntax.PREFIX_CODE;
import static trackitnus.logic.parser.CliSyntax.PREFIX_DESC;
import static trackitnus.logic.parser.CliSyntax.PREFIX_NAME;
import static trackitnus.model.Model.PREDICATE_SHOW_ALL_MODULES;

Expand All @@ -28,9 +27,8 @@ public class EditModuleCommand extends Command {
+ "Parameters: "
+ PREFIX_CODE + "CODE (must be an existing code) "
+ "[" + PREFIX_NAME + "NAME] "
+ "[" + PREFIX_DESC + "DESC] "
+ String.format("Example: %s %s %sCS1231S %sDiscrete Structures %sIntroductory mathematical tools",
Module.TYPE, COMMAND_WORD, PREFIX_CODE, PREFIX_NAME, PREFIX_DESC);
+ String.format("Example: %s %s %sCS1231S %sDiscrete Structures",
Module.TYPE, COMMAND_WORD, PREFIX_CODE, PREFIX_NAME);

public static final String MESSAGE_EDIT_MODULE_SUCCESS = "Edited Module: %1$s";
public static final String MESSAGE_NOT_EDITED = "At least one field to edit must be provided.";
Expand Down Expand Up @@ -61,9 +59,8 @@ private static Module createEditedModule(Module moduleToEdit,

Code originalCode = moduleToEdit.getCode();
Name updatedName = editModuleDescriptor.getName().orElse(moduleToEdit.getName());
String updatedDesc = editModuleDescriptor.getDesc().orElse(moduleToEdit.getDesc());

return new Module(originalCode, updatedName, updatedDesc);
return new Module(originalCode, updatedName);
}

@Override
Expand Down Expand Up @@ -103,15 +100,21 @@ public boolean equals(Object other) {
&& editModuleDescriptor.equals(e.editModuleDescriptor);
}

@Override
public String toString() {
return "EditModuleCommand{" +
"code=" + code +
", editModuleDescriptor=" + editModuleDescriptor +
'}';
}

simonteozw marked this conversation as resolved.
Show resolved Hide resolved
/**
* Stores the details to edit the module with. Each non-empty field value will replace the
* corresponding field value of the module.
*/
public static class EditModuleDescriptor {

private Code code;
private Name name;
private String desc;

public EditModuleDescriptor() {
}
Expand All @@ -120,16 +123,14 @@ public EditModuleDescriptor() {
* Copy constructor.
*/
public EditModuleDescriptor(EditModuleDescriptor toCopy) {
setCode(toCopy.code);
setName(toCopy.name);
setDesc(toCopy.desc);
}

/**
* Returns true if at least one field is edited.
*/
public boolean isAnyFieldEdited() {
return CollectionUtil.isAnyNonNull(name, desc);
return CollectionUtil.isAnyNonNull(name);
}

@Override
Expand All @@ -147,17 +148,7 @@ public boolean equals(Object other) {
// state check
EditModuleDescriptor e = (EditModuleDescriptor) other;

return getCode().equals(e.getCode())
&& getName().equals(e.getName())
&& getDesc().equals(e.getDesc());
}

public Optional<Code> getCode() {
return Optional.ofNullable(code);
}

public void setCode(Code code) {
this.code = code;
return getName().equals(e.getName());
}

public Optional<Name> getName() {
Expand All @@ -168,12 +159,11 @@ public void setName(Name name) {
this.name = name;
}

public Optional<String> getDesc() {
return Optional.ofNullable(desc);
}

public void setDesc(String desc) {
this.desc = desc;
@Override
public String toString() {
return "EditModuleDescriptor{" +
"name=" + name +
'}';
Comment on lines +162 to +166

Choose a reason for hiding this comment

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

Why do we need this? And Maybe change to Name: is this for testing purposes?

Copy link
Author

Choose a reason for hiding this comment

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

for testing only

}
}
}
1 change: 0 additions & 1 deletion src/main/java/trackitnus/logic/parser/CliSyntax.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public class CliSyntax {
public static final Prefix PREFIX_CODE = new Prefix("m/");
public static final Prefix PREFIX_TYPE = new Prefix("n/");
public static final Prefix PREFIX_WEIGHTAGE = new Prefix("w/");
public static final Prefix PREFIX_DESC = new Prefix("d/");
public static final Prefix PREFIX_DATE = new Prefix("d/");
public static final Prefix PREFIX_REMARK = new Prefix("r/");
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package trackitnus.logic.parser.module;

import static trackitnus.logic.parser.CliSyntax.PREFIX_CODE;
import static trackitnus.logic.parser.CliSyntax.PREFIX_DESC;
import static trackitnus.logic.parser.CliSyntax.PREFIX_NAME;

import java.util.stream.Stream;
Expand Down Expand Up @@ -39,18 +38,17 @@ private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Pre
*/
public AddModuleCommand parse(String args) throws ParseException {
ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_CODE, PREFIX_NAME, PREFIX_DESC);
ArgumentTokenizer.tokenize(args, PREFIX_CODE, PREFIX_NAME);

if (!arePrefixesPresent(argMultimap, PREFIX_CODE, PREFIX_NAME, PREFIX_DESC)) {
if (!arePrefixesPresent(argMultimap, PREFIX_CODE, PREFIX_NAME)) {
throw new ParseException(String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT,
AddModuleCommand.MESSAGE_USAGE));
}

Code code = ParserUtil.parseCode(argMultimap.getValue(PREFIX_CODE).get());
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
String desc = ParserUtil.parseString(argMultimap.getValue(PREFIX_DESC).get());

Module module = new Module(code, name, desc);
Module module = new Module(code, name);

return new AddModuleCommand(module);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static java.util.Objects.requireNonNull;
import static trackitnus.logic.parser.CliSyntax.PREFIX_CODE;
import static trackitnus.logic.parser.CliSyntax.PREFIX_DESC;
import static trackitnus.logic.parser.CliSyntax.PREFIX_NAME;

import trackitnus.commons.core.Messages;
Expand All @@ -28,7 +27,7 @@ public class EditModuleCommandParser implements Parser<EditModuleCommand> {
public EditModuleCommand parse(String args) throws ParseException {
requireNonNull(args);
ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_CODE, PREFIX_NAME, PREFIX_DESC);
ArgumentTokenizer.tokenize(args, PREFIX_CODE, PREFIX_NAME);

Code code;

Expand All @@ -46,9 +45,6 @@ public EditModuleCommand parse(String args) throws ParseException {
if (argMultimap.getValue(PREFIX_NAME).isPresent()) {
editModuleDescriptor.setName(ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()));
}
if (argMultimap.getValue(PREFIX_DESC).isPresent()) {
editModuleDescriptor.setDesc(ParserUtil.parseString(argMultimap.getValue(PREFIX_DESC).get()));
}

if (!editModuleDescriptor.isAnyFieldEdited()) {
throw new ParseException(EditModuleCommand.MESSAGE_NOT_EDITED);
Expand Down
20 changes: 5 additions & 15 deletions src/main/java/trackitnus/model/module/Module.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,17 @@ public class Module {

private final Code code;
private final Name name;
private final String desc;

/**
* Every field must be present and not null.
*
* @param code
* @param name
* @param desc
*/
public Module(Code code, Name name, String desc) {
CollectionUtil.requireAllNonNull(code, name, desc);
public Module(Code code, Name name) {
CollectionUtil.requireAllNonNull(code, name);
this.code = code;
this.name = name;
this.desc = desc;
}

public Code getCode() {
Expand All @@ -39,10 +36,6 @@ public Name getName() {
return name;
}

public String getDesc() {
return desc;
}

@Override
public boolean equals(Object other) {
if (other == this) {
Expand All @@ -55,24 +48,21 @@ public boolean equals(Object other) {

Module otherLesson = (Module) other;
return otherLesson.code.equals(code)
&& otherLesson.name.equals(name)
&& otherLesson.desc.equals(desc);
&& otherLesson.name.equals(name);
}

@Override
public int hashCode() {
// use this method for custom fields hashing instead of implementing your own
return Objects.hash(code, name, desc);
return Objects.hash(code, name);
}

@Override
public String toString() {
return " Code: "
+ getCode()
+ " Name: "
+ getName()
+ " Desc: "
+ getDesc();
+ getName();
}

/**
Expand Down
13 changes: 2 additions & 11 deletions src/main/java/trackitnus/storage/JsonAdaptedModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,14 @@ public class JsonAdaptedModule {

private final String code;
private final String name;
private final String desc;

/**
* Constructs a {@code JsonAdaptedModule} with the given module details.
*/
@JsonCreator
public JsonAdaptedModule(@JsonProperty("code") String code, @JsonProperty("name") String name,
@JsonProperty("desc") String desc) {
public JsonAdaptedModule(@JsonProperty("code") String code, @JsonProperty("name") String name) {
this.code = code;
this.name = name;
this.desc = desc;
}

/**
Expand All @@ -36,7 +33,6 @@ public JsonAdaptedModule(@JsonProperty("code") String code, @JsonProperty("name"
public JsonAdaptedModule(Module source) {
code = source.getCode().code;
name = source.getName().fullName;
desc = source.getDesc();
}

/**
Expand All @@ -62,12 +58,7 @@ public Module toModelType() throws IllegalValueException {
}
final Name modelName = new Name(name);

if (desc == null) {
throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, "Description"));
}
final String modelDesc = desc;

return new Module(modelCode, modelName, modelDesc);
return new Module(modelCode, modelName);
}

}
3 changes: 0 additions & 3 deletions src/main/java/trackitnus/ui/module/ModuleCard.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ public class ModuleCard extends UiPart<Region> {
private Label name;
@FXML
private Label id;
@FXML
private Label desc;


/**
Expand All @@ -45,7 +43,6 @@ public ModuleCard(Module module, int displayedIndex) {
id.setText(displayedIndex + ". ");
code.setText(module.getCode().code);
name.setText(module.getName().fullName);
desc.setText(module.getDesc());

Choose a reason for hiding this comment

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

maybe can remove the @FXML private Label desc on top too

Copy link
Author

Choose a reason for hiding this comment

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

Sure

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ public class AddModuleCommandParserTest {
@Test
public void parse_allFieldsPresent_success() {
AddModuleCommand expectedCommand = new AddModuleCommand(new Module(new Code("CS1231S"), new Name("Discrete "
+ "Structures"), "Sample"));
+ "Structures")));

CommandParserTestUtil.assertParseSuccess(parser, " n/Discrete Structures m/CS1231S d/Sample", expectedCommand);
// assertParseSuccess(parser, " 1 n/Discrete Structures m/CS1231S d/Sample", expectedCommand);
CommandParserTestUtil.assertParseSuccess(parser, " n/Discrete Structures m/CS1231S", expectedCommand);
}

// @Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
package trackitnus.logic.parser.module;

import static trackitnus.logic.parser.CommandParserTestUtil.assertParseFailure;
import static trackitnus.logic.parser.CommandParserTestUtil.assertParseSuccess;

import org.junit.jupiter.api.Test;

import trackitnus.commons.core.Messages;
import trackitnus.logic.commands.module.EditModuleCommand;
import trackitnus.model.commons.Code;
import trackitnus.model.commons.Name;

public class EditModuleCommandParserTest {
private final EditModuleCommandParser parser = new EditModuleCommandParser();

public static EditModuleCommand editModuleCommandBuilder(Code code, Name name) {
EditModuleCommand.EditModuleDescriptor descriptor = new EditModuleCommand.EditModuleDescriptor();
descriptor.setName(name);
return new EditModuleCommand(code, descriptor);
}

// note for Module: all user input needs leading space
@Test
public void parse_allFieldsPresent_success() {
// EditModuleCommand expected = new EditModuleCommand(new Code("CS1231S"),new EditModuleCommand
// .EditModuleDescriptor())
// assertParseSuccess(parser, "m/CS1231S n/Sample d/Sample", new Edi);
assertParseSuccess(parser, " m/CS2030S n/Sample", editModuleCommandBuilder(new Code("CS2030S"),
new Name("Sample")));
}

@Test
Expand All @@ -25,7 +34,6 @@ public void parse_noFieldsProvided_failure() {

@Test
public void parse_justModuleProvided_failure() {
assertParseFailure(parser, "m/CS1231S", String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT,
EditModuleCommand.MESSAGE_USAGE));
assertParseFailure(parser, " m/CS1231S", EditModuleCommand.MESSAGE_NOT_EDITED);
}
}