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

Implement link command #82

Merged
merged 4 commits into from
Sep 30, 2020

Conversation

dextertanyj
Copy link

@dextertanyj dextertanyj commented Sep 29, 2020

Resolves #86.
Closes #28.

@dextertanyj dextertanyj added this to the v1.2-extended milestone Sep 29, 2020
@dextertanyj dextertanyj requested review from a team, ypinhsuan and samlsm and removed request for a team September 29, 2020 11:01
Copy link

@samlsm samlsm left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

Comment on lines +49 to +54
// manually link first class to first student
ModuleClass moduleClass = model.getFilteredModuleClassList().get(0);
Student student = model.getFilteredStudentList().get(0);
Set<UUID> studentUuids = new HashSet<>(moduleClass.getStudentUuids());
studentUuids.add(student.getUuid());
ModuleClass modifiedModuleClass = new ModuleClass(moduleClass.getName(), studentUuids);
Copy link

Choose a reason for hiding this comment

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

would it be better to leave a line here?

Suggested change
// manually link first class to first student
ModuleClass moduleClass = model.getFilteredModuleClassList().get(0);
Student student = model.getFilteredStudentList().get(0);
Set<UUID> studentUuids = new HashSet<>(moduleClass.getStudentUuids());
studentUuids.add(student.getUuid());
ModuleClass modifiedModuleClass = new ModuleClass(moduleClass.getName(), studentUuids);
// manually link first class to first student
ModuleClass moduleClass = model.getFilteredModuleClassList().get(0);
Student student = model.getFilteredStudentList().get(0);
Set<UUID> studentUuids = new HashSet<>(moduleClass.getStudentUuids());
studentUuids.add(student.getUuid());
ModuleClass modifiedModuleClass = new ModuleClass(moduleClass.getName(), studentUuids);

Copy link
Author

Choose a reason for hiding this comment

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

I think for this case, it would be okay to not include an empty line above the comment since it is the first line in the method body. It will also be more in line with existing code.

src/test/java/seedu/address/testutil/TypicalStudent.java Outdated Show resolved Hide resolved
src/test/java/seedu/address/testutil/TypicalStudent.java Outdated Show resolved Hide resolved
src/test/java/seedu/address/testutil/TypicalStudent.java Outdated Show resolved Hide resolved
Copy link

@ypinhsuan ypinhsuan left a comment

Choose a reason for hiding this comment

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

Nice work!

@dextertanyj dextertanyj merged commit 10709c8 into AY2021S1-CS2103T-T10-4:master Sep 30, 2020
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.

Implement support for linking students with classes Link students to (module) classes
3 participants