Update add command to include block and room #63

schoolex · 2020-10-06T15:23:14Z

No description provided.

codecov-io · 2020-10-08T08:38:10Z

Codecov Report

Merging #63 into master will increase coverage by 0.40%.
The diff coverage is 74.66%.

@@             Coverage Diff              @@
##             master      #63      +/-   ##
============================================
+ Coverage     69.45%   69.85%   +0.40%     
- Complexity      431      441      +10     
============================================
  Files            77       79       +2     
  Lines          1401     1463      +62     
  Branches        146      151       +5     
============================================
+ Hits            973     1022      +49     
- Misses          387      397      +10     
- Partials         41       44       +3

Impacted Files	Coverage Δ	Complexity Δ
.../java/seedu/address/logic/commands/AddCommand.java	`100.00% <ø> (ø)`	`8.00 <0.00> (ø)`
.../java/seedu/address/model/util/SampleDataUtil.java	`20.00% <0.00%> (ø)`	`1.00 <0.00> (ø)`
src/main/java/seedu/address/ui/PersonCard.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../seedu/address/logic/parser/EditCommandParser.java	`87.87% <33.33%> (-5.67%)`	`13.00 <0.00> (ø)`
...in/java/seedu/address/logic/parser/ParserUtil.java	`91.07% <60.00%> (-6.76%)`	`20.00 <2.00> (+2.00)`	⬇️
...rc/main/java/seedu/address/model/person/Block.java	`70.00% <70.00%> (ø)`	`3.00 <3.00> (?)`
src/main/java/seedu/address/model/person/Room.java	`70.00% <70.00%> (ø)`	`3.00 <3.00> (?)`
...java/seedu/address/logic/commands/EditCommand.java	`95.55% <100.00%> (+0.55%)`	`12.00 <0.00> (ø)`
...a/seedu/address/logic/parser/AddCommandParser.java	`100.00% <100.00%> (ø)`	`5.00 <0.00> (ø)`
...ain/java/seedu/address/logic/parser/CliSyntax.java	`88.88% <100.00%> (+1.38%)`	`1.00 <0.00> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b1a35a...236a0dc. Read the comment docs.

athuyaoo · 2020-10-11T13:33:29Z

src/main/java/seedu/address/model/person/Person.java

-    public Person(Name name, Phone phone, Email email, Address address, Set<Tag> tags) {
-        requireAllNonNull(name, phone, email, address, tags);
+    public Person(Name name, Phone phone, Email email, Address address, Set<Tag> tags, Block block, Room room) {
+        requireAllNonNull(name, phone, email, address, tags, room);


How come block doesn't need to be non null?

…into branch-adding-block

src/main/java/seedu/address/model/person/Block.java

athuyaoo

Looks good! Only minor changes needed.

athuyaoo · 2020-10-11T19:04:01Z

src/test/java/seedu/address/logic/parser/AddCommandParserTest.java

                        + MATRICULATION_NUMBER_DESC_BOB,
                String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));

        // invalid matriculation number
        assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
-                + GENDER_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND + INVALID_MATRICULATION_NUMBER_DESC,
-                MatriculationNumber.MESSAGE_CONSTRAINTS);
+                        + BLOCKROOM_DESC_BOB + GENDER_DESC_BOB + TAG_DESC_HUSBAND


Perhaps reducing the space by 4 would help conform to coding standards better.

athuyaoo · 2020-10-11T19:05:44Z

src/test/java/seedu/address/testutil/TypicalPersons.java


    // Manually added - Person's details found in {@code CommandTestUtil}
    public static final Person AMY = new PersonBuilder().withName(VALID_NAME_AMY).withPhone(VALID_PHONE_AMY)
            .withEmail(VALID_EMAIL_AMY).withAddress(VALID_ADDRESS_AMY).withTags(VALID_TAG_FRIEND)
-            .withGender(VALID_GENDER_AMY)
+            .withGender(VALID_GENDER_AMY).withBlock("A").withRoom("420")


Perhaps having VALID_BLOCK and VALID_ROOM will be better for conforming with standards

athuyaoo · 2020-10-11T19:06:34Z

src/test/java/seedu/address/logic/commands/CommandTestUtil.java

@@ -40,6 +41,7 @@
    public static final String VALID_GENDER_BOB = "M";
    public static final String VALID_TAG_HUSBAND = "husband";
    public static final String VALID_TAG_FRIEND = "friend";
+    public static final String VALID_BLOCKROOM = "A420";


perhaps you can split this up to VALID_BLOCK and VALID_ROOM, so that you can reuse them? (especially in TypicalPersons.java)

You can make VALID_BLOCKROOM with the 2 as well.

Will update this

athuyaoo

Sorry I overlooked a few changes! I think this should be the last.

athuyaoo · 2020-10-12T04:05:11Z

src/test/java/seedu/address/testutil/PersonBuilder.java

+     * Sets the {@code MatriculationNumber} of the {@code Person} that we are building.
     * Sets the {@code gender} of the {@code Person} that we are building.


I think got small typo here. Matriculation number and gender?

athuyaoo · 2020-10-12T04:08:08Z

src/test/java/seedu/address/logic/parser/AddCommandParserTest.java

+                        + BLOCKROOM_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND
+                        + MATRICULATION_NUMBER_DESC_BOB + GENDER_DESC_BOB,


assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + BLOCKROOM_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND + MATRICULATION_NUMBER_DESC_BOB + GENDER_DESC_BOB,

Should the indentation be 8 spaces here? https://se-education.org/guides/conventions/java/intermediate.html#layout

athuyaoo · 2020-10-12T04:09:01Z

src/test/java/seedu/address/logic/parser/AddCommandParserTest.java

+                        + BLOCKROOM_DESC_BOB + GENDER_DESC_BOB + TAG_DESC_FRIEND
+                        + MATRICULATION_NUMBER_DESC_AMY + MATRICULATION_NUMBER_DESC_BOB,


assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + BLOCKROOM_DESC_BOB + GENDER_DESC_BOB + TAG_DESC_FRIEND + MATRICULATION_NUMBER_DESC_AMY + MATRICULATION_NUMBER_DESC_BOB,

Should the indentation be 8 spaces here? https://se-education.org/guides/conventions/java/intermediate.html#layout

athuyaoo · 2020-10-12T04:09:58Z

src/main/java/seedu/address/model/person/Room.java

+
+

I think it's better to remove this space?

athuyaoo · 2020-10-12T04:10:19Z

src/main/java/seedu/address/model/person/Person.java

@@ -57,6 +62,15 @@ public Address getAddress() {
        return address;
    }

+


Same here, additional line can be removed.

src/main/java/seedu/address/model/person/Block.java

schoolex added 11 commits September 23, 2020 16:49

Add code for tutorial 3

a796e27

Remove VSC IDE settings

8196eef

Update files to ensure compliance with checkstyle

d9d4d01

Add Block and Room class

377dd19

Add block number feature

a934585

Update add command to include block and room

96a3438

Remove bin folder

062f99d

Remove unneeded files

82346ef

Add newline in gitignore file

a8bde55

Removed unused imports and variables

2f62f57

Fix formatting bugs

eb4ee81

schoolex added priority.High Must do type.Task Not a story, bug, or an epic. labels Oct 6, 2020

schoolex added this to the v1.2 milestone Oct 6, 2020

schoolex marked this pull request as draft October 6, 2020 15:49

Update previous test to include blockroom field

70f667a

schoolex marked this pull request as ready for review October 8, 2020 08:38

athuyaoo self-requested a review October 9, 2020 08:47

athuyaoo reviewed Oct 11, 2020

View reviewed changes

schoolex added 3 commits October 11, 2020 21:40

Update missing non-null check in Person

2f9d96c

Merge branch 'master' into branch-adding-block

c83f089

Merge branch 'master' of https://github.com/AY2021S1-CS2103T-T11-2/tp …

a4480cd

…into branch-adding-block

schoolex linked an issue Oct 11, 2020 that may be closed by this pull request

Add block and room number #37

Closed

athuyaoo reviewed Oct 11, 2020

View reviewed changes

src/main/java/seedu/address/model/person/Block.java Show resolved Hide resolved

athuyaoo requested changes Oct 11, 2020

View reviewed changes

Fix formatting and some bugs

3f2dc00

athuyaoo requested changes Oct 12, 2020

View reviewed changes

Fix typos

f0b4da9

athuyaoo approved these changes Oct 12, 2020

View reviewed changes

athuyaoo requested review from athuyaoo and removed request for athuyaoo October 12, 2020 04:43

Fix more formatting bugs

236a0dc

athuyaoo merged commit 4083f49 into AY2021S1-CS2103T-T11-2:master Oct 12, 2020

schoolex linked an issue Oct 12, 2020 that may be closed by this pull request

As a Hall admin managing residents, i can create residents records #10

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update add command to include block and room #63

Update add command to include block and room #63

schoolex commented Oct 6, 2020

codecov-io commented Oct 8, 2020 •

edited

athuyaoo Oct 11, 2020

athuyaoo left a comment

athuyaoo Oct 11, 2020

athuyaoo Oct 11, 2020

athuyaoo Oct 11, 2020

schoolex Oct 12, 2020 •

edited

athuyaoo left a comment

athuyaoo Oct 12, 2020

athuyaoo Oct 12, 2020

athuyaoo Oct 12, 2020

athuyaoo Oct 12, 2020

athuyaoo Oct 12, 2020

		* Sets the {@code MatriculationNumber} of the {@code Person} that we are building.
		* Sets the {@code gender} of the {@code Person} that we are building.

		+ BLOCKROOM_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND
		+ MATRICULATION_NUMBER_DESC_BOB + GENDER_DESC_BOB,

		+ BLOCKROOM_DESC_BOB + GENDER_DESC_BOB + TAG_DESC_FRIEND
		+ MATRICULATION_NUMBER_DESC_AMY + MATRICULATION_NUMBER_DESC_BOB,

		@@ -57,6 +62,15 @@ public Address getAddress() {
		return address;
		}

Update add command to include block and room #63

Update add command to include block and room #63

Conversation

schoolex commented Oct 6, 2020

codecov-io commented Oct 8, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

athuyaoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schoolex Oct 12, 2020 • edited

Choose a reason for hiding this comment

athuyaoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Oct 8, 2020 •

edited

schoolex Oct 12, 2020 •

edited