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

Pull request dans le cadre d'un cours de génie logiciel #313

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

Conversation

Utorh
Copy link

@Utorh Utorh commented Apr 3, 2021

Pull Request

This PR fixes some code smells and minor issues

Changes proposed in this pull request:

  • Some explicit private constructors to overwrite implicit public ones created automatically by java
  • Constants for strings that are repeated more than twice
  • Some comments to justify empty methods
  • Refactor of the connect() method in CoreHandler.java
  • Merging ifs that don't need to be apart
  • Deleted unused commented out code lines
  • Proposition to create LaunchErrorException to replace Exception in Launcher.java
  • Renaming "name" to "keyName" in PersistableAttributeStore.serialize() to avoid confusion with PersistableAttributeStore.name
  • Putting duplicated code from InboundHandshake.java into an auxiliary function to avoid duplication
  • changing from replaceAll method to replace since not using regex in LoggerConstectFilter.init() for optimisation purposes

Copy link
Member

@mondain mondain left a comment

Choose a reason for hiding this comment

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

Approving based on cursory review; it looks good.

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.

2 participants