What is the correct way to handle the Textfield SetOnAction in a MVC pattern-CodePudding

I'm having an issue trying to get my TextField to connect with ActionEvent handle I define in my controller class. The error comes out to a java.lang.reflect.InvocationTargetException. What I have been trying to do is create an instance of my controller class in the view and then use a lamba method reference to call the handle method in the controller class.

View Class

package converter;

import javafx.scene.control.RadioButton;
import javafx.scene.control.TextField;
import javafx.scene.control.ToggleGroup;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;

public class View extends BorderPane{
    
    private Controller control = new Controller(new Model(), new View());
    
    //TextField
    private TextField input = new TextField();
    private TextField input2 = new TextField();

    //RadioButton
    private RadioButton distence = new RadioButton();
    private RadioButton tempeture = new RadioButton();
    private RadioButton weight = new RadioButton();
    //ToggleGroup
    private ToggleGroup group = new ToggleGroup();


    public String getConversion() 
    {
        return group.getSelectedToggle().getUserData().toString();
    }

    public double getInput() 
    {
        return Double.parseDouble(input.getText());
    }
    public double getInput2() 
    {
        return Double.parseDouble(input2.getText());
    }

    public void setInput(double value) 
    {
        input.setText(Double.toString(value));
    }

    public void setInput2(double value) 
    {
        input2.setText(Double.toString(value));
    }
    

    
    public View() 
    {
        
        System.out.println(control);
        //SetID
        //input.setPromptText("Input");
        //output.setPromptText("Output");
        
        input.setId("input");
        input2.setId("input2");

        //SetUserData
        distence.setUserData("dist");
        tempeture.setUserData("temp");
        weight.setUserData("weight");
        
        //Set Label
        distence.setText("Mile and Kilometer");
        tempeture.setText("Celsius and Fahrenheit");
        weight.setText("Pounds and Kilograms");

        //SetGroup
        distence.setToggleGroup(group);
        tempeture.setToggleGroup(group);
        weight.setToggleGroup(group);
        
        //Add TextField ActionEvent
        input.setOnAction(control::handle);
        input2.setOnAction(control::handle);
        
        //Add Group Listener
        group.selectedToggleProperty().addListener((ov, o , n) ->{
//          System.out.println(n.getUserData().toString());
            String tog = n.getUserData().toString();
            if (tog.equals("dist")) {
                
                input.setPromptText("Mile");
                input2.setPromptText("Kilometer");
                
            }else if(tog.equals("temp")) {
                input.setPromptText("Fahrenheit");
                input2.setPromptText("Celsius");
            }else if(tog.equals("weight")) {
                input.setPromptText("Pound");
                input2.setPromptText("Kilogram");
            }
        });
        
        StackPane left = new StackPane();
        StackPane right = new StackPane();

        VBox leftbox = new VBox(3);
        VBox rightbox = new VBox(2);

        leftbox.setSpacing(5);
        rightbox.setSpacing(10);

        leftbox.getChildren().addAll(distence, tempeture, weight);
        left.getChildren().add(leftbox);

        rightbox.getChildren().addAll(input, input2);
        right.getChildren().add(rightbox);

        this.setLeft(left);
        this.setRight(right);
        
    
    }

}

Controller Class

public class Controller implements EventHandler<ActionEvent>{
    private Model model;
    private View view;

    public Controller(Model model, View view) 
    {
        this.model = model;
        this.view = view;
    }
    
    public Controller() 
    {
        initalize();
    }
    
    public Controller initalize() 
    {
        this.model = new Model();
        this.view = new View();
        
        return this;
    }
    
    @Override
    public void handle(ActionEvent event) 
    {
        String id = ((javafx.scene.Node)event.getSource()).getId();
        String conversion = view.getConversion();

        switch (id) {
        case "input":

            if(conversion.equals("dist")) {
                view.setInput2(model.kilometer(view.getInput()));
                break;
            }else if(conversion.equals("temp")) {
                view.setInput2(model.cToF(view.getInput()));
                break;
            }else if(conversion.equals("weight")) {
                view.setInput2(model.kilogram(view.getInput()));
                break;
            }
        case "input2":

            if(conversion.equals("dist")) {
                view.setInput(model.mile(view.getInput2()));
                break;
            }else if(conversion.equals("temp")) {
                view.setInput(model.fToC(view.getInput2()));
                break;
            }else if(conversion.equals("weight")) {
                view.setInput(model.pound(view.getInput2()));
                break;
            }

        default:
            break;
        }
    }

}

Main class

package converter;

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.stage.Stage;

public class Main extends Application {
    private View view;
    
    @Override
    public void init() 
    {
        Model model = new Model();
        view = new View();
        new Controller(model, view);
    }

    @Override
    public void start(Stage primaryStage) {
        try {
            primaryStage.setMinWidth(350);
            primaryStage.setMinHeight(150);
            
            primaryStage.setScene(new Scene(view));
            primaryStage.show();
        } catch (Exception e) {
            e.printStackTrace();
        }
        
    }

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

Stack Trace

Exception in Application init method
java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:567)
    at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplicationWithArgs(LauncherImpl.java:465)
    at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication(LauncherImpl.java:364)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:567)
    at java.base/sun.launcher.LauncherHelper$FXHelper.main(LauncherHelper.java:1071)
Caused by: java.lang.RuntimeException: Exception in Application init method
    at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:896)
    at javafx.graphics/com.sun.javafx.application.LauncherImpl.lambda$launchApplication$2(LauncherImpl.java:196)
    at java.base/java.lang.Thread.run(Thread.java:831)
Caused by: java.lang.StackOverflowError
    at javafx.graphics/javafx.scene.Node.getScene(Node.java:1148)
    at javafx.graphics/javafx.scene.Node.updateCanReceiveFocus(Node.java:8502)
    at javafx.graphics/javafx.scene.Node.setTreeVisible(Node.java:8420)
    at javafx.graphics/javafx.scene.Node.updateTreeVisible(Node.java:8411)
    at javafx.graphics/javafx.scene.Node.<init>(Node.java:2596)
    at javafx.graphics/javafx.scene.Parent.<init>(Parent.java:1418)
    at javafx.graphics/javafx.scene.layout.Region.<init>(Region.java:627)
    at javafx.graphics/javafx.scene.layout.Pane.<init>(Pane.java:136)
    at javafx.graphics/javafx.scene.layout.BorderPane.<init>(BorderPane.java:219)
    at converter.View.<init>(View.java:52)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
    at converter.View.<init>(View.java:12)
Exception running application converter.Main

CodePudding user response：

Your exception occurs because you have infinite recursion on constructing a View instance:

public class View {

    // ...

    private Controller control = new Controller(new Model(), new View());

    // ...
}

When you create a View instance, you try to create a new View instance (to pass to the Controller constructor), which in turn will create a new View instance to pass to the Controller constructor, which, etc etc. Even if you iron out the dependencies here, you want these objects to have references to the same instance; you don't want to create new instances all over the place.

There are several different variations of MVC. It looks like you are trying to implement a "traditional" MVC in which:

The View observes the model, and updates if changes in the model occur
The View delegates user actions on the components it encapsulates (e.g. the text field) to the Controller
The Controller updates the Model

So:

The View should have a reference to both the Controller and the Model
The Controller should have a reference to the Model
The Model should know nothing about either the View or the Controller

I would also advise not making the Controller implement any EventHandler interfaces; just define the methods needed to process the user input. Define separate methods for each user action, instead of having one monolithic handle() method with endless switch or if-else statements.

So something like:

public class View { 

    private final Controller controller ;
    private final Model model ;

    private final TextField input ;
    private final TextField input2 ;

    // ...

    public View(Model model, Controller controller) {
        this.model = model ;
        this.controller = controller ;

        input = new TextField();
        input2 = new TextField();

        model.someProperty().addListener((obs, oldValue, newValue) -> {
            /* update controls */
        });

        model.someOtherProperty().addListener((obs, oldValue, newValue) -> {
            /* update controls */
        });

        input.setOnAction(event -> controller.handleInput(input.getText()));
        input2.setOnAction(event -> controller.handleInput2(input2.getText()));

        // layout etc

    }
}

public class Controller {

    public final Model model ;

    public Controller(Model model) {
         this.model = model ;
    }

    public void handleInput(String input) {
        model.setSomeValue(input); 
    }

    public void handleInput2(String input) {
        model.setSomeOtherValue(input2);
    }

    // etc
}

Then you assemble this with code like

Model model = new Model();
Controller controller = new Controller(model);
View view = new View(model, controller);

See Using JavaFX controller without FXML for a complete example.

CodePudding user response：

You are creating a circular dependency here. Controller depends on View depends on Controller.

This is bad design.

You should implement inversion of control and depend on an abstraction. The abstraction is either given to the view in its construction or there is a factory or some dependency injection framework in place. Another option is to let your View emit a view specific event that the controller subscribes to. There are many ways to solve this properly - circular dependency is not one of them.

The abstraction should be the smallest possible complete abstraction (interface segregation principle), so if you need a handler, you may have an interface with a handle(event) function and no more.

This doesn’t forbid the Controller to implement the handle function, it might violate the single responsibility principle, but that is to be decided at the problem and implementation of the controller.

See the SOLID principles here: Wikipedia SOLID

Update: You are almost there, just add setEventHandler as View property.

public void setEventHandler(@NotNull EventHandler handler) {
    this.handler = handler;
    updateHandler();
}

private void updateHandler() {
 //Add TextField ActionEvent
 input.setOnAction(handler::handle);
 input2.setOnAction(handler::handle);
}

and add it in your Main::init:

Controller controller = new Controller(model , view);
view.setEventHandler(controller);

You must also fix the default constructor of the controller.