Combines functions of "central GUI class" and "central data processing class". This contributes to the bloat, and makes the non-gui bits much harder to test efficiently (also due to automatic window showing).
Instantiation is messy: too much happening in the constructor, several overridable method calls in constructor, and showing the main window should not happen in constructor. Best would probably be to move to a private constructor / static factory method arrangement.
Sketch of new structure
Launching, command-line parsing etc. factored out into Main as of [2018-10-22 Mon], but that was a much smaller job than separating the GUI will be. After restructuring I envisage initialization as something like:
private final PuffinData puffinData;
private SwingGui(PuffinData puffinData) { this.puffinData = puffinData; // ... setup ... }
public void showGui() { // ... }
public static SwingGui create(PuffinData puffinData) { SwingGui gui = new SwingGui(puffinData); // any post-instantiation setup happens here return gui; }
}
public class Main {
public static void main(String[] argv) { // ... final PuffinData data = PuffinData.create(); final SwingGui gui = SwingGui.create(data); gui.showGui();
} }
A lot of PuffinApp methods are on the pattern "do something to data, call updateDisplay()". No problem splitting these out into a PuffinData class: just implement a "data changed" listener, have SwingGui register for it, and replace all the updateDisplay()s with fireDataChangedListener()s or whatever.
The best plan is probably to keep PuffinApp as the main class and gradually move things into PuffinData, adding unit tests for each moved method as I go. I could start by simply moving suites into PuffinData and creating a getter for it.
PuffinData is a pretty rubbish name, but I haven't come up with anything better yet. It's not just a data container: it holds various bits of state, like the current suite, sample, and orientation correction. "Workspace" might fit, but it sounds a bit too much like a GUI element. How about "Controller"? I think that's the best so far – certainly better than "PuffinData", which is just as bad as the late unlamented "Datum". However "Controller" might imply a model-view-controller architecture, which doesn't really correspond to the way PuffinPlot is organized. "Director" or "DataManager" maybe? "Notebook" is tempting, but doesn't really fit.
Challenges
PuffinData wraps suites and does top-level data operations. Both of these should be usable entirely headless. Anything requiring a "head" goes in SwingGui (name OK? I think so.).
It's going to be a lot of work. Lots of tangles to untangle. Probably a fair bit of boilerplate too in cases where the GUI calls straight through to the data model. Something like doThingAcion -> SwingGui.doThing -> PuffinData.doThing.
One major problem is testing. I'd want a lot of unit tests in place before starting on a refactoring like this, but unit testing Swing GUI code is hard. Even if I can get decent tests in place, they will slow down the test suite massively. Classic legacy code chicken-and-egg situation here: I want (ideally) everything under test before refactoring, but very hard to get tests in place before the code's been refactored…
Can I do this gradually? What's the minimum amount of code I can move into a PuffinData class without having to refactor big chunks of the codebase? I could start by simply making it a wrapper around the current PuffinApp.suites, perhaps. PuffinApp.getSuites() can just call through. Shouldn't break anything.
Where's PuffinApp used? As of [2018-10-22 Mon], not at all in data or plots. Lots in window and puffinplot of course but that's expected. In short, splitting it doesn't look too painful wrt effects on the rest of the codebase.
Next steps
There may be some more small bits I can pull out of PuffinApp before the big split. Build properties for a start – they're not a part of the GUI or the data model so probably belong in their own small class. The downloady bit of runPythonScriptInGui should be moved to JythonJarManager.
Initial problem statement
Various problems, some of them overlapping:
Sketch of new structure
Launching, command-line parsing etc. factored out into Main as of [2018-10-22 Mon], but that was a much smaller job than separating the GUI will be. After restructuring I envisage initialization as something like:
A lot of PuffinApp methods are on the pattern "do something to data, call updateDisplay()". No problem splitting these out into a PuffinData class: just implement a "data changed" listener, have SwingGui register for it, and replace all the updateDisplay()s with fireDataChangedListener()s or whatever.
The best plan is probably to keep PuffinApp as the main class and gradually move things into PuffinData, adding unit tests for each moved method as I go. I could start by simply moving suites into PuffinData and creating a getter for it.
PuffinData is a pretty rubbish name, but I haven't come up with anything better yet. It's not just a data container: it holds various bits of state, like the current suite, sample, and orientation correction. "Workspace" might fit, but it sounds a bit too much like a GUI element. How about "Controller"? I think that's the best so far – certainly better than "PuffinData", which is just as bad as the late unlamented "Datum". However "Controller" might imply a model-view-controller architecture, which doesn't really correspond to the way PuffinPlot is organized. "Director" or "DataManager" maybe? "Notebook" is tempting, but doesn't really fit.
Challenges
PuffinData wraps suites and does top-level data operations. Both of these should be usable entirely headless. Anything requiring a "head" goes in SwingGui (name OK? I think so.).
It's going to be a lot of work. Lots of tangles to untangle. Probably a fair bit of boilerplate too in cases where the GUI calls straight through to the data model. Something like doThingAcion -> SwingGui.doThing -> PuffinData.doThing.
One major problem is testing. I'd want a lot of unit tests in place before starting on a refactoring like this, but unit testing Swing GUI code is hard. Even if I can get decent tests in place, they will slow down the test suite massively. Classic legacy code chicken-and-egg situation here: I want (ideally) everything under test before refactoring, but very hard to get tests in place before the code's been refactored…
Can I do this gradually? What's the minimum amount of code I can move into a PuffinData class without having to refactor big chunks of the codebase? I could start by simply making it a wrapper around the current PuffinApp.suites, perhaps. PuffinApp.getSuites() can just call through. Shouldn't break anything.
Where's PuffinApp used? As of [2018-10-22 Mon], not at all in data or plots. Lots in window and puffinplot of course but that's expected. In short, splitting it doesn't look too painful wrt effects on the rest of the codebase.
Next steps
There may be some more small bits I can pull out of PuffinApp before the big split. Build properties for a start – they're not a part of the GUI or the data model so probably belong in their own small class. The downloady bit of runPythonScriptInGui should be moved to JythonJarManager.