Code review changes for new REFL gui

[OwenArnold]

This ticket is blocked by :

• TRAC9371

This ticket is blocks : TRAC10223

Nice work on 9371. Here are some changes i suggest from my code review.

1. Missing : TODO : DESCRIPTION in headers

2. No need for Code/Mantid/MantidQt/CustomInterfaces/src/IReflPresenter.cpp

3. Mark slots as such in the implementation documentation for each. Makes them easier to read and identify what they relate to.

4. Declare and initialise close to use in ReflLoadedMainViewPresenter::hasValidModel()

5. CreateTransmissionWorkspaceAuto should not need the following. Just delete the lines and test that it still works (creating a transmission workspace)

algCreateTrans->setProperty("Params",HACKYPARAMS); //HACK FOR NOW
algCreateTrans->setProperty("StartOverlap",10.0); //HACK FOR NOW
algCreateTrans->setProperty("EndOverlap",12.0); //HACK FOR NOW

7. Transmission workspaces should be called TRANS_{0} or TRANS_{0}_{1} where {0} and {1} should be substituted for run numbers of the transmission workspaces. See refl_gui.py

8. Multiple definitions of the same type : class MockView : public ReflMainView

Instead, create a MockObjects.h and put in the test directory. Include it where needed, but don't add to cmake test list.

9. Tests are hard to read (not your fault)

• Use TSM_ASSERT where possible, as this provides the ability to write a custom message explaining what the assertion is actually checking.
• Comment each derived class from ReflMainView in the tests. Explain what it is being used for.
• Evaluate the need for so many Fake types. Add ones that are really needed, and could be shared to the aforementioned MockObjects.h. Better to refactor the code in such a way that Fakes are not needed altogether.

10. Created Tranmsission workspaces (see 7) should not be deleted at the end of processing.

11. Add a bold Prototoype label to the GUI. I don't want users accidently picking this up at this stage.

'''Things to think about, but not necessarily correct:'''

Might be better to inject a new object rather than create one in an opaque fashion. For example QtReflMainView::setNew()

QtReflMainView::getFlag() has unsafe behaviour equivalent to pop found on many containers. Method has two responsibilities. What should happen if
it fails to achieve one or the other of the responsibilities. What happens if another instance of getFlag is run from another thread at the same time?
Is it better that the view clears the flags, or that the presenter commands it?

Algorithm execution should keep workspaces out of the ADS until required otherwise. Use setChild(true) on the algorithms. For example of misuse see ReflMainViewPresenter::processRow()

[OwenArnold]

This issue was originally trac ticket 10222