Merge ReflBlankMainViewPresenter and ReflLoadedMainViewPresenter

[Harry Jeffery]

Having interchangeable presenters can be very useful if their behaviour is radically different, such as a completely different analysis back end. However, these two presenters share 99% of their functionality through ReflMainViewPresenter.

The only difference in their behaviour is whether save() asks for a workspace name or not. This is trivial and should be handled as an if statement within a generic presenter, not a whole new presenter.

Additionally, when the user changes the workspace/model, the view destroys the presenter and creates an entirely new one to replace it. Rebuilding the entire presenter seems excessive and expensive. It also seems like it should be the presenter's prerogative when it is to be replaced. Instead of creating a new presenter, the view should notify the presenter that it wishes to load a different workspace and then the presenter can take care of the heavy lifting. If a new presenter is actually _required_ each time the workspace changes (which is doubtful), then the presenter can pass the view a new presenter to report to.

Another downside to having two separate presenters is we either have to duplicate a lot of testing, or just assume that success on one presenter implies success on the other.

[Owen Arnold]

I agree on both counts with the following provisos/suggestions:

• If the behaviour of the two presenters starts to deviate significantly, we need to encourage the creation of a new subclass rather than peppering the code with conditional statements.
• I haven't thoroughly thought this through, but one way of keeping the same code, but ensuring type safety would be to use compile type polymorphism (i.e. templated code) see CRTP ​http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern
• Yes, the presenter should be making these decisions. I doubt that the presenter construction is expensive in terms of either memory or CPU time, but I agree, it is excessive. One thing that is also a problem with the View creating the presenter in the current scenario is that it needs to know which type of presenter to create, this is an violation of our 'stupid view' approach. One way to solve this would be to pass a FactoryMethod object to the view, but I digress, and this is an over-engineering of the current problem.

[Harry Jeffery]

(In #10304) Blocked by #10364, which is merging the two sets of unit tests.

[Harry Jeffery]

This commit has been deleted.

[Harry Jeffery]

This commit has been deleted.

[Harry Jeffery]

#10302 is causing some nasty merge conflicts. Waiting for that to pass testing so that it can be merged into this branch.

[Harry Jeffery]

Added blocked by #10301 to ensure I complete that work first, rather than perform it in the midst of this and end up with nasty merge conflicts.

[Harry Jeffery]

Refs #10364 Merge Refl presenters into ReflMainViewPresenter

Changeset: 1dded72502f7701c04a21e45e257726ae1f60549

[Harry Jeffery]

Refs #10364 Add unit tests for ReflMainViewPresenter

These unit tests are by no means complete, and are due for a complete overhaul in the near future. They do however cover almost all functionality, so they are sufficient for the moment.

Changeset: 53c51fc79b07f7b809ee7ef8911a446b2a3e1f56

[Harry Jeffery]

Refs #10364 Remove ReflMainView.cpp

Changeset: 75675252bb72c830d8966c7973d74ab668b5a698

[Harry Jeffery]

Refs #10364 Move instrument list logic out of ReflMainView

The presenter is now responsible for giving the view a list of valid instruments and selecting a default one.

Changeset: 0d4eca48ac9dc5846c5e3d8116fbbf528851c640

[Harry Jeffery]

Refs #10364 Test loading invalid workspaces

The old tests were dropped during the merge. Let's re-add them.

Changeset: 051d75611562c5a909f9dacf1a2f014f0ef9664f

[Harry Jeffery]

Refs #10364 Cheer up doxygen

Changeset: 10f9827938225da4e0d29bf179dd09c8c62977a0

[Harry Jeffery]

Refs #10364 Fix build on Win7 and RHEL6

Changeset: 36052a719f31b78900c2725e74851cfa8b4f1e29

[Harry Jeffery]

Testing

• Inspect code
• Verify unit test is passing
• Play with the new Refl UI, verify nothing has broken

[Harry Jeffery]

(In #10380) Waiting for #10364 to pass before unit test can be added.

[Owen Arnold]

Merge remote-tracking branch 'origin/feature/10364_merge_refl_presenters'

Full changeset: ​6123bd47daefc28ddf88d7e9a8851f620cf5b8cc

[Dan Nixon]

Add testing for Vesuvio resolution algorithm

Refs #10364

Changeset: dff3da88d77fbd339a34c95eac334e7d0a10989b

[Stuart Campbell]

This ticket has been transferred to github issue ​11206