Skip to content

Change the setter methods to void type#198

Merged
Kouroshkt merged 1 commit into
mainfrom
feature1/config/Configuration.java
Feb 7, 2024
Merged

Change the setter methods to void type#198
Kouroshkt merged 1 commit into
mainfrom
feature1/config/Configuration.java

Conversation

@Kouroshkt

Copy link
Copy Markdown
Contributor

I changed the Setter's methods to void type in Configuration.java.

@github-advanced-security

Copy link
Copy Markdown

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@Mattan41 Mattan41 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you explain to me why the change of design pattern?

@Kouroshkt

Copy link
Copy Markdown
Contributor Author

They had return value and Setter methods have no return they make an opration.

@Mattan41 Mattan41 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

void version is best practice for Setters.

@yrlacornelia yrlacornelia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks better to me, good job!

@Kouroshkt Kouroshkt merged commit 72ea050 into main Feb 7, 2024
@Kouroshkt Kouroshkt deleted the feature1/config/Configuration.java branch February 7, 2024 10:06
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.

4 participants