-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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 Presentation Model Pattern #1710
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @EdisonE3 for your valuable contribution!
Please take a note of the following points:
- Increase Code Coverage try making it to 100%
- Use Logger to show some verbose logging of the work that is being done. Take a look at other implemented patterns to get an idea.
- Remove the Code Smells as reported by Sonar, let us know if there are any false positives.
Good Luck 👍🏻
/** | ||
*A class used to store the information of album. | ||
* | ||
* @author EdisonE3 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the author tag
/** | ||
* The constructor method of Album. | ||
* | ||
* @param rowId the id of the album | ||
* @param title the title of the album | ||
* @param artist the name of artist | ||
* @param isClassical whether the album is classical | ||
* @param composer the name of composer | ||
*/ | ||
public Album(int rowId, String title, String artist, boolean isClassical, String composer) { | ||
this.rowId = rowId; | ||
this.title = title; | ||
this.artist = artist; | ||
this.isClassical = isClassical; | ||
this.composer = composer; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use lombok
for boilerplate code
/** | ||
* a class used to start this demo. | ||
* | ||
* @author EdisonE3 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove author tag
/** | ||
* a class used to deal with albums. | ||
* | ||
* @author EdisonE3 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove author tag
/** | ||
* The class between view and albums, it is used to control the data. | ||
* | ||
* @author EdisonE3 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the author tag
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra spaces
/** | ||
* Generates the GUI of albums. | ||
* | ||
* @author EdisonE3 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the author tag
Thanks a lot! |
presentation/pom.xml
Outdated
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-pmd-plugin</artifactId> | ||
<configuration> | ||
<linkXRef>false</linkXRef> | ||
</configuration> | ||
<version>3.14.0</version> | ||
</plugin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, we use SonarCloud for static analysis
Thanks for your advices, I will try to make it better. |
Hello, the changes requested have been finished. @ohbus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @ohbus can merge once all the comments have been resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thank you so much for your contribution @EdisonE3
Kudos, SonarCloud Quality Gate passed! |
@all-contributors please add @EdisonE3 for code |
I've put up a pull request to add @EdisonE3! 🎉 |
Presentation Model Pattern
-Implemented Registry pattern addressing the issue #415
Description
-Represent the state and behavior of the presentation independently of the GUI controls used in the interface