Skip to content

House, Buyer/Seller Capabilities #62

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

Merged

Conversation

redcolorbicycle
Copy link

Currently, there are no specific classes for representing houses (landed or not) in the address book application.

To differentiate between landed and non panded properties in the address book application and to allow for specific functionalities and attributes for each.

Three new classes, House, Landed and Nonlanded, are created. Each class has its own add commands and functionalities.

Commands for adding buyers and sellers are edited to indicate the housing type preferred. Furthermore, people now have the capability to own a house.

Copy link

@limcaaarl limcaaarl left a comment

Choose a reason for hiding this comment

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

Just some minor issues with the javadocs descriptions and some of the variable names.

import seedu.address.model.person.Email;
import seedu.address.model.person.Name;
import seedu.address.model.person.Person;
import seedu.address.model.person.Phone;
import seedu.address.model.person.Seller;
import seedu.address.model.tag.Tag;

/**
Copy link

@limcaaarl limcaaarl Mar 18, 2024

Choose a reason for hiding this comment

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

I think this should be documented as AddSellerCommand object instead of AddCommand object.

Edit: The tagging didn't work properly, but this is talking about the javadocs under this class.

Comment on lines 133 to 136
* Parses a {@code String street} into a {@code Street}.
* Leading and trailing whitespaces will be trimmed.
*
* @throws ParseException if the given {@code street} is invalid.

Choose a reason for hiding this comment

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

Wrong description here.

Comment on lines +36 to +81
/**
* Constructs a House with a different unit number for editing purposes.
*
* @param unitNumber The new unit number of the house.
* @param house The existing house object.
*/
public House(UnitNumber unitNumber, House house) {
this.unitNumber = unitNumber;
this.postalCode = house.getPostalCode();
this.street = house.getStreet();
}

/**
* Constructs a House with a different postal code for editing purposes.
*
* @param postalCode The new postal code of the house.
* @param house The existing house object.
*/
public House(PostalCode postalCode, House house) {
this.unitNumber = house.getUnitNumber();
this.postalCode = house.getPostalCode();
this.street = house.getStreet();
}

/**
* Constructs a House with a different street for editing purposes.
*
* @param street The new street of the house.
* @param house The existing house object.
*/
public House(Street street, House house) {
this.unitNumber = house.getUnitNumber();
this.postalCode = house.getPostalCode();
this.street = street;
}

/**
* Constructs a copy of the given house for editing purposes.
*
* @param house The house to copy.
*/
public House(House house) {
this.unitNumber = house.getUnitNumber();
this.postalCode = house.getPostalCode();
this.street = house.getStreet();
}
Copy link

@limcaaarl limcaaarl Mar 18, 2024

Choose a reason for hiding this comment

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

I think there's no need for overloaded constructor for editing purposes.

For AB3 iirc, edit person command just passes the original value of the field, even if it's unedited, to the very same constructor. So no matter which field you edit, it will use the same constructor.

I need someone to confirm this.

Choose a reason for hiding this comment

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

I think so too because if we based the edit based on the Person, they only have one constructor but you can edit the fields as per normal.

Comment on lines 16 to 19
private final Street street;
private final House house;
private final PostalCode postalCode;
private final UnitNumber unitNumber;

Choose a reason for hiding this comment

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

I think a seller just needs to keep track of houses.

@@ -28,23 +28,20 @@ class JsonAdaptedPerson {
private final String name;
private final String phone;
private final String email;
private final String address;
private final String postalCode;
private final String housingtype;

Choose a reason for hiding this comment

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

perhaps housingType?

@@ -35,7 +35,7 @@ public class PersonCard extends UiPart<Region> {
@FXML
private Label phone;
@FXML
private Label address;
private Label housingtype;

Choose a reason for hiding this comment

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

This one too housingType

import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.commands.CommandTestUtil.*;
Copy link

@limcaaarl limcaaarl Mar 18, 2024

Choose a reason for hiding this comment

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

I think you accidentally replaced

import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_ADDRESS_DESC;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_EMAIL_DESC;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_NAME_DESC;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_PHONE_DESC;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_TAG_DESC;
import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_HUSBAND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_AMY;
import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;

with
import static seedu.address.logic.commands.CommandTestUtil.*;

private final Address address;
private final PostalCode postalCode;
// critically, this refers to what type the buyer WANTS, and what type the seller HAS
private final String housingtype;

Choose a reason for hiding this comment

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

I think you can rename this to housingType instead of housingtype.

@@ -137,7 +141,7 @@ public static class EditPersonDescriptor {
private Name name;
private Phone phone;
private Email email;
private Address address;
private String housingtype;
Copy link
Collaborator

Choose a reason for hiding this comment

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

housingType?

+ PREFIX_ADDRESS + "311, Clementi Ave 2, #02-25 "
+ PREFIX_HOUSINGTYPE + "HDB "
+ PREFIX_STREET + "Clementi Ave 2 "
+ PREFIX_BLOCK + "Block 311 "

Choose a reason for hiding this comment

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

I think this part has to be changed, because the prefix_block + "Block 311 " should be "311 " instead, since the block can only take in numbers and one letter. I believe that it should be the changes should also be made to level and unit number.

But this one is quite minor since it is the UI, can be changed later.

@felixchanyy felixchanyy changed the base branch from master to branch-not-done-1 March 19, 2024 15:12
@felixchanyy felixchanyy merged commit 5de956e into AY2324S2-CS2103-F09-1:branch-not-done-1 Mar 19, 2024
|| !argMultimap.getPreamble().isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddSellerCommand.MESSAGE_USAGE));
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
Copy link
Collaborator

Choose a reason for hiding this comment

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

AddSellerCommand

@felixchanyy felixchanyy added the enhancement New feature or request label Mar 21, 2024
@felixchanyy felixchanyy added this to the v1.2 milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants