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-T10-3] Decodex #27

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

Conversation

rizemon
Copy link

@rizemon rizemon commented Sep 30, 2021

Decodex is a Command Line Interface (CLI) application for Capture-The-Flag (CTF) players to quickly transform data from one format to another with extreme ease. The intuitive interaction can help speed up a player's performance during CTFs and save time without having to manually code the tedious data transformations.

Copy link

@crabnuggets crabnuggets left a comment

Choose a reason for hiding this comment

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

This is a really good DG and the quality of the writing/diagrams is very consistently written throughout. Well done!

Comment on lines +19 to +21
- [Introduction](#introduction)
- [Purpose of This Guide](#purpose-of-this-guide)
- [Developer Guide Usage](#developer-guide-usage)

Choose a reason for hiding this comment

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

Perhaps it would be good to consider moving the table of contents to the top. Otherwise, it might seem a little queer to have to come across sections that the developer has already read (e.g. Introduction, Purpose of This Guide, Developer Guide Usage) in the table of contents.


### UI Component

![UiClass.png](images/dg/UiClass.png)

Choose a reason for hiding this comment

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

It is good that only the commonly used methods are listed in the diagram to avoid overloading the reader with information. Once again, a good well-drawn diagram that is very comprehensible.

Comment on lines 199 to 201
![CommandClass1.png](images/dg/CommandClass1.png)

![CommandClass2.png](images/dg/CommandClass2.png)

Choose a reason for hiding this comment

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

Since there are many derived *Command classes and the association between these classes and the abstract Command class is the same for all of them (i.e., inheritence), maybe it would be better to have one diagram that can generalize and demonstrate the relationship between these classes and the abstract Command class (i.e., just have a general *Command class representation inheriting from the Command class). It was somewhat tedious for the readerto read through both diagrams only to realise that the main takeaway was the inheritence relationship between the classes.


The ***Architecture Diagram*** given below shows the high-level design of Decodex.

![architecture.png](images/dg/architecture.png)

Choose a reason for hiding this comment

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

This is really clean and comprehensible! The additional note below the diagram aids readers in understanding the interactions between components. Very well done.


Below is the class diagram showing the association between the abstract `Module` class and its derived `*Decoder` classes.

![ModuleClass.png](images/dg/ModuleClass.png)

Choose a reason for hiding this comment

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

As mentioned, I think it is worth considering simplifying the diagram here as with the Command class.


![RecipeClass.png](images/dg/RecipeClass.png)

> :pen: The reason for having `Recipe` objects is to enable users to run multiple modules at one go sequentially.

Choose a reason for hiding this comment

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

I like the addition of this note here as it provides developers with an understanding of why certain components of the program are implemented the way they are.


Below shows the sequence diagram of the initialisation of Decodex.

![Decodex Initialisation](images/dg/DecodexInitSeq.png)

Choose a reason for hiding this comment

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

Very interesting use of colours and highlighting which actually helps with the readability of the diagram. It is also really good that the highlight colour chosen for each component is kept consistent in the later parts as well.


#### SelectCommand

![SelectCommand](images/dg/SelectCommand.png)

Choose a reason for hiding this comment

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

Choosing to omit specific and probably complex notations for the implementation of the running of certain modules was a really good choice here.

Copy link

@zonglun99 zonglun99 left a comment

Choose a reason for hiding this comment

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

Well written DG with coverage of most if not all classes. Instructions and descriptions are very clear and concise.

| Terminology | Definition |
| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Data transformation | The conversion of one data format to another. |
| Application, Program | Refer to the `Decodex` program. This two terms are used interchangeably in this User Guide. |

Choose a reason for hiding this comment

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

Do you mean Developer Guide instead of User Guide?

Comment on lines 77 to 98
### Terminologies

| Terminology | Definition |
| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Data transformation | The conversion of one data format to another. |
| Application, Program | Refer to the `Decodex` program. This two terms are used interchangeably in this User Guide. |
| Encoding | Convert a message into a coded form. |
| Decoding | Convert a coded message into an intelligible form |
| Base64, Binary, Hexadecimal | Common types of data encoding standards. |
| Console | This refers to your command prompt window. |
| Argument | The additional information you provide to the program's command. |
| Module | A self-contained set of instructions to process your data into another form. |
| Recipe | Acts as a container for you to select your modules. When multiple modules are selected, this forms a "module chain". By default, you do not have any recipes. |

### Symbols

| Name | Definition |
| -------------------- | ----------------------------------------------------------------------------------------------------- |
| :bulb: | Represents a good tip for you. |
| :exclamation: | Represents something important that you should take note of. |
| :information_source: | Represents additional information regarding commands/features for you better understand how it works. |
| :pen: | Represents our rationale behind the design/implementation. |

Choose a reason for hiding this comment

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

Good use of table to define terminologies and different symbols for users' understanding.


This group of modules require at least one additional parameter as their encoding and decoding processes are dependent on the provided parameters. The constructors of these modules should receive parameters as required and stored in private variables. These variables will then be used in their respective `run(Data data)` method.

The current implementation of the abstract `Module` class provides a strong foundation top be inherited by much more complex modules, and developed into full-functioning modules for Decodex.

Choose a reason for hiding this comment

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

Possible typo here. Do you mean to be instead of top be?


![ParserClass.png](images/dg/ParserClass.png)

Below is the class diagram showing the association between the abstract `Command` class and its derived `*Command` classes.

Choose a reason for hiding this comment

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

Suggestion: To be consistent with usage of either *command or XYZcommand, I think would be less confusing to stick to just 1, same for RecipeXYZcommand

Copy link

@zonglun99 zonglun99 left a comment

Choose a reason for hiding this comment

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

Well written DG with coverage of most if not all classes. Instructions and descriptions are very clear and concise.


The ***Architecture Diagram*** given below shows the high-level design of Decodex.

![architecture.png](images/dg/architecture.png)

Choose a reason for hiding this comment

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

Diagram has a typo, I think you meant a file instead of an file.
image

This developer guide is made for developers who wish to further understand and/or develop **Decodex**.
This guide includes the setup instructions, design, implementation, testing, product scope, and other sections to give developers a better understanding of the application.

## Table of Contents <!-- omit in toc -->

Choose a reason for hiding this comment

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

Slightly odd that Table of Contents comes after Introduction, when the Intro is actually included in the Table of Contents

image


## Design

### Architecture

Choose a reason for hiding this comment

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

Architecture section is really clean and understandable. Sufficient details and explanations have been given. Well done.

Copy link

@WeiXuanYap WeiXuanYap left a comment

Choose a reason for hiding this comment

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

lgtm :)


The `Ui` component consists of:

- `Ui`: Manages access to the `Scanner` object that reads user input and also contains all the methods for printing to the user.

Choose a reason for hiding this comment

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

Do not need to create bullet point if there is only one point under UI


Below is a partial class diagram that shows an overview of the `Logic` component.

![LogicComponent(4).png](images/dg/LogicComponent.png)

Choose a reason for hiding this comment

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

image
image

Based on week 10 DG Tips, it is preferable to use the same diagramming tool.
image

Comment on lines 335 to 341
Modules that do not take in any parameters

This group of modules do not require any additional parameters as their encoding and decoding processes are fixed. Hence, the constructors of these modules should not receive any parameters.

Modules that take in at least one parameter

This group of modules require at least one additional parameter as their encoding and decoding processes are dependent on the provided parameters. The constructors of these modules should receive parameters as required and stored in private variables. These variables will then be used in their respective `run(Data data)` method.

Choose a reason for hiding this comment

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

Should include a bullet point/bold/underline to show that this is a subsection/elaboration section

Kair0s3 and others added 25 commits November 1, 2021 18:48
Link user to select module examples in command summary.
Fixed the TOC linking to one section
And also remove one '>'
Added show command and some fixes
Changes to replace the list of modules to table format
…into TianBoon-BugFix#1

# Conflicts:
#	src/main/java/decodex/ui/messages/ErrorMessages.java
Add explanation on separators in data
…op logic, to instead run until, a exit command is found, then break.
Kair0s3 and others added 30 commits November 8, 2021 16:56
Removed the additional loading method for Decodex in Ui component
* Move TOC
* Update links to modules
* Fix Command Summary
Change list module argument to be optional
* Change input examples to inline code blocks
* Add note for last test case of creating recipes
…rguments

Fix extraneous arguments for `reset` command
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