Issue
Consider this piece of code:
public void doSearch(ActionEvent event) {
String query = searchTextField.getText();
if (query.isEmpty()) {
data = FXCollections.observableArrayList(dc.getJobCoachRepo().getList());
usersTableView.setItems(data);
} else {
String searchOn = "search" + searchChoiceBox.getValue();
try {
Method m = this.getClass().getMethod(searchOn, String.class);
m.invoke(this, query);
} catch (Exception e) {
}
}
}
public void searchFirstName(String query) {
data = FXCollections.observableArrayList(dc.getJobCoachRepo().searchFirstName(query));
usersTableView.setItems(data);
}
...
...
I'm using java reflection here to avoid an if construct. The choicebox is used to let the user decide on what attribute he wants to search, there are 6 possibilities right now. I've gotten some comments from other students that using reflection is 'bad practice'. Is this the case? Why?
Solution
There are many reasons this is bad practice. Among them:
- It is not robust. The text that the user types in the text field has to match the name of a method, which implies a horrendous level of coupling between things that should not be related at all
- Reflection performs badly. Probably not a big deal here, but if there is no good reason to use reflection, you shouldn't.
- There is a ready-made better solution using lambda expressions.
Consider instead populating your combo box with Consumer<String>
objects:
ComboBox<Consumer<String>> searchChoiceBox = new ComboBox<>();
searchChoiceBox.getItems().add(createSearchOption(this::searchFirstName, "First Name"));
// ...
private Consumer<String> createSearchOption(Consumer<String> search, String name) {
return new Consumer<String>() {
@Override
public void accept(String s) {
search.accept(s);
}
@Override
public String toString() {
return name ;
}
};
}
Then you just do:
public void doSearch(ActionEvent event) {
String query = searchTextField.getText();
if (query.isEmpty()) {
data = FXCollections.observableArrayList(dc.getJobCoachRepo().getList());
usersTableView.setItems(data);
} else {
searchChoiceBox.getValue().accept(query);
}
}
Answered By - James_D
Answer Checked By - Senaida (JavaFixing Volunteer)