2
\$\begingroup\$

I'm primarily concerned about the design of this MVC application. But will gladly take any suggestions!

Primary objective: Create an interface that allows a user to choose a document from a database.

This portion of my program allows a user (editor/writer) to choose document drafts from a database by title after specifying some selection criteria. The user will typically only be choosing one document at a time, and editing the document to fit their needs; but they have the option to grab multiple documents. The program will then concatenate all documents together and the user can edit as needed. (This extracted portion of the program only prints the titles to System.out as proof that the calling Swing portion can retrieve data from the JavaFX panel).

The rest of the application (this is a small part of a single screen) has been written with Swing and is closely coupled with the logic of the program. I've extracted this small panel as a start to decouple the view from the controller and model. JavaFX (via fxml) seemed like a nice way to guarantee that the view will remain separated from the controller.

Have I achieved my goal of logic separation? Am I missing anything? How could the design be improved for maintainability and usability?

SwingMain.java

package FXMLControllers;

import javafx.application.Platform;
import javafx.collections.ObservableList;
import javafx.embed.swing.JFXPanel;
import javafx.fxml.FXMLLoader;
import javafx.scene.Scene;

import javax.swing.*;
import java.awt.*;
import java.io.IOException;

public class SwingMain extends JFrame{

    private NewDraftFromDBPanelController controller;

    public SwingMain() {
    }

    private void initSwingComponents() {
        JFrame f = new JFrame("Swing with JavaFX test");
        f.setLayout(new BorderLayout());
        JFXPanel draftFromDBPanel = new JFXPanel();
        JPanel buttonPanel = new JPanel();
        JButton showDocs = new JButton("Show Docs");
        buttonPanel.add(showDocs);
        f.add(draftFromDBPanel, BorderLayout.CENTER);
        f.add(buttonPanel, BorderLayout.SOUTH);
        f.setSize(1000,350);
        f.setVisible(true);
        f.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);

        Platform.runLater(() -> initFX(draftFromDBPanel));
        showDocs.addActionListener(e -> printAllDocs());
    }

    private void printAllDocs() {
        ObservableList<DraftPK> draftPKS = controller.retrieveAllDocs();
        for (DraftPK draft: draftPKS) {
            print(draft.getDraftName());
        }
    }

    private void initFX(JFXPanel draftFromDBPanel) {
        Scene scene = createScene();
        draftFromDBPanel.setScene(scene);
        DraftFromDBPanelModel model = new MockNewDraftFromDBPanelModel();
        controller.initData(model);
    }

    private Scene createScene() {
        Scene scene = null;
        try {
            FXMLLoader loader = new FXMLLoader(getClass().getResource("DraftFromDBPanel.fxml"));
            scene = new Scene(loader.load(), 200, 100);
            controller = loader.getController();
        } catch (IOException e) {
            e.printStackTrace();
        }
        return scene;
    }

    private void print(String message){
        System.out.print(message + "\n");
    }

    public static void main(String[] args) throws IOException {
        SwingMain test = new SwingMain();
        SwingUtilities.invokeLater(test::initSwingComponents);
    }
}

NewDraftFromDBPanelController.java

package FXMLControllers;

import javafx.beans.property.ReadOnlyStringWrapper;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.*;
import javafx.util.StringConverter;

public class NewDraftFromDBPanelController {

    @FXML
    private TableColumn<DraftPK, String> draftNameColumn;

    @FXML
    private TableColumn<DraftPK, String> sessionColumn;

    @FXML
    private TableColumn<DraftPK, String> authorColumn;

    @FXML
    private TableColumn<DraftPK, String> draftTypeColumn;

    @FXML
    private ComboBox<String> drafterChooser;

    @FXML
    private TableView<DraftPK> selectedDocumentsTable;

    private ObservableList<DraftPK> selectedDocuments;

    @FXML
    private Button removeFromTable;

    @FXML
    private ComboBox<DraftPK> documentChooser;

    @FXML
    private ToggleGroup dbButtonGroup;

    @FXML
    private RadioButton othersDrafts;

    @FXML
    private RadioButton finalDraft;

    @FXML
    private CheckBox changeDraft;

    @FXML
    private CheckBox notChangeDraft;

    @FXML
    private CheckBox historyCheckbox;

    @FXML
    private ComboBox<YearEntryDO> yearChooser;

    private YearEntryDO currentYear;
    private DraftFromDBPanelModel model;

    @FXML
    void initialize() {
        assert selectedDocumentsTable != null : "fx:id=\"selectedDocumentsTable\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert removeFromTable != null : "fx:id=\"removeFromTable\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert documentChooser != null : "fx:id=\"documentChooser\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert dbButtonGroup != null : "fx:id=\"dbButtonGroup\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert othersDrafts != null : "fx:id=\"othersDrafts\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert finalDraft != null : "fx:id=\"finalDraft\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert changeDraft != null : "fx:id=\"changeDraft\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert notChangeDraft != null : "fx:id=\"notChangeDraft\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert historyCheckbox != null : "fx:id=\"historyCheckbox\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert yearChooser != null : "fx:id=\"yearChooser\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        assert drafterChooser != null : "fx:id=\"drafterChooser\" was not injected: check your FXML file 'DraftFromDBPanel.fxml'.";
        bindComponents();
        initializeYearChooser();
        initializeDraftersChooser();
        initializeDocumentChooser();
        initializeSelectedDraftsTable();
    }

    private void bindComponents() {
        drafterChooser.visibleProperty().bind(othersDrafts.selectedProperty());
        drafterChooser.managedProperty().bind(drafterChooser.visibleProperty());
        changeDraft.visibleProperty().bind(historyCheckbox.selectedProperty().not().and(finalDraft.selectedProperty()));
        changeDraft.managedProperty().bind(changeDraft.visibleProperty());
        notChangeDraft.visibleProperty().bind(historyCheckbox.selectedProperty().not().and(finalDraft.selectedProperty()));
        notChangeDraft.managedProperty().bind(notChangeDraft.visibleProperty());
        yearChooser.visibleProperty().bind(historyCheckbox.selectedProperty());
        yearChooser.managedProperty().bind(yearChooser.visibleProperty());
    }

    private void initializeYearChooser() {
        yearChooser.setCellFactory(combobox -> createYearListCellFactory());
        yearChooser.setConverter(createSessionYearConverter());
    }

    private ListCell<YearEntryDO> createYearListCellFactory() {
        return new ListCell<YearEntryDO>() {
            @Override
            protected void updateItem(YearEntryDO item, boolean empty) {
                super.updateItem(item, empty);

                if (item == null || empty) {
                    setText(null);
                } else {
                    setText(item.getYearNum() + " " + item.getYearPart());
                }
            }
        };
    }

    private StringConverter<YearEntryDO> createSessionYearConverter() {
        return new StringConverter<YearEntryDO>() {
            @Override
            public String toString(YearEntryDO session) {
                if (session == null) {
                    return null;
                } else {
                    return session.getYearNum() + " " + session.getYearPart();
                }
            }

            @Override
            public YearEntryDO fromString(String string) {
                return null;
            }
        };
    }

    private void initializeDraftersChooser() {
        drafterChooser.setCellFactory(combobox -> createDraftersChooserCellFactory());
        drafterChooser.setConverter(createDraftersChooserConverter());
    }

    private ListCell<String> createDraftersChooserCellFactory() {
        return new ListCell<String>() {
            @Override
            protected void updateItem(String item, boolean empty) {
                super.updateItem(item, empty);

                if (item == null || empty) {
                    setText(null);
                } else {
                    setText(item);
                }
            }
        };
    }

    private StringConverter<String> createDraftersChooserConverter() {
        return new StringConverter<String>() {
            @Override
            public String toString(String initials) {
                if (initials == null) {
                    return null;
                } else {
                    return initials;
                }
            }

            @Override
            public String fromString(String string) {
                return null;
            }
        };
    }

    private void initializeDocumentChooser() {
        documentChooser.setCellFactory(draftChooser -> createDocumentChooserCellFactory());
        documentChooser.setConverter(createDocumentChooserConverter());
    }

    private ListCell<DraftPK> createDocumentChooserCellFactory() {
        return new ListCell<DraftPK>() {
            @Override
            protected void updateItem(DraftPK item, boolean empty) {
                super.updateItem(item, empty);

                if (item == null || empty) {
                    setText(null);
                } else {
                    setText(item.getDraftName());
                }
            }
        };
    }

    private StringConverter<DraftPK> createDocumentChooserConverter() {
        return new StringConverter<DraftPK>() {
            @Override
            public String toString(DraftPK draft) {
                if (draft == null) {
                    return null;
                } else {
                    return draft.getDraftName();
                }
            }

            @Override
            public DraftPK fromString(String string) {
                return null;
            }
        };
    }

    void initData(DraftFromDBPanelModel model) {
        this.model = model;
        this.currentYear = model.getCurrentYear();
        initializeDocumentChooserItems();
    }

    private void initializeDocumentChooserItems() {
        documentChooser.setItems(getDrafts());
    }

    private ObservableList<DraftPK> getDrafts() {
        return model.getDrafts(getSelectedYear(), getCurrentUser());
    }

    private YearEntryDO getSelectedYear() {
        YearEntryDO year;
        if (historyCheckbox.isSelected()) {
            year = yearChooser.getSelectionModel().getSelectedItem();
        } else {
            year = getCurrentYear();
        }
        return year;
    }

    private YearEntryDO getCurrentYear() {
        return currentYear;
    }

    private String getCurrentUser() {
        String user;
        if (othersDrafts.isSelected()){
            user = getSelectedUser();
        } else {
            user = model.getCurrentUserInitials();
        }
        return user;
    }

    private String getSelectedUser() {
        return drafterChooser.getSelectionModel().getSelectedItem();
    }

    private void initializeSelectedDraftsTable() {
        selectedDocumentsTable.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
        selectedDocuments = FXCollections.observableArrayList();
        draftNameColumn.setCellValueFactory(cellData -> new ReadOnlyStringWrapper(cellData.getValue().getDraftName()));
        authorColumn.setCellValueFactory(cellData -> new ReadOnlyStringWrapper(cellData.getValue().getDrftrInitls()));
        draftTypeColumn.setCellValueFactory(cellData -> new ReadOnlyStringWrapper(Integer.toString(cellData.getValue().getDrafttypeId())));
        sessionColumn.setCellValueFactory(cellData -> new ReadOnlyStringWrapper(Integer.toString(cellData.getValue().getSessYr())));

        draftNameColumn.setCellFactory(column -> new TableCell<DraftPK, String>() {
            @Override
            protected void updateItem(String item, boolean empty) {
                super.updateItem(item, empty);

                if (item == null || empty) {
                    setText(null);
                    setStyle("");
                } else {
                    setText(item);
                }
            }
        });
        selectedDocumentsTable.setItems(selectedDocuments);
    }


    //Begin Action Listeners

    @FXML
    private void setYearChooser(ActionEvent event) {
        yearChooser.setItems(model.getYears());
        event.consume();
    }

    @FXML
    private void handleYearSelection(ActionEvent actionEvent) {
        if (finalDraft.isSelected()){
            updateDraftListWithFinalDrafts(actionEvent);
        } else {
            updateDraftList(actionEvent);
        }
    }

    @FXML
    private void updateDraftListWithFinalDrafts(ActionEvent actionEvent) {
        setDraftSelectionOptions(model.getFinalDrafts(getSelectedYear()));
        actionEvent.consume();
    }

    @FXML
    private void updateDraftList(ActionEvent event) {
        ObservableList<DraftPK> drafts = getDrafts();
        setDraftSelectionOptions(drafts);
        event.consume();
    }

    private void setDraftSelectionOptions(ObservableList<DraftPK> drafts) {
        ObservableList<DraftPK> items = documentChooser.getItems();
        items.clear();
        items.setAll(drafts);
    }

    @FXML
    private void setDrafterChooser() {
        drafterChooser.setItems(model.getDrafters());
    }

    @FXML
    private void toggleChangeDraft(ActionEvent event) {
        changeDraft.setSelected(!notChangeDraft.isSelected());
        event.consume();
    }

    @FXML
    private void toggleNotChangeDraft(ActionEvent event) {
        notChangeDraft.setSelected(!changeDraft.isSelected());
        event.consume();
    }

    @FXML
    private void addSelectedDoc(ActionEvent event) {
        if (documentChooser.getValue() != null) {
            DraftPK selectedDoc = getSelectedDocumentFromComboBox();
            this.selectedDocuments.add(selectedDoc);
            event.consume();
        }
    }

    private DraftPK getSelectedDocumentFromComboBox() {
        return documentChooser.getSelectionModel().getSelectedItem();
    }

    @FXML
    private void removeSelectedDocFromTable(ActionEvent event) {
        ObservableList<DraftPK> selectedDrafts = getSelectedDraftsFromTable();
        selectedDocuments.removeAll(selectedDrafts);
        event.consume();
    }

    /**
     * Retrieves drafts that are currently selected by the user in the table. Intended for use in the removal of the
     * selected drafts. (As opposed to retrieving all of the documents in the table)
     *
     * @return ObservableList of DraftPKs
     */
    private ObservableList<DraftPK> getSelectedDraftsFromTable() {
        return selectedDocumentsTable.getSelectionModel().getSelectedItems();
    }

    public ObservableList<DraftPK> retrieveAllDocs() {
        return selectedDocuments;
    }
}

DraftFromDBPanel.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import java.net.URL?>
<?import javafx.geometry.Insets?>
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.RadioButton?>
<?import javafx.scene.control.TableColumn?>
<?import javafx.scene.control.TableView?>
<?import javafx.scene.control.ToggleGroup?>
<?import javafx.scene.layout.ColumnConstraints?>
<?import javafx.scene.layout.GridPane?>
<?import javafx.scene.layout.RowConstraints?>
<?import javafx.scene.layout.VBox?>

<GridPane maxHeight="-Infinity" maxWidth="-Infinity" prefHeight="233.0" prefWidth="912.0" xmlns="http://javafx.com/javafx/8.0.111" xmlns:fx="http://javafx.com/fxml/1" fx:controller="FXMLControllers.NewDraftFromDBPanelController">
    <columnConstraints>
        <ColumnConstraints halignment="CENTER" maxWidth="220.0" minWidth="34.0" prefWidth="177.0" />
        <ColumnConstraints maxWidth="287.0" minWidth="34.0" prefWidth="207.0" />
        <ColumnConstraints hgrow="SOMETIMES" maxWidth="336.0" minWidth="10.0" prefWidth="184.0" />
        <ColumnConstraints hgrow="SOMETIMES" maxWidth="525.0" minWidth="10.0" prefWidth="427.0" />
    </columnConstraints>
    <rowConstraints>
        <RowConstraints maxHeight="292.0" minHeight="10.0" prefHeight="252.0" vgrow="SOMETIMES" />
    </rowConstraints>
    <children>
        <VBox alignment="CENTER" prefHeight="226.0" prefWidth="384.0" GridPane.columnIndex="3">
            <children>
                <TableView fx:id="selectedDocumentsTable" prefHeight="173.0" prefWidth="283.0">
                    <columns>
                        <TableColumn fx:id="draftNameColumn" text="Draft Name" />
                        <TableColumn fx:id="sessionColumn" text="Session" />
                        <TableColumn fx:id="authorColumn" prefWidth="75.0" text="Author" />
                        <TableColumn fx:id="draftTypeColumn" prefWidth="75.0" text="Draft Type" />
                    </columns>
                    <columnResizePolicy>
                        <TableView fx:constant="CONSTRAINED_RESIZE_POLICY" />
                    </columnResizePolicy>
                </TableView>
                <Button fx:id="removeFromTable" alignment="CENTER" mnemonicParsing="false" onAction="#removeSelectedDocFromTable" text="Remove" textAlignment="CENTER" />
            </children>
        </VBox>
        <ComboBox id="documentChooser" fx:id="documentChooser" onAction="#addSelectedDoc" prefHeight="25.0" prefWidth="155.0" promptText="Select a document..." GridPane.columnIndex="2" GridPane.halignment="CENTER" />
        <VBox alignment="CENTER_LEFT" prefHeight="213.0" prefWidth="163.0" GridPane.columnIndex="1" GridPane.halignment="CENTER" GridPane.valignment="BOTTOM">
            <children>
                <Label text="From Database:" />
                <RadioButton id="ownDrafts" mnemonicParsing="false" onAction="#updateDraftList" selected="true" text="Own Drafts">
                    <toggleGroup>
                        <ToggleGroup fx:id="dbButtonGroup" />
                    </toggleGroup>
                </RadioButton>
                <RadioButton id="othersDrafts" fx:id="othersDrafts" mnemonicParsing="false" onAction="#setDrafterChooser" text="Other's Drafts" toggleGroup="$dbButtonGroup" />
                <ComboBox id="drafterChooser" fx:id="drafterChooser" onAction="#updateDraftList" prefWidth="150.0" promptText="Choose a Drafter..." />
                <RadioButton id="finalDrafts" fx:id="finalDraft" mnemonicParsing="false" onAction="#updateDraftListWithFinalDrafts" text="Final Drafts" toggleGroup="$dbButtonGroup" />
                <CheckBox id="changeDraft" fx:id="changeDraft" mnemonicParsing="false" onAction="#toggleNotChangeDraft" styleClass="warningMessage" text="This is a Change Draft" />
                <CheckBox id="notChangeDraft" fx:id="notChangeDraft" mnemonicParsing="false" onAction="#toggleChangeDraft" styleClass="warningMessage" text="This is NOT a Change Draft" />
            </children>
        </VBox>
        <VBox alignment="CENTER_LEFT" prefHeight="213.0" prefWidth="156.0" GridPane.valignment="CENTER">
            <children>
                <CheckBox id="historyCheckbox" fx:id="historyCheckbox" mnemonicParsing="false" onAction="#setYearChooser" text="History Year" />
                <ComboBox id="yearChooser" fx:id="yearChooser" onAction="#handleYearSelection" prefHeight="34.0" prefWidth="132.0" promptText="Choose a year..." />
            </children>
            <padding>
                <Insets top="2.0" />
            </padding>
        </VBox>
    </children>
    <stylesheets>
        <URL value="@DraftFromDbPanel.css" />
    </stylesheets>
</GridPane>

DraftFromDbPanel.css

.root {
    -fx-base: rgb(237,237,229);
    -fx-background-color: rgb(237,237,229);
    -fx-text-fill: black;
}

.check-box {
    -fx-padding: 2px;
    -fx-border-insets: 2px;
    -fx-background-insets: 2px;
}

.combo-box {
    -fx-padding: 2px;
}

.radio-button {
    -fx-padding: 2px;
    -fx-border-insets: 2px;
    -fx-background-insets: 2px;
}

.warningMessage {
    -fx-background-color: red;
    -fx-border-color: black;
    -fx-border-insets: 5px;
    -fx-background-insets: 5px;
    -fx-padding: 5px;
}

DraftFromDBPanelModel.java

package FXMLControllers;

import javafx.collections.ObservableList;

public interface DraftFromDBPanelModel {
    ObservableList<YearEntryDO> getYears();
    ObservableList<String> getDrafters();
    YearEntryDO getCurrentYear();
    String getCurrentUserInitials();

    ObservableList<DraftPK> getDrafts(YearEntryDO currentYear, String currentUserInitials);

    ObservableList<DraftPK> getFinalDrafts(YearEntryDO year);
}

MockNewDraftFromDBPanelModel.java

package FXMLControllers;

import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

public class MockNewDraftFromDBPanelModel implements DraftFromDBPanelModel {
    @Override
    public ObservableList<YearEntryDO> getYears() {
        ObservableList<YearEntryDO> sessions = FXCollections.observableArrayList();
        sessions.add(new YearEntryDO("2017", "1st"));
        sessions.add(new YearEntryDO("2016", "2nd"));
        sessions.add(new YearEntryDO("2015", "1st"));
        sessions.add(new YearEntryDO("2014", "2nd"));
        sessions.add(new YearEntryDO("2013", "1st"));
        sessions.add(new YearEntryDO("2012", "2nd"));
        sessions.add(new YearEntryDO("2011", "1st"));
        sessions.add(new YearEntryDO("2010", "2nd"));
        sessions.add(new YearEntryDO("2009", "1st"));
        sessions.add(new YearEntryDO("2008", "2nd"));
        return sessions;
    }

    @Override
    public ObservableList<String> getDrafters() {
        ObservableList<String> drafters = FXCollections.observableArrayList();
        drafters.add("ABC");
        drafters.add("DEF");
        drafters.add("GHI");
        drafters.add("JKL");
        drafters.add("MNO");
        drafters.add("PQR");
        drafters.add("STU");
        drafters.add("VWX");
        drafters.add("YNZ");
        return drafters;
    }

    @Override
    public YearEntryDO getCurrentYear() {
        return new YearEntryDO("2018", "2nd");
    }

    @Override
    public String getCurrentUserInitials() {
        return "PBJ";
    }

    @Override
    public ObservableList<DraftPK> getDrafts(YearEntryDO year, String userInitials) {
        ObservableList<DraftPK> drafts = FXCollections.observableArrayList();
        drafts.add(new DraftPK(0, "Draft0", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(1, "Draft1", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(2, "Draft2", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(3, "Draft3", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(4, "Draft4", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(5, "Draft5", Integer.parseInt(year.getYearNum()), userInitials));
        drafts.add(new DraftPK(6, "Draft6", Integer.parseInt(year.getYearNum()), userInitials));
        return drafts;
    }

    @Override
    public ObservableList<DraftPK> getFinalDrafts(YearEntryDO year) {
        ObservableList<DraftPK> finalDrafts = FXCollections.observableArrayList();
        finalDrafts.add(new DraftPK(0, "FinalDraft0", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(1, "FinalDraft1", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(2, "FinalDraft2", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(3, "FinalDraft3", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(4, "FinalDraft4", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(5, "FinalDraft5", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(6, "FinalDraft6", Integer.parseInt(year.getYearNum())));
        return finalDrafts;
    }
}

DraftPK.java

package FXMLControllers;

public class DraftPK {

    private String draftName;
    private String drafterInitials;
    private int draftTypeID;
    private int sessYear;

    public DraftPK(int draftTypeID, String draftName, int sessYear, String userInitials) {
        this.draftTypeID = draftTypeID;
        this.drafterInitials = userInitials;
        this.draftName = draftName;
        this.sessYear = sessYear;
    }

    public DraftPK(int draftTypeID, String draftName, int sessYear) {
        this.draftTypeID = draftTypeID;
        this.draftName = draftName;
        this.sessYear = sessYear;
    }

    public String getDraftName() {
        return draftName;
    }

    public String getDrftrInitls() {
        return drafterInitials;
    }

    public int getDrafttypeId() {
        return draftTypeID;
    }

    public int getSessYr() {
        return sessYear;
    }
}

YearEntryDO.java

package FXMLControllers;

public class YearEntryDO {

    private String yearNum;
    private String yearPart;

    YearEntryDO(String yearNum, String yearPart){
       this.yearNum = yearNum;
       this.yearPart = yearPart;
    }
    public String getYearNum() {
        return yearNum;
    }

    public String getYearPart() {
        return yearPart;
    }

}

Additional Notes

  • The GUI design has been handed down to me and the users and management are particularly adverse to change, but I'll gladly accept recommendations for an improved UX.

  • The documents are stored as XML in the database.

  • Users should be able to choose documents from any combination of criteria: other users, different years, draft types, first draft/final draft, etc.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

BE DRY

In your MockNewDraftFromDBPanelModel class you have a lot of code repetition and near duplication. Cut this out by using loops.

e.g. for getFinalDrafts

 @Override
    public ObservableList<DraftPK> getFinalDrafts(YearEntryDO year) {
        ObservableList<DraftPK> finalDrafts = FXCollections.observableArrayList();
        finalDrafts.add(new DraftPK(0, "FinalDraft0", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(1, "FinalDraft1", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(2, "FinalDraft2", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(3, "FinalDraft3", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(4, "FinalDraft4", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(5, "FinalDraft5", Integer.parseInt(year.getYearNum())));
        finalDrafts.add(new DraftPK(6, "FinalDraft6", Integer.parseInt(year.getYearNum())));
        return finalDrafts;
    }

May be reduced to

@Override
    public ObservableList<DraftPK> getFinalDrafts(YearEntryDO year) {
        ObservableList<DraftPK> finalDrafts = FXCollections.observableArrayList();
        for (int i = 0; i <= 6; i++) {
            finalDrafts.add(new DraftPK(i, "FinalDraft" + i, Integer.parseInt(year.getYearNum())));
        }
        return finalDrafts;
    }

The same obviously applies togetDrafts as well as getYears

Follow naming conventions

On the topic of those methods. Typically get methods, much like properties in other languages like C#, simply return an already set value. Seeing get methods where work is being done is unexpected, I would suggest renaming these to createFinalDrafts or even acquireFinalDrafts if you want a dictionary equivalent to 'get.'

On that same token, getYearNum returning a string isn't intuitive. In fact, it's surprising that you elected that to begin with since as far as I can see everywhere you use it you're using it as an integer and have to keep calling Integer.parseInt

Control your controller

Your controller class' NewDraftFromDBPanelController has an initializer filled with asserts. As you may know, asserts are not reliable and you're actually better of not having this code at all. JavaFX would fail to start and throw an error if any of the elements could not inflate from the FXML.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.