2

I'm trying to follow MVC for a test project, so my model should be completely independant from my view, however I'm not sure how I should update an Observable List which gets updated in a background thread (It's being given Strings about uploading files through FTP) so that the messages appear on the UI in a ListView.

I am using JavaFX and trying to get my program as loosely coupled as possible. At this current moment, the GUI in the view package is depending on the fact that my model updates my list using Platform.runLater(...) - which to my knowledge, my model should work completely independent from the view, and shouldn't have to conform to the View's needs.

Now the following code actually "works as intended" it's just not modelled correctly, and I'm not sure how I can model it correctly. Some initial research brought up that I might have to use Observer and observable - and have another class in the middle to act as my observable list - but I'm not sure how I would set this up.

So I have an Observable list which is updated on a background thread:

private ObservableList<String> transferMessages;

public FTPUtil(String host, int port, String user, String pass) {
    this.host = host;
    this.port = port;
    this.username = user;
    this.password = pass;       

    transferMessages = FXCollections.observableArrayList();

    connect();
}

public void upload(File src) {
    System.out.println("Uploading: " + src.getName());
    try {
        if (src.isDirectory()) {            
            ftpClient.makeDirectory(src.getName());
            ftpClient.changeWorkingDirectory(src.getName());
            for (File file : src.listFiles()) {
                upload(file);
            }
            ftpClient.changeToParentDirectory();
        } else {
            InputStream srcStream = null;
            try {
                addMessage("Uploading: " + src.getName());
                srcStream = src.toURI().toURL().openStream();
                ftpClient.storeFile(src.getName(), srcStream);
                addMessage("Uploaded: " + src.getName() + " - Successfully.");

            } catch (Exception ex) {
                System.out.println(ex);
                addMessage("Error Uploading: " + src.getName() + " - Speak to Administrator.");
            }
        }
    } catch (IOException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

}

private void addMessage(String message){

    Platform.runLater(() -> transferMessages.add(0, message));

}

The FTPUtil class is my model.

I also have a Model Manager class which is what controls this FTPUtil class:

public class ModelManager {

private ObservableList<String> fileAndFolderLocations;


private FTPUtil ftpUtil;

public ModelManager(String host, int port, String user, String pass) {

    ftpUtil = new FTPUtil(host, port, user, pass);
    fileAndFolderLocations = FXCollections.observableArrayList();

}

public boolean startBackup() {

    Task task = new Task() {
        @Override
        protected Object call() throws Exception {

            System.out.println("I started");
            ftpUtil.clearMessages();

            for(String location : fileAndFolderLocations){
                File localDirPath = new File(location);         
                ftpUtil.upload(localDirPath);
            }               
            return null;
        }           
    };      
    new Thread(task).start();

    return true;
}

public void addFileOrFolder(String fileOrFolder){
    if(!fileAndFolderLocations.contains(fileOrFolder)){
        fileAndFolderLocations.add(fileOrFolder);
    }       
}

public boolean removeFileOrFolder(String fileOrFolder){
    return fileAndFolderLocations.remove(fileOrFolder);
}

public ObservableList<String> getFilesAndFoldersList() {
    return fileAndFolderLocations;
}

public ObservableList<String> getMessages() {
    return ftpUtil.getMessages();
}

}

Finally is my GUI:

public class BackupController {

private Main main;
private ModelManager mm;

@FXML
private ListView<String> messagesList;

@FXML
void forceBackup(ActionEvent event) {
    mm.startBackup();           

}

public void initController(Main main, ModelManager mm) {
    this.main = main;
    this.mm = mm;

    messagesList.setItems(mm.getMessages());
}


}
APK
  • 47
  • 2
  • 8
  • add an isolation layer: keep the model ignorant of the view (that is do not use runLater), keep a separate list for showing the items, listen to changes of the original messages and propagate them to the separate list on the fx thread (that is use runLater here) – kleopatra Oct 08 '17 at 10:33
  • @kleopatra Hey, as by isolation layer, do you mean a class which simply holds another observable list, or extends an observable list -> which is then observes my model (which should be observable) for when its list changes? – APK Oct 08 '17 at 11:36
  • same place that you do it now, BackgroundController looks fine for the task – kleopatra Oct 08 '17 at 11:54

2 Answers2

4

The basic setup:

  • do not use Platform.runLater in the model
  • do not set the model/manager's list of messages directly as items to the listView
  • do keep a separate observableList of items and set that to the list
  • install a listener on the manager's list that keeps the items in sync with the messages: wrap those modifications in Platform.runLater

A very raw snippet to illustrate the setup:

private Parent getContent() {
    ModelManager manager = new ModelManager();
    ObservableList<String> uploading = FXCollections.observableArrayList("one", "two", "three");

    ObservableList<String> items = FXCollections.observableArrayList();
    manager.getMessages().addListener((ListChangeListener) c -> {

        while (c.next()) {
            if (c.wasAdded()) {
                Platform.runLater(() ->  
                    items.addAll(c.getFrom(), c.getAddedSubList()));
            } 
            if (c.wasRemoved()) {
                Platform.runLater(() ->
                     items.removeAll(c.getRemoved()));
            }
        }
    });


    ListView<String> list = new ListView<>(items);
    Button button = new Button("start");
    button.setOnAction(ev -> {
        uploading.stream().forEach(e -> manager.addFile(e));
        manager.startBackup();
    });
    BorderPane pane = new BorderPane(list);
    pane.setBottom(button);
    return pane;
}

@Override
public void start(Stage stage) throws Exception {
    Scene scene = new Scene(getContent());
    stage.setScene(scene);
    stage.show();
}
kleopatra
  • 51,061
  • 28
  • 99
  • 211
  • Great, this works like a charm :D I still have one question: the list that my model contains is currently an ObservableList - should it remain this way or turn into an ArrayList. As the view is no longer depending on the model to update the UI, should I be using the JavaFX ObservableList in the model? If I were to change it to ArrayList, I wouldn't be able to add a listener would I? – APK Oct 08 '17 at 13:19
  • 1
    exactly ... somewhere along the chain has to be something observable – kleopatra Oct 08 '17 at 13:27
  • Great. Thank you for helping me :D I tried to +1 your reply, but I dont have enough rep. – APK Oct 08 '17 at 13:39
1

You need a wrapper around the list that posts changes on the correct thread.

ObservableList<String> source = FXCollections.observableArrayList();
ObservableList<String> items = new UiThreadList(source);
ListView<String> list = new ListView<>(items);

with

import javafx.application.Platform;
import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;
import javafx.collections.transformation.TransformationList;

class UiThreadList<T> extends TransformationList<T, T> {
    public UiThreadList(ObservableList<? extends T> source) {
        super(source);
    }

    @Override
    protected void sourceChanged(ListChangeListener.Change<? extends T> change) {
        Platform.runLater(() -> fireChange(change));
    }

    @Override
    public int getSourceIndex(int index) {
        return index;
    }

    @Override
    public T get(int index) {
        return getSource().get(index);
    }

    @Override
    public int size() {
        return getSource().size();
    }
}

The idea of this solution is similar to the one of @kleopatra above.

Tobias Diez
  • 251
  • 3
  • 10