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

Updated Developer Guide #1

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Updated Developer Guide #1

wants to merge 15 commits into from

Conversation

chuaweiwen
Copy link
Contributor

No description provided.


==== UI component

UI component provides with the API of Graphical User Interface. The figure below shows the structure of the UI component.

Choose a reason for hiding this comment

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

UI component provides with the API of GUI...what does this mean?
"The UI component provides the API to interact with the Graphical User Interface..."?

_Figure 8 : Interactions inside the Logic component for the `delete 1` command_

==== Model component
Model Component handles the contacts in address book and all it's stored details in data structures which provides API to create, read, update and delete the contacts in address book. The figure below shows the structure of Model component.
Copy link

@justinpoh justinpoh Oct 22, 2017

Choose a reason for hiding this comment

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

The Model component...
The figure below shows the structure of the Model component.


* stores a `UserPref` object that represents the user's preferences.
* stores the Address Book data.
* exposes an unmodifiable `ObservableList<ReadOnlyPerson>` that can be 'observed' e.g. the UI can be bound to this list so that the UI automatically updates when the data in the list change.
* does not depend on any of the other three components.

=== Storage component
==== Storage component
The storage component handles the storage of all data of address book in the local device. The figure below shows the structure of Storage component.
Copy link

@justinpoh justinpoh Oct 22, 2017

Choose a reason for hiding this comment

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

The Storage component handles...
...shows the structure of the Storage component.

@@ -302,9 +317,9 @@ The `ExportCommand` uses `XmlAddressBookStorage` class to generate a xml file ba
}
----

The method `getRangefromInput()` splits the String range using the regex "," and returns a String array for the different values in the String range.
The method `getRangefromInput()` splits the `String` range using the regex `","` and returns a `String` array for the different values in the `String` range.
Copy link

@justinpoh justinpoh Oct 22, 2017

Choose a reason for hiding this comment

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

I think using delimiter instead of regex sounds better.

@@ -315,16 +330,16 @@ private String[] getRangeFromInput() {
}
----

To determine which contacts should be added to the exportBook we have to check the the user input, there are 3 cases:
To determine which contacts should be added to the `exportBook` we have to check the the user input, there are three cases:

Choose a reason for hiding this comment

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

double "the"
I think we turn the comma into a full stop and start a new sentence:
To determine which contacts should be added to the exportBook we have to check the user input. There are three cases:

@@ -517,7 +540,7 @@ The binder for refreshing the image every time the picture is updated is impleme
}
----

The new image stored in directory is given a unique name which is formed using hashcode of the unique email address of each contact.
The new image stored in directory is given a unique name which is formed using hashcode of the unique email address of each contact:

Choose a reason for hiding this comment

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

"...formed by hashing the unique email address of each contact:"

* By specifying the index, there is no ambiguity as to who should be assigned the display picture.
* The `PATHOFIMAGE` must be an absolute path on the local device to make sure the image is referenced.
* The image is stored in directory with a unique name to avoid conflict. Hashcode of user's email address is used to maintain uniqueness.
* The task is done using CLI to follow project requirements.

Choose a reason for hiding this comment

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

fulfil instead of follow

**Pros:** Easier for new developers to understand the sequence diagram and maintains event-driven nature. +
**Cons:** There might be clash in naming of two different images for large database of contacts
**Cons:** There might be clashes in naming of two different images for large database of contacts.

Choose a reason for hiding this comment

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

"in the naming"

**Alternative 2:** Pop up a `FileChooser` after command is entered +
**Pros:** Easier for users to mention the correct image at a quick pace +
**Cons:** Will no longer be a CLI process completely
**Cons:** Might be problematic for user to copy and paste and might result in error of path giving fail command. +

Choose a reason for hiding this comment

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

Might be problematic for users to copy and paste, and this might result in an erroneous path given, leading to an invalid command.

**Cons:** Will no longer be a CLI process completely
**Cons:** Might be problematic for user to copy and paste and might result in error of path giving fail command. +
**Alternative 2:** Pop up a `FileChooser` after command is entered. +
**Pros:** Easier for users to mention the correct image quickly. +

Choose a reason for hiding this comment

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

choose instead of mention?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants