I am developing an ERP system, called 'TeleMart' as a practice at Campus. I have noticed that one of my JavaFX Controller's have become quite complex and large, and most importantly, quite confusing. I have to write really large variable names for page elements, etc. Although I have a quite large code base, I want only this one file reviewed mainly, but review for other files would help as well. Link to the GitHub repository.com.hassanalthaf.telemart.viewmodels.DashboardViewModel.java/* * Program developed by Hassan Althaf. * Copyright  2015, Hassan Althaf. * Website: http://hassanalthaf.com */package com.hassanalthaf.telemart.viewmodels;import com.hassanalthaf.telemart.Main;import com.hassanalthaf.telemart.customers.Customer;import com.hassanalthaf.telemart.customers.CustomerController;import com.hassanalthaf.telemart.inventory.ProductController;import com.hassanalthaf.telemart.inventory.Product;import com.hassanalthaf.telemart.orders.Order;import com.hassanalthaf.telemart.orders.OrderController;import com.hassanalthaf.telemart.orders.OrderItem;import com.hassanalthaf.telemart.orders.OrderState;import com.hassanalthaf.telemart.users.UserRanks;import com.hassanalthaf.telemart.users.UserState;import java.io.IOException;import java.net.URL;import java.util.ResourceBundle;import javafx.collections.ObservableList;import javafx.event.ActionEvent;import javafx.fxml.FXML;import javafx.fxml.FXMLLoader;import javafx.fxml.Initializable;import javafx.scene.Parent;import javafx.scene.Scene;import javafx.scene.control.Button;import javafx.scene.control.CheckBox;import javafx.scene.control.Label;import javafx.scene.control.Menu;import javafx.scene.control.MenuItem;import javafx.scene.control.TableView;import javafx.scene.control.TextArea;import javafx.scene.control.TextField;import javafx.scene.input.MouseEvent;import javafx.scene.layout.AnchorPane;import javafx.stage.Stage;/** * FXML Controller class * * @author hassan */public class DashboardViewModel implements Initializable {    @FXML    private Parent dashboard;    @FXML    private AnchorPane home;    @FXML    private AnchorPane viewInventory;    @FXML    private AnchorPane addCustomer;    @FXML    private AnchorPane addInventory;    @FXML    private AnchorPane manageCustomers;    @FXML    private AnchorPane addOrder;    @FXML    private AnchorPane manageOrders;    @FXML    private TableView productTableView;    @FXML    private Menu userMenu;    @FXML    private TextField addCustomerNicNumber;    @FXML    private TextField addCustomerName;    @FXML    private TextField addCustomerContactNumber;    @FXML    private TextField addCustomerAddress;    @FXML    private TextField addCustomerEmail;    @FXML    private CheckBox addCustomerMembership;    @FXML    private Label addCustomerSuccess;    @FXML    private Label addCustomerErrors;    @FXML    private Label addInventorySuccess;    @FXML    private Label addInventoryErrors;    @FXML    private TextField addInventoryBrand;    @FXML    private TextField addInventoryModel;    @FXML    private TextField addInventoryColour;    @FXML    private TextField addInventoryUnitPrice;    @FXML    private TextField addInventoryAvailableQuantity;    @FXML    private TextArea addInventorySpecifications;    @FXML    private TableView customersTableView;    @FXML    private Label addOrdersSuccessBox;    @FXML    private Label addOrdersErrorsBox;    @FXML    private TextField addOrdersQuantity;    @FXML    private TableView addOrdersTableView;    @FXML    private Button addOrdersSelectedCustomer;    @FXML    private Label totalBillValue;    @FXML    private Label discount;    @FXML    private TableView manageOrdersTableView;    private CustomerController customerController;    private ProductController productController;    private OrderController orderController;    private AnchorPane currentPage;    private UserState userState;    private OrderState orderState;    private void changePage(AnchorPane page, int[] allowedRanks) {        boolean allowed = false;        for (int rank : allowedRanks) {            if (rank == this.userState.getUser().getRank()) {                allowed = true;            }        }        if (allowed) {            this.currentPage.setOpacity(0);            this.currentPage = page;            this.currentPage.toFront();            this.currentPage.setOpacity(1);        }    }    public void populateProductsTable() {        ObservableList<Product> products = this.productTableView.getItems();        products.clear();        products.addAll(this.productController.fetchAllProducts());    }    public void populateCustomersTable() {        ObservableList<Customer> customers = this.customersTableView.getItems();        customers.clear();        customers.addAll(this.customerController.fetchAllCustomers());    }    public void menuItemClick(ActionEvent event) {        Object source = (MenuItem)event.getSource();        MenuItem clickedItem;        if(source instanceof MenuItem) {            clickedItem = (MenuItem)source;        } else {            return;        }        String id = clickedItem.getId();        if (this.userState.getUser() != null) {            switch (id) {                case homeMenuItem:                    this.changePage(this.home, new int[]{UserRanks.CASHIER.getValue(), UserRanks.SALES_EXECUTIVE.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    break;                case viewInventoryMenuItem:                    this.changePage(this.viewInventory, new int[]{UserRanks.CASHIER.getValue(), UserRanks.SALES_EXECUTIVE.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    this.populateProductsTable();                    break;                case addCustomerMenuItem:                    this.changePage(this.addCustomer, new int[]{UserRanks.CASHIER.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    break;                case addInventoryMenuItem:                    this.changePage(this.addInventory, new int[]{UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    break;                case manageCustomersMenuItem:                    this.changePage(this.manageCustomers, new int[]{UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    this.populateCustomersTable();                    break;                case addOrderMenuItem:                    this.changePage(this.addOrder, new int[]{UserRanks.CASHIER.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    break;                case manageOrdersMenuItem:                    this.changePage(this.manageOrders, new int[]{UserRanks.SALES_EXECUTIVE.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    this.populateManageOrdersTable();                    break;                default:                    this.changePage(this.home, new int[]{UserRanks.CASHIER.getValue(), UserRanks.SALES_EXECUTIVE.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                    break;            }        }    }    @FXML    private void addCustomer(MouseEvent event) {        this.addCustomerErrors.setOpacity(0);        this.addCustomerSuccess.setOpacity(0);        String nicNumber = this.addCustomerNicNumber.getText();        String name = this.addCustomerName.getText();        int contactNumber;        try {            contactNumber = Integer.parseInt(this.addCustomerContactNumber.getText());        } catch (Exception exception) {            contactNumber = 0;        }        String address = this.addCustomerAddress.getText();        String email = this.addCustomerEmail.getText();        boolean membership = this.addCustomerMembership.isSelected();        try {            this.customerController.addNewCustomer(nicNumber, membership, name, contactNumber, address, email);            this.addCustomerSuccess.setText(Successfully stored customer details!);            this.addCustomerSuccess.setOpacity(1);        } catch (Exception exception) {            this.addCustomerErrors.setText(exception.getMessage());            this.addCustomerErrors.setOpacity(1);        }    }    @FXML    private void addInventory(MouseEvent event) {        this.addInventorySuccess.setOpacity(0);        this.addInventoryErrors.setOpacity(0);        String brand = this.addInventoryBrand.getText();        String model = this.addInventoryModel.getText();        String colour = this.addInventoryColour.getText();        double unitPrice;        try {            unitPrice = Double.parseDouble(this.addInventoryUnitPrice.getText());        } catch (Exception exception) {            unitPrice = 0;        }        int availableQuantity;        try {            availableQuantity = Integer.parseInt(this.addInventoryAvailableQuantity.getText());        } catch (Exception exception) {            availableQuantity = 0;        }        String specifications = this.addInventorySpecifications.getText();        try {            this.productController.addNewProduct(brand, model, colour, unitPrice, availableQuantity, specifications);            this.addInventorySuccess.setText(Successfully stored new inventory details!);            this.addInventorySuccess.setOpacity(1);        } catch (Exception exception) {            this.addInventoryErrors.setText(exception.getMessage());            this.addInventoryErrors.setOpacity(1);        }    }    @FXML    private void inventoryTableRefresh(MouseEvent event) {        this.populateProductsTable();    }    private Product getSelectedProduct() {        Product product = (Product)this.productTableView.getSelectionModel().getSelectedItem();        return product;    }    @FXML    private void inventoryTableViewDetails(MouseEvent event) throws Exception {        if (this.productTableView.getSelectionModel().getSelectedItem() != null) {            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/ViewInventoryDetailsView.fxml));            Parent viewInventoryDetails = fxmlLoader.load();            ViewInventoryDetailsViewModel viewInvetoryDetailsViewModel = fxmlLoader.getController();            viewInvetoryDetailsViewModel.show(this.getSelectedProduct());        }    }    @FXML    private void inventoryTableUpdate(MouseEvent event) throws Exception {        if (this.productTableView.getSelectionModel().getSelectedItem() != null) {            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/UpdateInventoryDetailsView.fxml));            Parent updateInventoryDetails = fxmlLoader.load();            UpdateInventoryDetailsViewModel updateInventoryDetailsViewModel = fxmlLoader.getController();            updateInventoryDetailsViewModel.show(this.getSelectedProduct(), this);        }    }    @FXML    private void inventoryTableDelete(MouseEvent event) {        if (this.productTableView.getSelectionModel().getSelectedItem() != null) {            int id = this.getSelectedProduct().getId();            this.productController.deleteProduct(id);            this.populateProductsTable();        }    }    private Customer getSelectedCustomer() {        Customer customer = (Customer)this.customersTableView.getSelectionModel().getSelectedItem();        return customer;    }    @FXML    private void customerTableRefresh(MouseEvent event) {        this.populateCustomersTable();    }    @FXML    private void customerTableUpdate(MouseEvent event) throws Exception {        if (this.customersTableView.getSelectionModel().getSelectedItem() != null) {            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/UpdateCustomerDetails.fxml));            Parent updateCustomerDetails = fxmlLoader.load();            UpdateCustomerDetailsViewModel updateCustomerDetailsViewModel = fxmlLoader.getController();            updateCustomerDetailsViewModel.show(this.getSelectedCustomer(), this.customerController, this);        }    }    @FXML    private void customerTableView(MouseEvent event) throws Exception {        if (this.customersTableView.getSelectionModel().getSelectedItem() != null) {            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/ViewCustomer.fxml));            Parent viewCustomer = fxmlLoader.load();            ViewCustomerViewModel viewCustomerViewModel = fxmlLoader.getController();            viewCustomerViewModel.show(this.getSelectedCustomer());        }    }    @FXML    private void customerTableDelete(MouseEvent event) {        if(this.customersTableView.getSelectionModel().getSelectedItem() != null) {            int id = this.getSelectedCustomer().getId();            this.customerController.deleteCustomer(id);            this.populateCustomersTable();        }    }    @FXML    private void addOrdersSelectProduct(MouseEvent event) throws IOException {        FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/SelectProduct.fxml));        Parent selectProduct = fxmlLoader.load();        SelectProductViewModel selectProductViewModel = fxmlLoader.getController();        selectProductViewModel.show(this, this.orderState.getSelectedProducts());    }    public void selectProduct(Product product) {        this.orderState.select(product);        this.addOrdersSuccess(Product successfully selected!);    }    private void addOrdersSuccess(String message) {        this.addOrdersErrorsBox.setOpacity(0);        this.addOrdersSuccessBox.setText(message);        this.addOrdersSuccessBox.setOpacity(1);    }    private void addOrdersError(String message) {        this.addOrdersSuccessBox.setOpacity(0);        this.addOrdersErrorsBox.setText(message);        this.addOrdersErrorsBox.setOpacity(1);    }    private OrderItem getSelectedOrderItem() {        return (OrderItem)this.addOrdersTableView.getSelectionModel().getSelectedItem();    }    private void refreshAddOrderItemsTable() {        ObservableList<OrderItem> orderItems = this.addOrdersTableView.getItems();        orderItems.clear();        orderItems.addAll(this.orderState.getOrderItems());    }    private void updateTotalBillValue(double value) {    }    @FXML    private void addOrderItem(MouseEvent event) {        boolean valid = true;        if (this.orderState.isProductSelected()) {            int quantity;            try {                quantity = Integer.parseInt(this.addOrdersQuantity.getText());            } catch (Exception exception) {                quantity = 0;                valid = false;            }            try {                this.orderState.setQuantity(quantity);            } catch (Exception exception) {                this.addOrdersError(exception.getMessage());                valid = false;            }            if (valid) {                OrderItem orderItem = this.orderState.getOrderItem();                orderItem.setUnitPrice(orderItem.getProduct().getUnitPrice());                this.orderState.setOrderItem(orderItem);                this.orderState.saveOrderItem();                this.refreshBillValues();                this.refreshAddOrderItemsTable();                this.addOrdersSuccess(Successfully added product!);                this.addOrdersQuantity.setText();            }        } else {            this.addOrdersError(Please select a product!);        }    }    @FXML    private void removeOrderItem(MouseEvent event) {        if (this.addOrdersTableView.getSelectionModel().getSelectedItem() != null) {            this.orderState.removeOrderItem(this.getSelectedOrderItem());            this.refreshBillValues();            this.refreshAddOrderItemsTable();            this.addOrdersSuccess(Successfully removed product!);        } else {            this.addOrdersError(Select an item to remove from table.);        }    }    public void setOrderCustomer(Customer customer) {        this.orderState.setCustomer(customer, false);        this.addOrdersSelectedCustomer.setText(customer.getNicNumber());        this.addOrdersSelectedCustomer.setDisable(false);        this.refreshBillValues();        this.addOrdersSuccess(Successfully selected customer!);    }    @FXML    private void addOrdersSelectCustomer(MouseEvent event) throws IOException {        FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/SelectCustomer.fxml));        Parent selectCustomer = fxmlLoader.load();        SelectCustomerViewModel selectCustomerViewModel = fxmlLoader.getController();        selectCustomerViewModel.show(this);    }    @FXML    private void addOrdersViewSelectedCustomer(MouseEvent event) throws IOException {        FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/ViewCustomer.fxml));        Parent viewCustomer = fxmlLoader.load();        ViewCustomerViewModel viewCustomerViewModel = fxmlLoader.getController();        viewCustomerViewModel.show(this.orderState.getSelectedCustomer());    }    private void resetSelectedButton() {        this.addOrdersSelectedCustomer.setDisable(true);        this.addOrdersSelectedCustomer.setText(None);    }    @FXML    private void addOrdersUnselectCustomer(MouseEvent event) {        this.orderState.setCustomer((Customer)null, true);        this.resetSelectedButton();        this.refreshBillValues();        this.addOrdersSuccess(Successfully un-selected customer!);    }    @FXML    private void addOrdersReset(MouseEvent event) {        this.orderState = new OrderState();        this.resetSelectedButton();        this.addOrdersQuantity.setText();        this.addOrdersTableView.getItems().clear();        this.addOrdersSuccess(Order has been successfully reset!);    }    @FXML    private void submitOrder(MouseEvent event) {        if (!this.orderState.isCustomerSelected()) {            this.addOrdersError(You need to select a customer!);            return;        }        try {            this.orderController.save(this.orderState.getOrder(), this.userState.getUser().getId());            this.addOrdersSuccess(Successfully created order!);        } catch (Exception exception) {            exception.printStackTrace();            this.addOrdersError(exception.getMessage());        }    }    public void refreshBillValues() {        this.totalBillValue.setText(String.format(%.2f, this.orderState.getBillValue()));        this.discount.setText(String.format(%.2f, this.orderState.getDiscountedAmount()));    }    private void populateManageOrdersTable() {        try {            ObservableList<Order> orders = this.manageOrdersTableView.getItems();            orders.clear();            orders.addAll(this.orderController.fetchAll());        } catch (Exception exception) {            exception.printStackTrace();        }    }    @FXML    private void manageOrderView(MouseEvent event) throws Exception {        if (this.manageOrdersTableView.getSelectionModel().getSelectedItem() != null) {            Order order = (Order)this.manageOrdersTableView.getSelectionModel().getSelectedItem();            FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource(/com/hassanalthaf/telemart/views/ViewOrder.fxml));            Parent viewOrder = fxmlLoader.load();            ViewOrderViewModel viewOrderViewModel = fxmlLoader.getController();            viewOrderViewModel.show(order);        }    }    @FXML    private void undoOrder(MouseEvent event) throws Exception {        if (this.manageOrdersTableView.getSelectionModel().getSelectedItem() != null) {            Order order = (Order)this.manageOrdersTableView.getSelectionModel().getSelectedItem();            order = this.orderController.fetch(order.getId());            this.orderController.delete(order);            this.populateManageOrdersTable();        }    }    public void show(Parent main, UserState userState) {        Scene scene = new Scene(this.dashboard);        Stage stage = new Stage();        stage.setScene(scene);        stage.setTitle(Main.APPLICATION_TITLE);        stage.setResizable(false);        this.userMenu.setText(userState.getUser().getUsername());        stage.show();        Stage mainStage = (Stage)main.getScene().getWindow();        mainStage.close();        this.userState = userState;        this.orderState = new OrderState();        this.customerController = new CustomerController();        this.productController = new ProductController();        this.orderController = new OrderController();    }    @Override    public void initialize(URL url, ResourceBundle rb) {        this.currentPage = this.home;    }}This is my first ever Java project which is of this size, therefore, review in all topics would be helpful, but my main focus is on:Making the above code more neat and maintainable.How I could improve not showing pages to un-authorised people than what I am doing currently.Any better way of doing what I am trying to do.Any bad practices.Any conceptual or conventional flaw in my code.Is my code having issues with being SOLID? How can I rectify that?
Maintaining neat JavaFX Controller files
java;javafx
TL;DR : move your application logic out of the UI component(s).Tactics    boolean allowed = false;    for (int rank : allowedRanks) {        if (rank == this.userState.getUser().getRank()) {            allowed = true;        }    }Don't type that out every time, think about what you are doing, and write a function to do it.  With a bit of thought, you should be able to see that this code is really  // Warning: pseudo code ahead boolean allowed = set(allowedRanks).contains(rank);When you get that far, you might then realize that int is the wrong way to track UserRank -- what you really want is an Enum. EnumSet<UserRank> allowedRanks = EnumSet.of(...); boolean allowed = allowedRanks.contains(user.getRank());A guideline: a function should perform coordination, calculation, or creation.  If you see more than one of those things happening together, that's a suggestion that you are doing too much in one place.BEFOREprivate void changePage(AnchorPane page, int[] allowedRanks) {    boolean allowed = false;    for (int rank : allowedRanks) {        if (rank == this.userState.getUser().getRank()) {            allowed = true;        }    }    if (allowed) {        this.currentPage.setOpacity(0);        this.currentPage = page;        this.currentPage.toFront();        this.currentPage.setOpacity(1);    }}AFTERprivate void changePage(...) {    // pure coordination, do the calculation elsewhere    if (allowedToView(userState.getUser, allowedRanks)) {        turnPage(currentPage, page);    }}StrategyOne area where you are heading toward trouble is the separation of your UI, and your logic.  An examplepublic void menuItemClick(ActionEvent event) {    Object source = (MenuItem)event.getSource();    MenuItem clickedItem;    if(source instanceof MenuItem) {        clickedItem = (MenuItem)source;    } else {        return;    }    String id = clickedItem.getId();    if (this.userState.getUser() != null) {        switch (id) {            case homeMenuItem:                this.changePage(this.home, new int[]{UserRanks.CASHIER.getValue(), UserRanks.SALES_EXECUTIVE.getValue(), UserRanks.MANAGER.getValue(), UserRanks.ADMINISTRATOR.getValue()});                break;    // ...ActionEvent, MenuItem, AnchorPane are all part of your UI framework.  User, UserRanks are part of your model.  You shouldn't be able to see that code in the same place.Thought experiment: imagine that you wanted to have a web version of this program.  Which parts would get rid of?  Well, you probably wouldn't need the JavaFX classes any longer, so all that can go.  You probably would need your Users, and UserRanks, and the rules that decide who can see bits of information.  That's the big hint that there are two pieces that should be separated -- the UI isn't the program, it's how the human operator talks to the program.Thought experiment: how would you write automated tests?  As a rule, trying to write good tests for the elements in the UI framework is a pain.  But making sure that page transitions work should be testable.  Answer: three components.  The outer component is the expression of your UI in the language of the framework you are using; you make it as thin as you possibly can.  The second component is the UI logic, written in an agnostic form.  All of the UI decision making is happening in the second component; the first component is just translation.Heuristic that you have it right -- the first component looks really trivial.These two separate components together make up your UI; when you are thinking about the hypothetical web version, they both go away.Second component talks to the third component, which is where all of the application rules live.  Second component and third component send messages back and for about what the user tried to do, and what to show.The sequence of messages might look like1 to 2: MouseEvent(TurnPageButton.clicked)2 to 3: PageTurnRequest(from:4 to:5)3 to 2: HidePage(4)2 to 1: page4.opacity = 03 to 2: ShowPage(5)2 to 1: page5.toFront2 to 1: page5.opacity = 1Things to checkThe code in component 1 is so simple it can't possibly be brokenAll of the constants that JavaFX uses (like OPACITY_HIDE=0, OPACITY_SHOW=1, and so on) live in component 2Component 2 has in memory every significant property of every object in the UI.  In other words, you should be able to look at component 2 in the debugger, and see the opacity of every single page.Your automated test framework can pretend to be component 1, trading message back and forth with component 2 to make sure that the UI works.  You shouldn't need to duplicate any code to make this happen.Your automated test framework can pretend to be component 2, trading message back and forth with component 3 to make sure the application behind the UI works.Other    try {        availableQuantity = Integer.parseInt(this.addInventoryAvailableQuantity.getText());    } catch (Exception exception) {        availableQuantity = 0;    }User input validation is its own thing; it belongs in the app, not in the UI.  There's often an anti corruption component that is responsible for taking unvalidated data, checking that it conforms to the rules governing its type, and then expressing the data as a immutable value type, so that the rest of the program knows that everything is correctly typed.ValidQuantity availableQuantity = ValidQuantity.parse(userInput);Either availableQuantity holds a valid quantity value (can quantity be negative?), or it throws an exception  (IllegalArgumentException is the usual choice).  Typically, in a desk top application where the user can fix the problem, you would send some sort of message to make a visual change (message box, highlight text field, beep, something like that) for the user to see.Doing something arbitrary when the user's intent is ambiguous is almost always a bad idea.  Doing so without logging the fact is perverse.  The log doesn't need to be shown to the user; in fact, you don't need to be able to persist it, but you should at least have something in memory you can look at when you are debugging and wondering what the heck just happened.