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

[CS2113-T16-2] Elderly In Your Hood #39

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

Conversation

owensoh
Copy link

@owensoh owensoh commented Sep 30, 2021

Our team intends to assist the elderly helpers by automating certain tasks for them.

@@ -8,26 +8,192 @@

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

### Elderly risk categorisation

Choose a reason for hiding this comment

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

Maybe this section should be further down instead of being the first section, as the first section is usually a brief introduction of the app/overview of architecture.

@@ -8,26 +8,192 @@

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

### Elderly risk categorisation
#### Implementation
The elderly risk categorisation is an addition to the current `Elderly` class. It utilises

Choose a reason for hiding this comment

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

Since it is an addition to the Elderly class, the section on the Elderly class should probably be above this.

Choose a reason for hiding this comment

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

The Elderly class is not described before this section, this can cause confusion.

- `Doctor` — A class that contains information about a doctor

Below is a UML class diagram for the elderly risk categorisation:
![](team/img/elderly_implementation.png)

Choose a reason for hiding this comment

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

The class diagram formatting is wrong as the attribute type is not shown, and there are no brackets behind each method (containing parameters when needed).
image

![](team/img/elderly_implementation.png)


#### How the classes interact with each other

Choose a reason for hiding this comment

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

This section is comprehensive and well written :)

UML Diagram of the interaction between classes is shown below :
![Screenshot](https://user-images.githubusercontent.com/70097982/138797711-e0545ea3-c0bd-48fc-9e28-48a2814e255f.png)

#### Interaction of Classes

Choose a reason for hiding this comment

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

Would be good to add a sequence diagram here to summarise how the function works as the description is slightly lengthy.


## Non-Functional Requirements

{Give non-functional requirements}
1. The product is not required to ensure that the dosage of medicine keyed are safe.
2. The product should work on both 32-bit and 64-bit system.
3. This version of product does not allow for addition of hospital and doctors.

## Glossary

Choose a reason for hiding this comment

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

In general, the DG is well written but can be organised better (the introduction seems to be at the bottom instead of the top) and have a section for the overall architecture/flow of the app. Would also be good to have a table of contents for better navigability.

Copy link

@nvbinh15 nvbinh15 left a comment

Choose a reason for hiding this comment

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

Overall, the DG is well-written with a lot of content included. There are some minor bugs with the class diagrams, and a table of content can be included for better navigability. Well done!

@@ -8,26 +8,192 @@

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

### Elderly risk categorisation

Choose a reason for hiding this comment

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

The sections should be numbered and a table of content with links should be included so readers can navigate easily.

@@ -8,26 +8,192 @@

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

### Elderly risk categorisation
#### Implementation

Choose a reason for hiding this comment

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

There should be a blank line after the section header, same issue at line 48.

- `Doctor` — A class that contains information about a doctor

Below is a UML class diagram for the elderly risk categorisation:
![](team/img/elderly_implementation.png)

Choose a reason for hiding this comment

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

There should be brackets () after method names. Also, the relationships between classes are already shown by the arrows, so don't need to put Doctor and Hospital in the attribute sections of MediumRiskElderly and HighRiskElderly.

Screen Shot 2021-10-28 at 09 41 15

Choose a reason for hiding this comment

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

For the elderly class, there should be two horizontal bars to indicate that there is no attribute and no method in this class

Choose a reason for hiding this comment

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

The association were specified with multiplicity but name of the association is not specified

Choose a reason for hiding this comment

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

The attribute data type and method return type are not specified

This information in both classes is then stored in `Elderly` class.

UML Diagram of the interaction between classes is shown below :
![Screenshot](https://user-images.githubusercontent.com/70097982/138797711-e0545ea3-c0bd-48fc-9e28-48a2814e255f.png)

Choose a reason for hiding this comment

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

Same issue with the previous class diagram. The relationships between Elderly, Next-Of-Kin, and Record should be shown with arrows. Therefore, the attributes can be omitted.

When using the `findbymed`, `findbydiet`, `findbyname` functions, they will take generate a set of
medicine, diets and real names respectively. The function `checkSimilarities` in `ElderlyList.class`
will then iterate through each item in the set and compare it with the query term. If there is a
similarity score of `>=0.8`, the closest match will be printed.

Choose a reason for hiding this comment

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

perhaps you could show a diagram (sequence or class) to complement this explanation?


UML Diagram of the interaction between classes is shown below :
![Screenshot](https://user-images.githubusercontent.com/70097982/138797711-e0545ea3-c0bd-48fc-9e28-48a2814e255f.png)

Choose a reason for hiding this comment

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

this is correct UML notation, the first diagram used in the guide can be edited to match this notation (although some standardisation is needed e.g. in Elderly members and visibility symbol have a space in between but not in the other classes)


#### Interaction of Classes
`Elderly` class will interact with `Next-of-kin` class through the use of `addNok()` function. This function will

Choose a reason for hiding this comment

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

unsure if this blank line is intended or not, but does not seem to serve any purpose and is quite abrupt

is used to view the details that are stored.

#### Future Implementation and Considerations

Choose a reason for hiding this comment

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

a short explanation of how exactly this can be implemented could be given (similar to proposed undo/redo feature in AB3 developer guide here)


## Non-Functional Requirements

{Give non-functional requirements}
1. The product is not required to ensure that the dosage of medicine keyed are safe.
2. The product should work on both 32-bit and 64-bit system.
3. This version of product does not allow for addition of hospital and doctors.

## Glossary
Copy link

@rainish2000 rainish2000 Oct 28, 2021

Choose a reason for hiding this comment

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

Adding to the previous reviewer's feedback, your team could look into having more diagrams of different types such as sequence diagrams (currently only 2 diagrams that are both class diagrams) to show your UML and documentation skills.

Comment on lines +106 to +111
The `Elderly` class interacts with two classes, which is `Next-of-Kin` class and `Record` class. As can
be seen by the association between the three classes, the `Elderly` class stores `Next-of-Kin` and `Record`
as ArrayLists in its attributes.
- `Next-of-Kin` class stores the contact information of the Next-of-Kin, such as name, phone number,
email, address and relationship to the Elderly
- `Record` stores the details of the Elderly, such as the elderly phone number and address

Choose a reason for hiding this comment

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

image

Would a sequence diagram help to explain the process?

Comment on lines 85 to 103
#### An example of AutoSuggestions
Assuming we have an elderly that exist in the system called `John Tan`. We, as the user, remember
that his name was `<SOMETHING> Tan`, and it was either `Josh` or `John`. We try `Josh`
first.

![Example of AutoSuggestions](team/img/autosuggestions_example.png)

Instead of just prompting that the system could not find `Josh Tan`, the search term went through
the Levenshtein Distance logic and the name `John Tan` returned with a result of `0.9375`. As you
can see, this was our intended result and showcases the intended solution to our problem.

#### Potential Shortcomings of this function
There may be two strings that have similar edit distance to each other but may not be related to
each other. This will include words like `sitting` and `kitten` for example. This may give
inaccurate suggestions. However, since this is **not a key function** and is only meant to be a
complementary tool, **it will not be a major problem**.

### Addition of Next-of-Kin and Record Classes
#### Implementation

Choose a reason for hiding this comment

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

image
Would the document look more structured if more indentation was applied or more blank spaces between paragraphs ?

ruyian and others added 30 commits November 7, 2021 22:10
Changes made to DG to prevent additional line
Change made to DG to add appendixes
Change the regex checker to allow useful invalid risk level
Edit minor bug with doctor
Changes made to DG to finish Appendix E and update table of contents
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.

9 participants