Refactor code to avoid errors in CheckStyle task #38
Conversation
The code within the imagelab, sound folder and Run.java file has been refactored to follow the convention as indicated by the CheckStyle gradle task. This commit fixes the errors causing CheckStyle not reporting any error The most interesting errors were: * Use of magic numbers. https://stackoverflow.com/questions/47882/what-is-a-magic-number-and-wh y-is-it-bad * Hidden field. https://stackoverflow.com/questions/7776046/checkstyle-how-to-resolve-h idden-field-error * Must match pattern https://stackoverflow.com/questions/13146771/java-code-conventions-must -match-pattern-a-za-za-z0-9 * Utility classes should not have a public or default constructor https://stackoverflow.com/questions/14398747/hide-utility-class-constru ctor-utility-classes-should-not-have-a-public-or-def Resolves: MetroCS#32
The comment of the private constructor has been improved deleting unnecesary information
There was a problem hiding this comment.
A lot of changes in one pass. This needs a dedicated reviewer before the project can do its internal review. Please have at least one of your teammates add a comment requesting to be a reviewer.
A very quick scan looks like most of the changes are right on target. However, some of the changes might be inappropriate; that is, they may alleviate a complaint from Checkstyle but may not improve the code or may even have a negative impact on project maintenance.
|
I performed all the changes by myself. I've reviewed the project and it under the account MSU Denver CS. So I guess that by one of my teammates you mean a colleague from the University, Am I right? I'm afraid I'm not a member of the MSU Denver CS so I cannot ask a teammate to review the changes Please let me know if there is something I can do to help you review the impact of the changes to the project |
|
I apologize, I thought you might have a closer connection to others working on the project! This project is relatively new as open source and is sadly lacking information for contributors, noticeably any "How to Contribute", "Contribution Workflow", or "Development Conventions". The current protocol is for potential contributors to "request" to claim an issue (by making a comment to the issue) which then results in potential assignment to that issue. In this instance, User Story #32 is an "Epic", so there are component stories (#33, #34, #35) and each is already assigned. Another concern is actually due to a weakness associated with User Story #32. In particular, the Assessment Criteria over-simplifies the intent to simply making errors not be reported. It needs clarification of the intent of such code modifications. For example, the use of "TODO:" is desired and part of the coding convention. (Checkstyle is being used to ensure that any embedded TODO comments are well identified and reported.) Thus, "fixing" those Checkstyle errors requires changes to the code implementation, not just changing "TODO:" to some other string (e.g., "TO DO:"), which just hides the shortcoming without improving the product. I apologize for the lack of documentation even at this early stage of the project!! |
|
I appreciate your understanding! Let's see how thing go over the next week. The issues currently in progress are being worked on by undergraduate students relatively new to software development. The semester ends in a couple of weeks, so either those issues will be resolved or they will go back into the "To Do" column of the project board. |
Hi!
I refactored the code as requested in #32 to ensure it adheres to conventions set by CheckStyle.
Let me know if you want me to make more changes
Best regards,
Alex