Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Problem] Possible design flaw about overloaded method #12594

Closed
2 tasks done
wwmayer opened this issue Feb 24, 2024 · 1 comment · Fixed by #12471
Closed
2 tasks done

[Problem] Possible design flaw about overloaded method #12594

wwmayer opened this issue Feb 24, 2024 · 1 comment · Fixed by #12471
Labels
Bug This issue or PR is related to a bug WB Part Related to the Part Workbench

Comments

@wwmayer
Copy link
Contributor

wwmayer commented Feb 24, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Problem description

In ComplexGeoData there is this virtual method declared:

    virtual ElementMapPtr resetElementMap(ElementMapPtr elementMap = ElementMapPtr(),
                                          ElementMapResetPolicy forceEmpty = ForceEmptyMap);

In the sub-class TopoShape a method with the same name but different number of arguments is re-defined

    virtual Data::ElementMapPtr resetElementMap(
        Data::ElementMapPtr elementMap=Data::ElementMapPtr());

Because the method has a different signature it overloads the method of the base class instead of overriding it. This leads to the compiler warning: 'Part::TopoShape::resetElementMap' hides overloaded virtual function [-Woverloaded-virtual]
Because TopoShape is a central class that is used by a lot of classes directly or indirectly a full rebuild prints this warning a several thousand times.

Usually, in such cases one could add this line to the class declaration of TopoShape:
using ComplexGeoData::resetElementMap;
but because in both versions all argument are optional this leads of course to an error that resetElementMap is ambiguous.

So, this is a strong indication that there is a design flaw which may result into unexpected behaviour.

Full version info

Not relevant here

Subproject(s) affected?

Part

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@maxwxyz maxwxyz added Bug This issue or PR is related to a bug WB Part Related to the Part Workbench labels Feb 24, 2024
@bgbsww
Copy link
Contributor

bgbsww commented Feb 26, 2024

This crept in as part of the toponaming work, and is ultimately related to the elementMap in ComplexGeoData not being initialized before being used in some cases. The correct solution is likely to revert to code without the forceEmpty parameter which resolves the design flaw that was created. There is active work on solving the null elementMap problem in the toponaming team; I expect this to be resolved shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This issue or PR is related to a bug WB Part Related to the Part Workbench
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants