4

I'm new to JavaFX and I was wondering what are the best practices in this language to develop a MVC database application, I think my question will be pretty simple if you are an senior developer.

Let us consider a simple example of a basic application developed in JavaFX : a ToDoList linked with a SQL Database.

  • The database is just one table Task with an id and a taskDescr VARCHAR field.
  • The purpose is pretty easy : we just want to display the task in a TableView or ListView and be able to add some tasks.

That's what our application looks like :

ToDoList GUI

I decided to split my code into four parts, DAO for the classes who represents datas in the table (Task.java), the DAO class who access the database (its behavior does not matter here). The model who represents the Model part of our TodoList (containing a list of task and performing operations on it, calling the DAO, etc..). The FXML Views and the Controller :

Project structure

Next, you can find the code of the different classes that interest us (We supposed that the DAO is OK (setting id automatically) an we do not handle error cases to simplify code :

Task.java

public class Task {

    private int id;
    private SimpleStringProperty task;

    public Task(int i, String s){
        this.id = i;
        this.task = new SimpleStringProperty(s);
    }

    public void setId(int i){
        this.id = i;
    }

    public int getId() {
        return id;
    }

    public String getTask() {
        return task.get();
    }

    public void setTask(String task) {
        this.task.set(task);
    }

    @Override
    public boolean equals(Object o){
        if(this.id == ((Task)o).id)
            return true;
        return false;
    }
}

ToDoListModel.java

public class ToDoListModel {

    private List<Task> taskList;
    private DAO dao;

    public ToDoListModel(){
        this.taskList = new ArrayList<Task>();
        this.dao = new DAO();
    }

    public void loadDatabase(){
        this.taskList = this.dao.getAllTasks();
    }

    public void addTask(Task t){
        // Operations throwing Exceptions such as : Does the task t is already in the list, etc...
        this.taskList.add(t);
        this.dao.createTask(t);
    }

    public void deleteTask(Task t){
        this.taskList.remove(t);
        this.dao.deleteTask(t);
    }

    public List<Task> getTaskList() {
        return taskList;
    }
}

Controller.java

public class Controller {

    private final ToDoListModel model;

    @FXML
    private TableView<Task> taskTable;
    @FXML
    private TableColumn<Task, String> taskColumn;
    @FXML
    private TextField taskTextField;

    public Controller(ToDoListModel m){
        this.model = m;
    }

    @FXML
    protected void initialize() {
        this.model.loadDatabase();

        // Setting up data table
        taskColumn.setCellValueFactory(new PropertyValueFactory<Task, String>("task"));
        ObservableList<Task> taskObservableList = FXCollections.observableList(this.model.getTaskList());
        taskTable.setItems(taskObservableList);
    }

    @FXML
    public void handleAddButton(ActionEvent e) {
        Task t = new Task(-1, this.taskTextField.getText());

        // What operations to do here ?
        this.model.addTask(t);
        this.taskTable.getItems().add(t);
        this.taskTable.refresh();
    }

}

Main.java

public class Main extends Application {

    @Override
    public void start(Stage primaryStage) throws Exception{
        ToDoListModel model = new ToDoListModel();
        primaryStage.setTitle("My Todo");
        FXMLLoader loader = new FXMLLoader();
        loader.setLocation(getClass().getResource("views/View.fxml"));
        loader.setController(new Controller(model));
        Parent root = loader.load();
        primaryStage.setScene(new Scene(root));
        primaryStage.show();
    }

    public static void main(String[] args) {
        launch(args);
    }
}

Finally, my question is : Is my approach good ? I mean the fact that I've created a ToDoListModel with a list of task, the fact that I update my list of Objects Task at the same task I update my database with the DAO (a create in the DAO will be performed after an add in the list) and the most important : what operations should I do in the handleAddButton of my Controller ? Here I used first the add method in my TodoListModel but it's not enough because my observable list is wrongly updated (The added task appears but we can not select it with the mouse). Then, when I add it also in the TableView items, the Task appears twice and has been added twice in the list.

As a result I've understood that the ObservableList was linked to the List I have in my ToDoListModel but what am I supposed to do if I want to do operations on that list only in my model but getting the ObservableList updated correctly ? (Selectable item etc...)

Duplication example

Thank you in advance for your help and your patience, Sincerely, Paul

  • 1
    There is no logic for error handling. It is not good to work with the database in the UI thread. `ToDoListModel` works well with `ObservableList` and does not need to keep its instance (she is sitting in controls). – mr mcwolf Jan 16 '18 at 09:42
  • `handleAddButton` can use `ToDoListModel` to immediately save the element in the database, but this is useful when small changes are made. When you make more edits, it's a good idea to make a separate save button that just allows one request to the database (`upsert`) to save the entire list and one `delete` to delete. – mr mcwolf Jan 16 '18 at 09:49
  • Hi, Yep as I said i didn't code the error handling just to simplify the code posted here. Okay so I have to replace my List in my TaskListModel by an ObservableList and just doing operations on it ? I think I didn't understand your second post :/ – Paul Thirozier Jan 16 '18 at 10:14

1 Answers1

3

Here is an example implementation
The DAO class takes care of connecting to the database (may use a pool or something else). In this case, it makes a simple connection.

public class DAO {
    public Connection getConnection() throws SQLException {
        return  DriverManager.getConnection("jdbc:mysql://192.168.40.5:3306/test", "root", "");
    }
}

The ToDoListModel class takes care of working with the database by using an instance of DAO to get a valid connection.

public class ToDoListModel {
    private DAO dao;

    public static ToDoListModel getInstance() {
        ToDoListModel model = new ToDoListModel();
        model.dao = new DAO();

        return model;
    }

    private ToDoListModel() {
    }

    public void addTask(Task task) throws SQLException {
        try(Connection connection = dao.getConnection()) {
            String q = "insert into todo (name) values (?)";

            try(PreparedStatement statement = connection.prepareStatement(q, Statement.RETURN_GENERATED_KEYS)) {
                statement.setString(1, task.getName());
                statement.executeUpdate();

                try(ResultSet rs = statement.getGeneratedKeys()) {
                    if(rs.next()) {
                        task.setId(rs.getInt(1));
                    }
                }
            }
        }
    }

    public void deleteTask(Task task) throws SQLException {
        try(Connection connection = dao.getConnection()) {
            String q = "delete from todo where id = ?";

            try(PreparedStatement statement = connection.prepareStatement(q)) {
                statement.setInt(1, task.getId());
                statement.executeUpdate();
            }
        }
    }

    public ObservableList<Task> getTaskList() throws SQLException {
        try(Connection connection = dao.getConnection()) {
            String q = "select * from todo";

            try(Statement statement = connection.createStatement()) {
                try(ResultSet rs = statement.executeQuery(q)) {
                    ObservableList<Task> tasks = FXCollections.observableArrayList();

                    while (rs.next()) {
                        Task task = new Task();
                        task.setId(rs.getInt("id"));
                        task.setName(rs.getString("name"));

                        tasks.add(task);
                    }

                    return tasks;
                }
            }
        }
    }
}

The controller uses ToDoListModel to initialize TableView controls and add operations (editing and reading - I did not implement them because I stick to your code)

public class Controller {

    @FXML
    private TextField textField;

    @FXML
    private TableView<Task> tableView;

    @FXML
    private TableColumn<Task, String> nameTableColumn;

    @FXML
    private Button addButton;

    @FXML
    private void initialize() {
        nameTableColumn.setCellValueFactory(cdf -> cdf.getValue().nameProperty());

        addButton.disableProperty().bind(Bindings.isEmpty(textField.textProperty()));

        CompletableFuture.supplyAsync(this::loadAll)
            .thenAccept(list -> Platform.runLater(() -> tableView.getItems().setAll(list)))
            .exceptionally(this::errorHandle);
    }

    @FXML
    private void handleAddButton(ActionEvent event) {
        CompletableFuture.supplyAsync(this::addTask)
                .thenAccept(task -> Platform.runLater(() -> {
                    tableView.getItems().add(task);

                    textField.clear();
                    textField.requestFocus();
                }))
                .exceptionally(this::errorHandle);
    }

    private Task addTask() {
        try {
            Task task = new Task(textField.getText());
            ToDoListModel.getInstance().addTask(task);

            return task;
        }
        catch (SQLException e) {
            throw new RuntimeException(e);
        }
    }

    private ObservableList<Task> loadAll() {
        try {
            return ToDoListModel.getInstance().getTaskList();
        }
        catch (SQLException e) {
            throw new RuntimeException(e);
        }
    }

    private Void errorHandle(Throwable throwable) {
        throwable.printStackTrace();
        return null;
    }
}

Any database operations are asynchronous with CompletableFuture but you can use whatever you prefer. The important thing is to remember that UI threads can only be made uniquely by it.

mr mcwolf
  • 2,574
  • 2
  • 14
  • 27
  • Ok I see what you meant, So as I understood here, the classic "Model-View-Controller" pattern can not be implemented in JavaFX because the model (that usually carry all the data and interacts with the the class DAO which interacts with the database) is represented in JavaFX by the controllers itself ? I thought that in heavy application manipulating a lot of datas (database datas and no-database datas) we were using a Model that manage all that thinks and gives it to the Controller. "UI threads can only be made uniquely by it." -> I didn't get this point :/ ? – Paul Thirozier Jan 16 '18 at 11:05
  • Oh ok i got it, so you mean that my `this.model.addTask(t)` changed the UI ? Did I correctly understand the MVC principle in my precedent comment ? What about heavy applications that have to manage other data in their Model ? – Paul Thirozier Jan 16 '18 at 12:07
  • @PaulThirozier, Yes, all changes to the data you work with (the model) are reflected in the user interface. Therefore, these changes should only be done in the Application thread. In your example there is no asynchronous data loading, so there is nothing to worry about (except that the user interface will freeze). This is the approach for larger projects. The model here is `Task`, if you are used to the `Swing` architecture, this may confuse you (there is a need to make a model to keep the data collection) – mr mcwolf Jan 16 '18 at 13:31
  • Oh yes I think that i am a bit confused with these differents approach. So, as I understand, if I build a heavy JavaFX application, I should consider your approach with asynchronous data loading (I understand you code, thank you man) and if I need to handle other datas that are not in a database, I create another class for each type of datas. However, implementing a "ModelContainer" for all the datas is not a good practice ? – Paul Thirozier Jan 16 '18 at 14:07
  • @PaulThirozier The need to implement TableModel in Swing is due to the JTable architecture. Without this model it is difficult to work with swing tables (at all, swing has a similar architecture). At FX you do not have to take care of the controls so much. Just give them the data you want them to work with. – mr mcwolf Jan 16 '18 at 14:45
  • The presence of "ModelContainer" does not sound reasonable. When you work with different data, you make different models for them. The models themselves are in no way tied to the controls in which you use them (as opposed to TableModel, ListModel, ComboBoxModel ...) or the repositories in which you can serialize them (database, file, web api ...). You can also replace individual models without affecting others. So the use of a common model is by no means correct. – mr mcwolf Jan 16 '18 at 14:45
  • Hmm. Your controller should not have data access code in it; this is the responsibility of the DAO. See http://www.oracle.com/technetwork/java/dataaccessobject-138824.html. (It is a *data* access object, not a *connection* access object.) I would argue that this isn't really MVC either, as you don't have an independent UI model. (The class you have called a model looks more like a service; i.e. it is not part of the presentation tier at all.) I agree that last point is debatable and that is a viable approach (although there's no reason to use an `ObservableList` in this case). – James_D Jan 16 '18 at 17:08
  • @James_D, the controller does not include a database access code (SQLException processing only to possibly display an error message dialog). Regarding the ObstelList, this is really superfluous, but I use it to be able to submit directly to TableView # setItems (if necessary). – mr mcwolf Jan 16 '18 at 17:38
  • Yeah, sorry: I meant the *model* should not have database access code. For the observable list, though; that's an issue. If you allow to pass the list directly to `table.setItems(...)` then your model *must* be single threaded (it must only update that list on the FX Application Thread, since changes will modify the UI. At that point the model becomes a genuine UI model, not a service. – James_D Jan 16 '18 at 17:42
  • 1
    Here's the problem. The driving motivation behind MVC-type architectures is to make it possible to have multiple views of the same data (model). If the data are modified from one view, other views should be able to update (and of course this is done without the model having any dependency on the view(s)). Your design doesn't allow for this: imagine, e.g. a `ListView` of the same data (perhaps in another tab). If both the table and list can modify the data, there is no mechanism for communication in your design, as the only place the table's data is accessible is in the controller. – James_D Jan 16 '18 at 17:52
  • @James_D for that I agree. if several controls need to be updated with the same data, it is appropriate to use a common ObservationList to make part of ToDoListModel – mr mcwolf Jan 16 '18 at 18:03
  • OK, so follow that thought process. Now that observable list (in the model) can only be updated on the UI thread (because it would be bound to the UI controls). So your threading code (currently in the controller) needs to move to the model... So I think a lot of the responsibilities here are in the wrong place. (Or at least, there are a lot of possible changes you might want to the application that would result in a major refactoring being necessary.) – James_D Jan 16 '18 at 18:06
  • @James_D I do not agree with that. for me, the model is not working to thread. if the controller requests the data asynchronously, it is advisable to take care to process the response in the UI thread. The problem I'm seeing is the error handling (messaging) they should also be done in the UI thread, asking for access to a Window Object (Dialog # initOwner), and this binds the model to the view. – mr mcwolf Jan 16 '18 at 18:21
  • My point is that if you change the model so that it shares an observable list with multiple controllers, which directly use that list as the backing data for their UI controls, then you *must not* change that list (i.e. *must not* change the model) on a background thread. So the *model* must take the responsibility of running the asynchronous code, and only performing updates to its data on the UI thread. Your current controller code, in that scenario, would modify the list via the call to `CompletableFuture.supplyAsync(...)`, resulting in a "Not on FX Application Thread" exception. – James_D Jan 16 '18 at 18:41
  • I agree with you on the exception handling, etc. – James_D Jan 16 '18 at 18:42
  • @James_D yes, if the ObstableList moves to ToDoListModel, synchronization must also be changed. – mr mcwolf Jan 16 '18 at 18:57
  • In DAO logic, which side is responsible for limiting string length to fit in the size of the varchar column? – Guangliang Apr 01 '21 at 16:19
  • @Guangliang it's part of the validation of user input. in the case of `mysql`, this is implemented by the database layer. – mr mcwolf Apr 02 '21 at 04:37