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

[CS2113T-F12-4] CCAManager #48

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

Conversation

ianwangeeen
Copy link

This product is designed for the NUS CCA manager who has a lot of administrative tasks to keep track of. The interface uses CLI instead of GUI to fit the preferences of the CCA manager.

Copy link

@kengjit kengjit left a comment

Choose a reason for hiding this comment

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

Overall well-done DG, perhaps you could further elaborate on the existing methods and classes

Comment on lines 45 to 56
### **Note:**
* `makeMemberEntry()`: Creates a member in the memberList.
* `writeMemberFile()`: Writes the data to the csv file.

The *sequence diagram* below shows how various components of the architecture interact with one another when a user inputs a **valid** command `"delete /m 1"`
![Architecture Sequence Diagram2](images/deleteMemberArchitecture.PNG)

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
### **Note:**
* `getKeywordStatus()`: Gets the keyword value from the Keyword enum class.
* `deleteMember(members, entry)`: Deletes the member from the memberList.
* `deleteMemeber(memberNumber)`: Deletes the member using the index number provided by the user.
* `writeMemberFile()`: Writes the data to the csv file.
Copy link

Choose a reason for hiding this comment

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

Perhaps it would be better to choose a better title than "Note"?


{Describe the value proposition: what problem does it solve?}
###[Proposed] Storage component
Copy link

Choose a reason for hiding this comment

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

You might want to add a whitespace between the ### and [Proposed] to prevent formatting problems

Comment on lines 46 to 56
* `makeMemberEntry()`: Creates a member in the memberList.
* `writeMemberFile()`: Writes the data to the csv file.

The *sequence diagram* below shows how various components of the architecture interact with one another when a user inputs a **valid** command `"delete /m 1"`
![Architecture Sequence Diagram2](images/deleteMemberArchitecture.PNG)

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}
### **Note:**
* `getKeywordStatus()`: Gets the keyword value from the Keyword enum class.
* `deleteMember(members, entry)`: Deletes the member from the memberList.
* `deleteMemeber(memberNumber)`: Deletes the member using the index number provided by the user.
* `writeMemberFile()`: Writes the data to the csv file.
Copy link

Choose a reason for hiding this comment

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

Could perhaps split these methods into different components? (i.e. Member, MemberStorage...)


The *sequence diagram* below shows how various components of the architecture interact with one another when a user inputs a **valid** command `"add /m Bob /s A01231234B /p 98765432"`

![Architecture Sequence Diagram](images/ArchitectureSequence.png)
Copy link

Choose a reason for hiding this comment

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

Perhaps you could standardize what you want User to be.

image
image

Copy link

@praj-bellakka praj-bellakka left a comment

Choose a reason for hiding this comment

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

Looks good so far, but there are some formatting issues and minor fixes that can be taken care of!

* [Getting Started](#getting-started)
* [Design](#design)
* [Architecture](#architecture)
* [Appendix: Requirements](#appendix-requirements)

Choose a reason for hiding this comment

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

Should there be a link to the Manual Testing section for consistency?


The *sequence diagram* below shows how various components of the architecture interact with one another when a user inputs a **valid** command `"add /m Bob /s A01231234B /p 98765432"`

![Architecture Sequence Diagram](images/ArchitectureSequence.png)

Choose a reason for hiding this comment

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

Should there be an 'X' to denote the end of the lifeline of an object to show its deletion? If PUML doesn't allow it, maybe note could be written at the start to explain this behaviour.


{Describe the value proposition: what problem does it solve?}
###[Proposed] Storage component

Choose a reason for hiding this comment

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

Perhaps your group missed out a space here, as the heading formatting is not showing up

Suggested change
###[Proposed] Storage component
### [Proposed] Storage component

Comment on lines 84 to 89
*can save attendance data in CSV format and read them back into the `AttendanceList` object.
*automatically deletes to AttendanceStorage.csv whenever a member is deleted from the `AttendanceList` object.

The `training schedule` component
*can save trainings schedules and read them back into the `TrainingList` object.
*automatically deletes in the file whenever a training schedule is deleted from the `TrainingList` object

Choose a reason for hiding this comment

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

Perhaps you are missing out on a space between the starting character and the star (*) to show the lines as a list

Suggested change
*can save attendance data in CSV format and read them back into the `AttendanceList` object.
*automatically deletes to AttendanceStorage.csv whenever a member is deleted from the `AttendanceList` object.
The `training schedule` component
*can save trainings schedules and read them back into the `TrainingList` object.
*automatically deletes in the file whenever a training schedule is deleted from the `TrainingList` object
* can save attendance data in CSV format and read them back into the `AttendanceList` object.
* automatically deletes to AttendanceStorage.csv whenever a member is deleted from the `AttendanceList` object.
The `training schedule` component
* can save trainings schedules and read them back into the `TrainingList` object.
* automatically deletes in the file whenever a training schedule is deleted from the `TrainingList` object

Teckwhye and others added 26 commits October 27, 2021 13:28
Add delete by name for member
Change information regarding delete member command and expected output for member commands
Add find training function, add Index to TrainingSchedule
Teckhwee v2.0
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.

8 participants