Body execute prevents executing extensionexecute for its extensions

[abdullah]

Similar to the viewprovider case.

Body::execute is:

Code: Select all

    Part::TopoShape tipShape;
    if ( tip ) {
        if ( !tip->getTypeId().isDerivedFrom ( PartDesign::Feature::getClassTypeId() ) ) {
            return new App::DocumentObjectExecReturn ( "Linked object is not a PartDesign feature" );
        }

        // get the shape of the tip
        tipShape = static_cast<Part::Feature *>(tip)->Shape.getShape();

        if ( tipShape.getShape().IsNull () ) {
            return new App::DocumentObjectExecReturn ( "Tip shape is empty" );
        }

        // We should hide here the transformation of the baseFeature
        tipShape.transformShape (tipShape.getTransform(), true );

    } else {
        tipShape = Part::TopoShape();
    }

    Shape.setValue ( tipShape );
    return App::DocumentObject::StdReturn;

This prevents a call to all the previous executes of the base classes up the inheritance, including the one of DocumentObject, who is responsible from call extensionExecute for its extensions.

Apparently is not important from a functional point of view, but it is preventing from executing:

Code: Select all

App::DocumentObjectExecReturn *OriginGroupExtension::extensionExecute() {
    try { // try to find all base axis and planes in the origin
        getOrigin ();
    } catch (const Base::Exception &ex) {
        //getExtendedObject()->setError ();
        return new App::DocumentObjectExecReturn ( ex.what () );
    }

    return GeoFeatureGroupExtension::extensionExecute ();
}

In the case of Body::execute, at least the last line, I think, should be at least:

Code: Select all

return DocumentObject::execute();

or

Code: Select all

return Part::Feature::execute();

if there is a need to execute up the hierarchy, because:

Code: Select all

App::DocumentObjectExecReturn *Feature::execute(void)
{
    this->Shape.touch();
    return GeoFeature::execute();
}

Aside from this but related

When things go wrong, usually there is a return with a code, like in:
return new App::DocumentObjectExecReturn ( "Tip shape is empty" );

I do understand that things went wrong, however, did we voluntarily decide that on the first error we stop processing the execute and do not process the extensions?

I note that the processing of the potential different extensions is consistent with stopping on the first error:

Code: Select all

    //call all extensions
    auto vector = getExtensionsDerivedFromType<App::DocumentObjectExtension>();
    for(auto ext : vector) {
        auto ret = ext->extensionExecute();
        if (ret != StdReturn)
            return ret;
    }

    return StdReturn;

If one extension fails, the next ones are not executed.

This means that on failure, the extended functionality is ignored.

This differs from other extended functionalities, such as updateData, where all extensions are executed:

Code: Select all

void ViewProvider::updateData(const App::Property* prop)
{
    auto vector = getExtensionsDerivedFromType<Gui::ViewProviderExtension>();
    for (Gui::ViewProviderExtension* ext : vector)
        ext->extensionUpdateData(prop);
}

One can argue though that the cases are different. This is, we decide to operate differently execute() and other functions.

I do not have a strong opinion on what should be the behaviour, I am just interested in knowing how it should be, so to have consistent code and a consistent pattern to follow.