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-T09-4] easyLog #16

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

Conversation

ongweisheng
Copy link

@ongweisheng ongweisheng commented Mar 4, 2021

easyLog is a command line interface (CLI) application for warehouse employees to manage their items and orders in their warehouse. It is optimized for CLI users so that frequent tasks can be done faster by typing in commands.

Copy link

@oscarlai1998 oscarlai1998 left a comment

Choose a reason for hiding this comment

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

Generally the developer guide is missing a lot of information but the description used is pretty detailed and concise.


1. Fork the easyLog repository [here](https://github.com/AY2021S2-CS2113T-T09-4/tp) and git clone it to any location on
your computer.

Choose a reason for hiding this comment

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

There is no subsection on how to read the developer guide.

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)

Choose a reason for hiding this comment

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

Links are broken and not working

Comment on lines 36 to 51
### 1.3 Setting up

1. Fork the easyLog repository [here](https://github.com/AY2021S2-CS2113T-T09-4/tp) and git clone it to any location on
your computer.

2. Open any IDE (IntelliJ Idea preferred as mentioned in the prerequisites) and click `Configure` -> `Project Defaults`
-> `Project Structure` -> `New`

3. Next, go to `Import Project` and select the build.gradle file.

4. After opening the project, go to `src` -> `main` -> `java` -> `seedu.easylog` -> EasyLog and right click on the
EasyLog class.

5. Upon successful run, the following opening message will be shown:

```

Choose a reason for hiding this comment

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

There are no instructions on how to set up with the jar file. This should be added so users know how to run using the jar file


![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)

Different Commands execute by the program.

Choose a reason for hiding this comment

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

I believed it should be executed instead of execute

Copy link

@ivanchongzhien ivanchongzhien left a comment

Choose a reason for hiding this comment

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

Overall looks good! Diagram styles seem consistent and clean, just a few diagram and grammatical issues to check on.

Comment on lines 72 to 76

### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)

Choose a reason for hiding this comment

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

I think the use of the rectangle with the clipped corner to label your diagram is incorrect? I think that label is mainly used for labelling for loops, while loops etc. in sequence diagrams.

This usage is seen in other diagrams as well.

rectangle clipped

Comment on lines 86 to 89
### 2.5 Parser Component

![Parser Diagram](https://user-images.githubusercontent.com/57165946/112991918-42677680-919a-11eb-8868-79391cfc2c9e.png)

Choose a reason for hiding this comment

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

Perhaps consider using a caption at the bottom of the diagram to label it instead?


![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)

Different Commands execute by the program.

Choose a reason for hiding this comment

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

I think there's a typo here (executed*) ?

Comment on lines 60 to 64
### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)


Choose a reason for hiding this comment

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

Consider using the same line style throughout the diagram (and the entire DG as well)? Currently, three different line styles are used, straight angled lines, curved lines, and 90 degree lines (refer image)

Also consider rearranging the components to try avoid crossing over of lines?

linestyle

{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing}
{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing} <br> <br>

[Return to Top](#introduction)

Choose a reason for hiding this comment

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

Nice return to top feature! Maybe you have not implemented it so it does not work on the website. Otherwise, nice feature to have.

Comment on lines 26 to 30
The purpose of this developer guide is to allow any interested contributors, who wish to develop this application
further or just curious about the workings of this application. This would allow potential contributors to be able to
dive right in to improving the code, performance, features or even adding new features much more easily due to the
understanding of the structure of the codebase and implementation of existing features.

Choose a reason for hiding this comment

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

Good job, purpose is stated! However, you might want to check the comma in the first sentence.

Copy link

@Hemrish Hemrish left a comment

Choose a reason for hiding this comment

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

Good attempt overall! Contains minor issues that can easily be rectified


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)
Copy link

Choose a reason for hiding this comment

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

image
It would have been better to include an empty attribute box for SaveData.
Also, the methods should contain a pair of brackets at the end


### 2.5 Parser Component

![Parser Diagram](https://user-images.githubusercontent.com/57165946/112991918-42677680-919a-11eb-8868-79391cfc2c9e.png)
Copy link

Choose a reason for hiding this comment

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

image
image

It would be better if the parameters for a method is included in the class diagram. Methods without parameters can also have an empty pair of brackets at the end.


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)
Copy link

Choose a reason for hiding this comment

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

image
image
repeatedOrdersAdd is missing its parameters

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)

Choose a reason for hiding this comment

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

Is it better to include the links to each section right after it is created so that it won't be missed out in the future?


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)

Choose a reason for hiding this comment

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

The UML class diagram shown below for the storage component seems wrong. The SaveData and Receipt class should only have 3-row boxes, one for the class name, one for attributes and the last one for method. The methods should be combined in one box.

image

Comment on lines 30 to 172
* *glossary item* - Definition
Abbreviation | Full title | Definition
-------- | ---------- | ----------
**CI** | Continuous Integration | Combining parts of a software product to form a whole
**IntelliJ** | IntelliJ | An Integrated Development Environment written in Java
**CLI** | Command Line Interface | A program that accepts text inputs to execute operating system functions
**GUI** | Graphical User Interface | An interface that allows users to interact through graphical icons
**Mainstream OS** | Windows, Linux, Unix, OS-X | Operating systems

Choose a reason for hiding this comment

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

I like the way your team added technical jargon to the glossary. It makes things clearer for developers not familiar with certain terms used in the document.


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)

Choose a reason for hiding this comment

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

The command component UML diagram seems a bit cluttered and too complex. The arrow in the UML class diagram should be pointing to the parent class that the class is inherited from instead of the other way round. You may consider extract out the child of OrdersCommand and ItemsCommand into separate diagrams to provide multi-level views without being too complex.

image


### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)

Choose a reason for hiding this comment

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

Is it better to realign the components so that the arrow lines do not overlap with each other?


### 2.5 Parser Component

![Parser Diagram](https://user-images.githubusercontent.com/57165946/112991918-42677680-919a-11eb-8868-79391cfc2c9e.png)

Choose a reason for hiding this comment

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

You may consider adding a footer or caption for all diagrams in the guide to explain what is the diagram showing.

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)
Copy link

Choose a reason for hiding this comment

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

Table of contents incomplete.


### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)
Copy link

Choose a reason for hiding this comment

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

Such lines can be ambiguous as to where the line starts from, it could be from storage or command.
DGreview1


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)
Copy link

Choose a reason for hiding this comment

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

methods should have a () after the method name to show difference between class methods and class variables. Methods and variables should be grouped together respectively in 1 compartment in the diagram each.
image


### 2.5 Parser Component

![Parser Diagram](https://user-images.githubusercontent.com/57165946/112991918-42677680-919a-11eb-8868-79391cfc2c9e.png)
Copy link

Choose a reason for hiding this comment

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

Too many compartments in each component of the diagram. 1 component should only have 3 components, name, class variables and class methods. Class variables and methods should be grouped together in the same compartments.
image


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)
Copy link

Choose a reason for hiding this comment

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

Arrows should be pointing towards the parent class instead of the child. Diagram is too cluttered with confusing arrows and lines. Lines that intersect can be confusing as to where are the ends of the arrow.
image

{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing}
{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing} <br> <br>

[Return to Top](#introduction)
Copy link

Choose a reason for hiding this comment

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

Return to top link is broken


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)
Copy link

Choose a reason for hiding this comment

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

Style of diagram significantly different from the previous 2 class diagrams. Can consider changing to the same format and style for better consistency throughout the developer guide.


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)

Choose a reason for hiding this comment

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

For a class diagram, "( )" should be appended to class methods to ensure clear distinction between class variables and class methods. Also, all class variables should be grouped under one compartment in the class and all class methods should be grouped another compartment of the class instead of each attribute and method having its own separate compartment, like in the class Receipt.
112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5

{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing}
{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing} <br> <br>

[Return to Top](#introduction)

Choose a reason for hiding this comment

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

Please make sure to check that all links in your guide work.

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)

Choose a reason for hiding this comment

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

Table of contents is incomplete.

Comment on lines 33 to 34
- Java 11 (can be downloaded from [here](https://www.oracle.com/sg/java/technologies/javase-jdk11-downloads.html))
- IntelliJ Idea (can be downloaded from [here](https://www.jetbrains.com/idea/download/#section=mac))

Choose a reason for hiding this comment

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

I like that you included links to the prerequisite software. Really helps someone new who is not tech savvy to figure out how to set your app up.

Comment on lines 165 to 172
Abbreviation | Full title | Definition
-------- | ---------- | ----------
**CI** | Continuous Integration | Combining parts of a software product to form a whole
**IntelliJ** | IntelliJ | An Integrated Development Environment written in Java
**CLI** | Command Line Interface | A program that accepts text inputs to execute operating system functions
**GUI** | Graphical User Interface | An interface that allows users to interact through graphical icons
**Mainstream OS** | Windows, Linux, Unix, OS-X | Operating systems

Choose a reason for hiding this comment

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

I like the inclusion of a glossary of technical terms to help the people who are unfamiliar with these terms.


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)

Choose a reason for hiding this comment

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

The arrows from Command to its 4 subclasses should be the other way round. Also, hollow arrowheads should be used instead of solid arrowheads to represent inheritance, like what yall did for the Parser and Storage.

Please ensure that it is reflected in your diagram that Command is an abstract class by including the tag {abstract}.
112992782-15679380-919b-11eb-8c46-436ba701a754

Comment on lines +38 to +49
1. Fork the easyLog repository [here](https://github.com/AY2021S2-CS2113T-T09-4/tp) and git clone it to any location on
your computer.

2. Open any IDE (IntelliJ Idea preferred as mentioned in the prerequisites) and click `Configure` -> `Project Defaults`
-> `Project Structure` -> `New`

3. Next, go to `Import Project` and select the build.gradle file.

4. After opening the project, go to `src` -> `main` -> `java` -> `seedu.easylog` -> EasyLog and right click on the
EasyLog class.

5. Upon successful run, the following opening message will be shown:

Choose a reason for hiding this comment

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

The instructions are clear, comprehensive and easy for anyone to follow. Nice!

Copy link

@marklowsk marklowsk left a comment

Choose a reason for hiding this comment

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

I think your DG is heavily still in WIP, keep up the hard work and progress!


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)

Choose a reason for hiding this comment

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

image
I think there are some mistakes in the diagram:
{abstract} keyword not stated in diagram.
extends not used in diagram.


### 2.5 Parser Component

![Parser Diagram](https://user-images.githubusercontent.com/57165946/112991918-42677680-919a-11eb-8868-79391cfc2c9e.png)

Choose a reason for hiding this comment

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

image

image

I think you should adhere to the UML standards from the above SS of our course material.


### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)

Choose a reason for hiding this comment

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

image

I'm not sure if this will count as a bug, but the aforementioned components are not mentioned in the document or the architecture diagram.


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)

Choose a reason for hiding this comment

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

I am not sure if you are missing the method () brackets or you did not follow the standard for UML class diagrams.
attribute types are also missing.

Copy link

@xseh xseh left a comment

Choose a reason for hiding this comment

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

Overall needs more details and labeling, generally good flow and organisation. Could be improved with usage of icons and other markup to make the DG easier to read and understand. Consider standardising and simplifying the UML diagrams and use accurate symbols to represent each component.

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)
Copy link

Choose a reason for hiding this comment

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

Table of contents is incomplete

Copy link

Choose a reason for hiding this comment

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

Is it better to include the links to each section right after it is created so that it won't be missed out in the future?

I agree with kewenlok.
Maybe it is better to do so as the table of contents may grow larger than expected?


### 2.6 Command Component

![Command Diagram](https://user-images.githubusercontent.com/57165946/112992782-15679380-919b-11eb-8c46-436ba701a754.png)
Copy link

Choose a reason for hiding this comment

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

Diagram is inaccurate, the use of arrows to represent parent and child classes are wrong. Consider using a triangle for an arrow head (the arrow head is a bit ambiguous) or follow the previous UML diagrams for consistency. Can also consider simplifying the diagram by omitting the irrelevant or less important details.


Different Commands execute by the program.

### 2.7 Model Component
Copy link

Choose a reason for hiding this comment

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

Empty. Missing description, UML diagrams and other components.

Comment on lines 101 to 102
### 2.8 Exceptions Component
Possible exceptions existing in the program.
Copy link

Choose a reason for hiding this comment

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

Missing information. Details such as description, diagrams and other components can be added to make the component easier to understand for developers.

- Constants: Fixed values used in the program
- Messages: Output response from the program.

## Implementation
Copy link

Choose a reason for hiding this comment

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

Empty. Consider populating the page with detailed descriptions of each class and other implementation details.

Comment on lines 65 to 66
The Architecture Diagram shown above illustrates the high-level design of easyLog. We will now proceed to explain each
component and their respective functionalities below.
Copy link

Choose a reason for hiding this comment

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

Further elaborations on how each components interacts with one another would improve the documentation. Consider adding more details such expanding on the various components and a brief description of each one.


### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)
Copy link

Choose a reason for hiding this comment

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

Non-standard usage of diagram symbols. I believe the shapes used for labeling the storage component is inaccurate; it is used in UML to indicate labels of various types of code instead of as a label for the name of component. Consider using a note or omitting it entirely to avoid confusion. Applies to the rest of the diagrams.

Also consider using labeling or captions to describe the type of diagram drawn (Object, class or sequence diagrams), making understanding the diagram easier.

Looking for save data.
Save data not found.
```

Copy link

Choose a reason for hiding this comment

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

Consider using various icons to indicate different levels of elaboration. Collate the icons and its usages into a table for easy reference. The icons and purpose allows developers to understand how to use the DG and it improves organisation and overall readability of the DG.

Comment on lines 33 to 34
- Java 11 (can be downloaded from [here](https://www.oracle.com/sg/java/technologies/javase-jdk11-downloads.html))
- IntelliJ Idea (can be downloaded from [here](https://www.jetbrains.com/idea/download/#section=mac))
Copy link

Choose a reason for hiding this comment

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

Can consider adding information on the reasons for using this software and add indication of recommendation. For example, "Tip: IntelliJ is recommended as the DG gives instructions on how to set up the application using IntelliJ". The same applied for Java 11 (can be because of compatibility, etc. )

Comment on lines 81 to 84
### 2.4 Ui Component

The ui component deals with interactions with the user by displaying the appropriate messages according to the users
input.
Copy link

Choose a reason for hiding this comment

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

Further elaboration needed, consider adding some diagrams and detailed descriptions on the innerworkings of the UI components.

Copy link

@tzexern tzexern left a comment

Choose a reason for hiding this comment

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

Good job for some parts in the DG!

Even though it's largely incomplete, it's good to clear up some misconceptions of the UML diagram requirements set by the module.
Changes are required for better overall readability.

Comment on lines 9 to 11
- [Introduction](#introduction)
* [To be added](#to-be-added)
+ [To be added](#to-be-added)
Copy link

Choose a reason for hiding this comment

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

Is it better to include the links to each section right after it is created so that it won't be missed out in the future?

I agree with kewenlok.
Maybe it is better to do so as the table of contents may grow larger than expected?


### 2.1 Architecture

![Architecture Diagram](https://user-images.githubusercontent.com/57165946/112990632-ddf7e780-9198-11eb-99ec-f29aafbfc04f.png)
Copy link

Choose a reason for hiding this comment

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

image

Maybe it's better to avoid crossing of edges between the classes?


### 2.3 Storage Component

![Storage Diagram](https://user-images.githubusercontent.com/57165946/112991416-ab022380-9199-11eb-9ead-cdfcd19cf8e5.png)
Copy link

Choose a reason for hiding this comment

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

image

Should the top left box be omitted instead? (This can be seen in other UML diagrams in this DG as well)

For the Receipt class, should there be three rows instead? (Similar occurrences can be seen in other UML diagrams in this DG as well)


![Command Diagram](https://user-images.githubusercontent.com/60378963/113083308-92802080-920e-11eb-8e4b-66bc7fdf79f2.png)

Different Commands execute by the program.
Copy link

Choose a reason for hiding this comment

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

Maybe a more elaborate description is needed here?

e00426142 and others added 21 commits April 1, 2021 12:48
Updated UI with no extra line spacing
- App initialization diagram
- Explanation for steps during app initialization
Fix Bug: Decimal not Accepted as Valid Item Unit Price
Fix minor bugs regarding storage component
Orders feature: check error messages and fix trim() problem.
The items add command of this version of easyLog is consistent with its orders add command. Specifically, the user needs to input item price and item command in 1 line. Also, the user will be prompted again and again if a pair of valid item price and stock is not given. The coding style is also improved.
…into ZhuZikun-FixItemsAddEXceptionsProblems

Fix Merge Conflict
…blems

Fix items add consistency issues and exceptions problems
Fix partial mismatching expections.
e00426142 and others added 30 commits April 12, 2021 22:12
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.