Strange typecast switch in sketcher code

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
User avatar
DeepSOIC
Posts: 7616
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Strange typecast switch in sketcher code

Postby DeepSOIC » Tue Oct 29, 2019 8:11 pm

https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L4401

Code: Select all

void free(std::vector<Constraint *> &constrvec)
{
    for (std::vector<Constraint *>::iterator constr=constrvec.begin();
         constr != constrvec.end(); ++constr) {
        if (*constr) {
            switch ((*constr)->getTypeId()) {
                case Equal:
                    delete static_cast<ConstraintEqual *>(*constr);
                    break;
                case Difference:
                    delete static_cast<ConstraintDifference *>(*constr);
                    break;
                case P2PDistance:
                    delete static_cast<ConstraintP2PDistance *>(*constr);
                    break;
                case P2PAngle:
                    delete static_cast<ConstraintP2PAngle *>(*constr);
                    break;
                case P2LDistance:
                    delete static_cast<ConstraintP2LDistance *>(*constr);
                    break;
                case PointOnLine:
                    delete static_cast<ConstraintPointOnLine *>(*constr);
                    break;
                case Parallel:
                    delete static_cast<ConstraintParallel *>(*constr);
                    break;
                case Perpendicular:
                    delete static_cast<ConstraintPerpendicular *>(*constr);
                    break;
                case L2LAngle:
                    delete static_cast<ConstraintL2LAngle *>(*constr);
                    break;
                case MidpointOnLine:
                    delete static_cast<ConstraintMidpointOnLine *>(*constr);
                    break;
                case None:
                default:
                    delete *constr;
            }
        }
    }
    constrvec.clear();
}
Any idea why cast? Seems pretty equivalent to just iterating over constrvec and calling delete it exept for None case. The None case looks funny too, as why wouldn't one want to delete the thing anyway?.. but may be legit.
abdullah wrote:🔔
eivindkvedalen
Posts: 602
Joined: Tue Jan 29, 2013 10:35 pm

Re: Strange typecast switch in sketcher code

Postby eivindkvedalen » Tue Oct 29, 2019 9:01 pm

DeepSOIC wrote:
Tue Oct 29, 2019 8:11 pm
https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L4401

Code: Select all

void free(std::vector<Constraint *> &constrvec)
{
 ...
}
Any idea why cast? Seems pretty equivalent to just iterating over constrvec and calling delete it exept for None case. The None case looks funny too, as why wouldn't one want to delete the thing anyway?.. but may be legit.
Agree, the Constraint class has virtual functions, so the (correct) virtual destructor will be invoked when just doing delete *constr. There is no reason to check for *constr != nullptr; it is ok to delete it anyway. It should be possible to replace the whole loop by just

Code: Select all

std::for_each(begin(constrvec), end(constrvec), std::default_delete<Constraint>());
Eivind
wmayer
Site Admin
Posts: 15971
Joined: Thu Feb 19, 2009 10:32 am

Re: Strange typecast switch in sketcher code

Postby wmayer » Tue Oct 29, 2019 9:09 pm

The None case looks funny too, as why wouldn't one want to delete the thing anyway?.. but may be legit.
For None there is no break so that it falls through to the default case. So, Eivind is right that the whole thing can be replaced with the one-liner.
User avatar
DeepSOIC
Posts: 7616
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Strange typecast switch in sketcher code

Postby DeepSOIC » Tue Oct 29, 2019 10:15 pm

ok, thanks