Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
2k stars 574 forks source link

Type#GetLoadDependencies(): VERIFY() that only config object types are returned #10169

Closed Al2Klimov closed 1 week ago

yhabteab commented 1 week ago

No way, you are introducing yet another confusing loop! Please merge this with the existing loop that performs a nullptr check.

Al2Klimov commented 1 week ago

But this looks so cool:


const std::unordered_set<Type*>& TypeImpl<Service>::GetLoadDependencies() const
{
    static const auto deps ([] {
        auto typeApiListener (GetByName("ApiListener").get());
        auto typeEndpoint (GetByName("Endpoint").get());
        auto typeHost (GetByName("Host").get());
        auto typeZone (GetByName("Zone").get());

        VERIFY(typeApiListener);
        VERIFY(typeEndpoint);
        VERIFY(typeHost);
        VERIFY(typeZone);

        VERIFY(ConfigObject::TypeInstance->IsAssignableFrom(typeApiListener));
        VERIFY(ConfigObject::TypeInstance->IsAssignableFrom(typeEndpoint));
        VERIFY(ConfigObject::TypeInstance->IsAssignableFrom(typeHost));
        VERIFY(ConfigObject::TypeInstance->IsAssignableFrom(typeZone));

        return std::unordered_set<Type*>{ typeApiListener, typeEndpoint, typeHost, typeZone, };
    }());

    return deps;
}
yhabteab commented 1 week ago

Seriously, it's all about coolness? :-)

With every bit of such a useless loop you slow down the compilation process, so I'd rather group them like this instead:

VERIFY(typeApiListener);
VERIFY(ConfigObject::TypeInstance->IsAssignableFrom(typeApiListener));

...