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

OP-1199 Small improvements #2107

Open
wants to merge 11 commits into
base: OP-1199-Main-Store-Inventory
Choose a base branch
from

Conversation

mwithi
Copy link
Member

@mwithi mwithi commented Dec 20, 2024

Small improvements for OP-1199.

@mwithi mwithi changed the base branch from develop to OP-1199-Main-Store-Inventory December 20, 2024 00:42
@mwithi mwithi requested a review from dbmalkovsky December 20, 2024 09:36
jTableInventory.getColumnModel().getColumn(i).setMinWidth(pColumwidth[i]);
}
}

private JComboBox<String> getComboBox() {
if (statusComboBox == null) {
statusComboBox = new JComboBox<String>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
statusComboBox = new JComboBox<String>();
statusComboBox = new JComboBox<>();

Replace explicit type object with <>

});
pagesCombo.addItemListener(new ItemListener() {
pagesComboBox.addItemListener(new ItemListener() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

anonymous type can be replaced by a lambda (starting with JDK 8)

} else {
jTableInventory.getColumnModel().getColumn(i).setCellRenderer(new ColorTableCellRenderer());
}
}
jTableInventory.getSelectionModel().addListSelectionListener(new ListSelectionListener() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

anonymous type can be replaced by a lambda (starting with JDK 8)

}

ofPagesLabel.setText("/" + totalPages + " " + MessageBundle.getMessage("angal.common.pages.txt"));
Copy link
Collaborator

@dbmalkovsky dbmalkovsky Jan 9, 2025

Choose a reason for hiding this comment

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

first single character strings can just use single quotes

second why not use a formatted message that will move the need for two concatenations

Something like:

angal.common.pages.fmt.txt                                                                                 = /{0} Pages

startIndex = 0;
int page = startIndex / PAGE_SIZE + 1;
Copy link
Collaborator

@dbmalkovsky dbmalkovsky Jan 9, 2025

Choose a reason for hiding this comment

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

The result of startIndex / PAGE_SIZE is always zero (startIndex is 0). So the result is always 1.

private boolean[] columnCentered = { false, false, false, true, true, true, true, true, true, true };
private boolean[] columnDecimalNumber = { false, false, false, false, false, false, false, false, true, true };
private Class< ? >[] columnsClasses = { String.class, Integer.class, String.class, String.class, String.class, LocalDate.class, Integer.class,
Integer.class, BigDecimal.class, BigDecimal.class };
private MedicalInventory inventory = null;
Copy link
Collaborator

@dbmalkovsky dbmalkovsky Jan 9, 2025

Choose a reason for hiding this comment

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

Suggested change
private MedicalInventory inventory = null;
private MedicalInventory inventory;

Initialization to null is redundant

There are at least 7 other occurrences.

@dbmalkovsky
Copy link
Collaborator

Generally it looks OK but the code (especially the original code) has many code implementation issues. I only marked those that I could see in the diff from this PR.

Remind me when this gets merged to go back and really go other the code constructs and newer language level options.

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