(core): new awesome proposal to manage data in services
Description
The well-known appXml2 introduced multiple data in services, as well as deferred objects, which imply an automatic start and stop of services depending on the presence of their data.
The a priori knowledge of the data needed for a service is described in the plugin.xml
of a module. Since it is tedious to write, and since the information is already present in the documentation of the service, we chose to generate these descriptions from the doxygen documentation. This works well but may be cumbersome, error-prone, and hard to understand since it is not a common way to achieve that. That is the first weakness in our current approach.
EDIT 07/06/2021: This has been removed in Sight 21.0. The plugin.xml must be written manually now.
The second weakness is that we need to tell the OSR how to link a deferred object to an object identifier (label). To achieve that, and especially because of C++ applications, a new method fwServices::registerObject()
was introduced. Technically, this is only needed when you intend to use a service with a deferred object. But actually, you never know so this should be done everywhere, which is not the case so far and was only been done when necessary, so when such a case occurred in one of our application. Most of the time, people do not know this mechanism and it is not clear to them why they need it.
The third weakness lies in the way we get the data. Currently, people have to choose between getting locked_ptr
or weak_ptr
, which may be a subtle difference and adds to the overall complexity. And if it is badly used, race conditions can still appear, even after this fix.
If we think about what an average C ++ developer would expect, we can reasonably say that all data information should be contained entirely in the service header. This is not the case now. The list and type of data objects lie only in the doxygen. The key is generally in the cpp file AND in the doxygen if it is well written. The average developper does not expect the doxygen to generate code. Neither he wants to "register" objects.
Proposal
The C++ code in the header should contain all data information: type, key name, access. It should be straightforward for the average developer to add and access data. Thinking twice, what is more natural than having its data has member variables? They could be initialized with the correct type, key and access, doing exactly what the registerObject function does, but forcing it to be done, avoiding it to be forgotten. The member variable would be or hold weak ptr, thus there would be no more question about using locked_ptr
or weak_ptr
. And the parsing of the doxygen could be replaced by a parsing of these member variables.
Example:
class SMesher : public ::fwServices::IOperator
{
// ...
private:
data_ptr< ::fwMedData::ImageSeries, AccessType::INPUT> m_imagePtr { "imageSeries" };
data_ptr< ::fwMedData::ModelSeries, AccessType::OUTPUT> m_meshPtr { "modelSeries" };
};
// Could replace the following existing code
static const std::string s_IMAGE_INPUT = "imageSeries";
static const std::string s_MODEL_OUTPUT = "modelSeries";
//-----------------------------------------------------------------------------
SMesher::SMesher() noexcept
{
this->registerObject(s_IMAGE_INPUT, ::fwServices::IService::AccessType::INPUT, false);
this->registerObject(s_MODEL_OUTPUT, ::fwServices::IService::AccessType::OUTPUT, false, true);
}
// And instead of accessing data this way
::fwMedData::ImageSeries::csptr imageSeries = this->getInput< ::fwMedData::ImageSeries >(s_IMAGE_INPUT);
// We could simply do
auto imageSeries = *m_image_ptr;
Outcomes
Readability, simplicity, robustness.
Links / references
This is a potential fix for #160 (closed).