Skip to content

[CS2113-F14-2] CEGStudyBuddy#36

Open
HightechTR wants to merge 585 commits into
nus-cs2113-AY2425S2:masterfrom
AY2425S2-CS2113-F14-2:master
Open

[CS2113-F14-2] CEGStudyBuddy#36
HightechTR wants to merge 585 commits into
nus-cs2113-AY2425S2:masterfrom
AY2425S2-CS2113-F14-2:master

Conversation

@HightechTR
Copy link
Copy Markdown

@HightechTR HightechTR commented Feb 21, 2025

CEGStudyBuddy is a desktop application that tracks and plans university courses efficiently for NUS Computer Engineering students who want a structured academic roadmap.

@nus-se-bot nus-se-bot closed this Feb 21, 2025
@damithc damithc reopened this Feb 21, 2025
rchlai added a commit to rchlai/tp that referenced this pull request Mar 16, 2025
Ashertan256 pushed a commit to Ashertan256/tp that referenced this pull request Mar 20, 2025
…ses-of-members

Clear expense amounts upon deletion. To implement: logic to delete ex…
title += " " + parts[i];
i += 1;
}
// reset i back to the ending element of title
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid comments for your own/coder's reference. Write comments for the reader's reference to maximize readability.

}
// reset i back to the ending element of title
i -= 1;
} else if (parts[i].startsWith("mc/")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid the usage of magic strings

} else if (parts[i].startsWith("mc/")) {
mc = Integer.parseInt(parts[i].substring(3));
} else if (parts[i].startsWith("y/")) {
y = Integer.parseInt(parts[i].substring(2));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid the usage of magic numbers as well.

Comment thread docs/DeveloperGuide.md
@@ -1,38 +1,369 @@

# Developer Guide

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your developer guide is concise and detailed! Keep up your great work!

Comment thread docs/DeveloperGuide.md Outdated
### Command Component
All of CEGStudyBuddy’s commands have their own command class, which are subclasses of an abstract Command class.

![CommandClassDiagram.png](CommandClassDiagram.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If i recall, Prof Akshay said to remove the non-standard icons beside the class in the diagram

Comment thread docs/DeveloperGuide.md Outdated
- It creates a `PlanData` folder to store data if one doesn’t already exist.
- It loads the user’s course plan at startup and saves changes automatically.

![img_7.png](img_7.png)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are the names in the diagram correct? Think should have semicolons before the name - eg. :SwitchPlanCommand instead of SwitchPlanCommand

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe can add a few activation bars

Comment thread docs/DeveloperGuide.md
---

## Design

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe can consider having an overall architecture diagram/explanation for easier visualisation of all the components

Copy link
Copy Markdown

@jiajun2002 jiajun2002 left a comment

Choose a reason for hiding this comment

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

nice diagram overall!

Rishi7830 and others added 30 commits April 8, 2025 08:29
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.

10 participants