Request for Comments-42: Improve the core of ApplicationEngine

[Request for Comments- 42] Improve the core of ApplicationEngine

Status

• Author: Guillaume Pasero
• Submitted on 31.10.2017
• Open for comments

Content

What changes will be made and why they would make a better Orfeo ToolBox?

After some time, I have seen several potential improvements that would make the code of ApplicationEngine more robust, and easier to use. The OTB Application are now used very often. New developers usually start by writing an application, so it is important to have a clear and robust framework.

• Handling of "UserValue" flag : the optional boolean in 3rd argument of the functions SetParameterString/SetParameterInt/... is not very convenient. I think it is possible to get rid of it using a simple state flag "IsInPrivateDo" in the Application class. This flag would be set to ON during calls to DoInit(), DoUpdateParameters() and DoExecute(). During these calls, the values set on parameters are always "Automatic" values. Outside these calls, we can assume that the values set on parameters come from the user.
• Also about the "UserValue" and "AutomaticValue" flag : why do we have both ? I believe we could choose the convention : UserValue = not(AutomaticValue). The initial default value of a parameter would be considered "automatic". We could remove 1 of these 2 flags, there are already enough in the Parameter class.
• Handling of the m_Active flag: this flag should be handled mostly by the Parameters themselves in the ApplicationEngine module. For instance the call to EnableParameter() in otb::Wrapper::CommandLineLauncher::LoadParameters() should not be here, but rather in each "param->SetValue(string)". In a similar way, in otb::Wrapper::Application::SetParameterUserValue(), the call to EnableParameter() should not be here (in addition, it will always switch ON an EmptyParameter). The benefit is also to guarantee the same behaviour on CLI / GUI /Python.
• The ParameterType_Empty should be refactored. There are a lot of special cases for this parameter because its value (a boolean flag) and its Active flag are actually the same. For instance, in the current state of ApplicationEngine, it is impossible to have an Empty parameter with Role_Output. This parameter should have its value stored in a specific flag, proper functions Application::Set/GetParameterBool(), and maybe easy conversions to/from string and integer values. Maybe a specific ParameterType_Boolean can be added...
• It would be nice if all the parameters could be set using a common Application::SetParameterString(), or SetParameterStringList(). Same remark for getters. It would remove some long sections of if/elseif/elseif/... in the CommandLineLauncher but also in the Input/OutputXML parameters.
• In the Parameter base class, the flag m_IsChecked is only used by QtWrapper, so it should be moved outside ApplicationEngine.
• There is also room for improvement in the architecture of Parameters. There is a lot of code that is duplicated between similar parameter types, we should consider a better class architecture to gather common features between parameters with similar behaviour.
• In the Parameter base class : the enum m_DefaultValueMode is never used in OTB. We could check if other external projects use it. We could get rid of it.

I know this module is delicate, and that regressions can occur easily, but in the long term we should try to keep this code clean.

If you see other potential improvements to the core of this module, feel free to give your idea.

Check improvements to fix bug #1475

When will those changes be available (target release or date)?

No target release for the moment.