2017-02-23 40 views
0

考虑这段代码:是Java反思不好的做法?

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); 

} 
... 
... 

我使用Java反射这里如果结构来避免。选择框用于让用户决定他想要搜索的属性,现在有6种可能性。我从其他学生那里得到一些评论,认为使用反思是'坏习惯'。是这样吗?为什么?

+5

这是非常糟糕的,因为这样会使得你的用户界面,以你的方法名,这应该是完全无关。稍后做出看似无害的改变会产生意想不到的灾难性后果。 – shmosel

+2

使用反射并不是一个坏习惯。 *做这种事*是一种不好的做法。当你想通过ID在几个相似的对象中选择一个更好的方法是从字符串到策略对象的映射。 – chrylis

+0

@shmosel这并不一定是他们绑在一起,而是他们被捆绑得如此不透明。 – chrylis

回答

2

这是不好的做法有很多原因。其中:

  1. 它不健壮。用户在文本字段中键入的文本必须与方法的名称匹配,这意味着应该完全不相关的事物之间存在可怕的级联。
  2. 反射性能很差。这里可能不是什么大不了的事,但如果没有充分的理由来使用反思,你就不应该这样做。
  3. 有一个使用lambda表达式的现成更好的解决方案。

请考虑改用填充组合框与Consumer<String>对象:

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 ; 
     } 
    }; 
} 

然后你只需要做:

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); 

    } 
} 
+0

优秀的答案,谢谢! – wimdetr

0

是的,反射速度很慢,并以这种方式使用时会产生脆弱的代码。

如果你想避免if语句,你应该使用多态。使用public void search(String query)创建一个接口Searcher,为每个要执行的搜索类型创建实现,然后将每个实现的实例作为Map<String, Searcher>的值,并以搜索选择框的值作为关键字。

由于Java枚举是对象,因此您也可以使用枚举作为映射。每个枚举值将定义它们自己的​​实现。然后你可以调用你想要的实现SearchEnumTypeName.valueOf(searchChoiceBox.getValue()).search(query)