Skip to content

Classification UI enhancements#141

Merged
emays merged 4 commits intoikmdev:mainfrom
emays:feature/IKM-20241118-classification
Nov 22, 2024
Merged

Classification UI enhancements#141
emays merged 4 commits intoikmdev:mainfrom
emays:feature/IKM-20241118-classification

Conversation

@emays
Copy link
Contributor

@emays emays commented Nov 20, 2024

No description provided.

@emays emays requested a review from carldea November 20, 2024 18:23
@emays emays requested a review from rbradleyD November 20, 2024 18:41
private void elkOwlReasonerIncremental(ActionEvent actionEvent) {
reinferAllHierarchy = false;
private void runIncrementalReasoner() {
if (!confirmRun("incremental"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make literal Strings as constants? magic values: "incremental"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't know what magic values are

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant magic numbers. You might consider making an enum or constant string.

  enum ReasonerRunState {
      INCREMENTAL,
      FULL
  }
// or
 public static final String INCREMENTAL = "incremental";



...// later in code
if (!confirmRun(INCREMENTAL)) {
   // stuff
}

https://stackoverflow.com/questions/47882/what-are-magic-numbers-and-why-do-some-consider-them-bad

if (this.reasonerService == null) {
this.reasonerService = rs;
item.setSelected(true);
} else if (rs.getName().equals("ElkSnomedReasonerService")) {
Copy link
Contributor

@carldea carldea Nov 21, 2024

Choose a reason for hiding this comment

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

Can we make literal Strings as constants? magic values: "ElkSnomedReasonerService"?

Copy link
Contributor

@carldea carldea left a comment

Choose a reason for hiding this comment

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

@emays Looks good. but I do have some minor questions and feedback.

I will approve, but let me know if you would like to update the code as per suggestion.

@emays emays merged commit 4b37a08 into ikmdev:main Nov 22, 2024
@emays emays deleted the feature/IKM-20241118-classification branch November 22, 2024 15:12
swaroopsalvi added a commit that referenced this pull request Feb 12, 2025
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