Skip to content

Improve maintainability by making examination parameter defaults immutable#1547

Open
melihberkpir wants to merge 3 commits intoinformatici:developfrom
melihberkpir:improve-maintainability
Open

Improve maintainability by making examination parameter defaults immutable#1547
melihberkpir wants to merge 3 commits intoinformatici:developfrom
melihberkpir:improve-maintainability

Conversation

@melihberkpir
Copy link

The SonarQube analysis highlights multiple maintainability issues in core configuration classes such as ExaminationParameters, mainly caused by mutable default values and unclear responsibility boundaries. This change improves Maintainability by making default configuration values immutable, reducing accidental modification risks and improving code readability. As a result, future extensions and refactorings can be performed more safely with lower risk of regression.

/.env
.DS_Store
.DS_Store
.DS_Store
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this line duplicated 3 times?

Copy link
Author

Choose a reason for hiding this comment

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

I think I accidentally ran this 3 times in the terminal while pushing.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @melihberkpir, please remove two of them, thanks!


super(fileProperties);


Copy link
Collaborator

Choose a reason for hiding this comment

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

The only changes I see in this file are related to adding spaces. Why?

Copy link
Member

@mwithi mwithi Jan 15, 2026

Choose a reason for hiding this comment

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

there's a new object... @melihberkpir why did you apply only to weight property and not the others?

Comment on lines -137 to +150
WEIGHT_MIN = myGetProperty("WEIGHT_MIN", DEFAULT_WEIGHT_MIN);
/*WEIGHT_MIN = myGetProperty("WEIGHT_MIN", DEFAULT_WEIGHT_MIN);
WEIGHT_MAX = myGetProperty("WEIGHT_MAX", DEFAULT_WEIGHT_MAX);
WEIGHT_STEP = myGetProperty("WEIGHT_STEP", DEFAULT_WEIGHT_STEP);
WEIGHT_INIT = myGetProperty("WEIGHT_INIT", DEFAULT_WEIGHT_INIT);
WEIGHT_INIT = myGetProperty("WEIGHT_INIT", DEFAULT_WEIGHT_INIT);*/
Copy link
Member

Choose a reason for hiding this comment

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

Hi @melihberkpir, if you feel these fields should be removed, just remove them, there's no need to keep them commented ;)

Comment on lines +5 to +8
public final int min;
public final int max;
public final int init;
public final double step;
Copy link
Member

@mwithi mwithi Jan 15, 2026

Choose a reason for hiding this comment

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

I would go here with private fields and getters (only).
If we don't mind to abandon Java<14 we can use record type for this purpose, and also implement some validation checks in this class, wdyt?

@@ -0,0 +1,16 @@
package org.isf.generaldata;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing the license header too.

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.

3 participants