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-T10-1] EasInternship #12

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

Conversation

Ridiculouswifi
Copy link

EasInternship aims to streamline the process of searching for internship opportunities. It optimized for CLI users who are familiar with typing commands and general CLI usage.

Comment on lines 10 to 75
public void execute(ArrayList<String> args) {
String role = "";
String company = "";
String startDate = "01/01";
String endDate = "01/01";
boolean hasRole = false;
boolean hasCompany = false;

uiCommand.clearInvalidFlags();
for (String arg : args) {
String[] words = arg.split(" ", 2);
String flag = words[0];
switch (flag) {
case "role":
hasRole = true;
if (words.length > 1) {
role = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "company":
hasCompany = true;
if (words.length > 1) {
company = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "from":
if (words.length > 1) {
startDate = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "to":
if (words.length > 1) {
endDate = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
default:
uiCommand.addInvalidFlag(flag);
break;
}

}

if (!hasRole) {
uiCommand.addInvalidFlag("role");
}

if (!hasCompany) {
uiCommand.addInvalidFlag("company");
}
if (!uiCommand.getInvalidFlags().isEmpty()) {
uiCommand.printInvalidFlags();
return;
}

Internship newInternship = new Internship(role, company, startDate, endDate);
internships.addInternship(newInternship);
uiCommand.showEditedInternship(newInternship, "add");
}
Copy link

Choose a reason for hiding this comment

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

This method is a bit long, perhaps you could refactor it to separate the argument parsing and the logic for creation of a new internship.

Comment on lines 22 to 56
switch (flag) {
case "role":
hasRole = true;
if (words.length > 1) {
role = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "company":
hasCompany = true;
if (words.length > 1) {
company = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "from":
if (words.length > 1) {
startDate = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
case "to":
if (words.length > 1) {
endDate = words[INDEX_DATA];
} else {
uiCommand.addInvalidFlag(flag);
}
break;
default:
uiCommand.addInvalidFlag(flag);
break;
}
Copy link

Choose a reason for hiding this comment

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

Since the if checks and else statements are the same for all the switch cases, maybe you could do the if else checks before the switch cases so that the switch cases would be more concise.

Comment on lines 52 to 61
public void updateStatus(String userStatus) throws InvalidStatus {
List<String> statuses = Arrays.asList("Application Pending", "Application Completed", "Accepted", "Rejected");
for (String status : statuses) {
if (status.equalsIgnoreCase(userStatus)) {
this.status = status;
return;
}
}
throw new InvalidStatus();
}
Copy link

Choose a reason for hiding this comment

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

You could consider using an enum here to make the code easier to update and expand on in the future.

Copy link

@3CCLY 3CCLY left a comment

Choose a reason for hiding this comment

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

Quite a detailed documentation guide, good job on that


This is represented with the class diagram below:

![](UML/Internship-Model_Component.png)
Copy link

Choose a reason for hiding this comment

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

I believe the visibility of the methods and attributes in this diagram should be signs like "-" and "+" instead of squares and circles. You can consider configuring your plantuml to reflect this.

Comment on lines 32 to 39
### 1.1 Architecture
The architecture of EasInternship is designed to follow the MVC (Model-View-Controller) pattern to facilitate separation of concerns, modularity, and maintainability.

- **View (UI)**: Responsible for interacting with the user by displaying output and reading input.
- **Controller (Command and Parser)**: Responsible for parsing user input and invoking the appropriate commands.
- **Model (InternshipList)**: Manages the state of the application, including the list of internships and tasks.
- **Storage**: Responsible for loading and saving data from and to the disk.

Copy link

Choose a reason for hiding this comment

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

I feel that an architecture diagram help could be a very helpful diagram to add here to give a brief overview of how your different classes interact with each other

Copy link

Choose a reason for hiding this comment

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

Is the format for the "new Internship()" method call correct? My understanding is that it should call a constructor of a new object of "InternshipList" that is created at that point, instead of pointing to an existing object of "InternshipList" with a lifeline before the "new Internship()" method is called.

Comment on lines 94 to 97
### Overview

The `EasInternship` class serves as the entry point of the application. It manages the application's main loop, where the user is continually prompted for input, and commands are processed in response. The class is responsible for initializing the UI, loading saved data, and handling user input until the user chooses to exit the program.

Copy link

Choose a reason for hiding this comment

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

I feel that the overview would be more fitting being placed earlier in the documentation guide, perhaps you can give this some consideration?


## 1. Design

### 1.1 Architecture

Choose a reason for hiding this comment

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

Consider adding an architecture diagram


This is represented with the class diagram below:

![](UML/Internship-Model_Component.png)

Choose a reason for hiding this comment

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

iirc we are supposed to change the default style of the class diagram that is given by PlantUML? Ie use "+" and "-" instead of red squares and green circles.


This is represented with the class diagram below:

![](UML/Internship-Model_Component.png)

Choose a reason for hiding this comment

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

also can consider only adding the important methods, instead of spamming all your methods to improve clarity

There are two main functions, `loadFromFile` and `saveFromFile`.

The following sequence diagrams depict how the Storage Functions work.
![](UML/loadFromFile.png)

Choose a reason for hiding this comment

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

the sequence diagram shouldnt have 2 sets of class name?
image


---

### Overview

Choose a reason for hiding this comment

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

This structure feels abit weird to me. Might be better if you continued to follow the naming structure above? (ie 1.2, 1.3, 2.1, etc)

Ridiculouswifi and others added 27 commits November 2, 2024 21:38
…r-stories

update the value proposition and user story
Add CalendarCommand getUsage to HelpCommand
# Conflicts:
#	src/main/java/seedu/duke/Internship.java
#	src/main/java/seedu/duke/Parser.java
#	src/main/java/seedu/duke/Storage.java
update the ug for stanard output for sortCommand
Implement functionality to mark internships as favourite and filter by favourite internships
rahuljai-05 and others added 30 commits November 12, 2024 01:53
# Conflicts:
#	docs/DeveloperGuide.md
Add FilterCommand and FavouriteCommand to DG index
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.

7 participants