Search code examples
c++stlcppcheck

C++ CppCheck algorithm suggestion (std::find_if instead of raw loop) pertinence


CppCheck suggest me to replace one of my code by a STL algorithm, I'm not against it, but I don't know how to replace it. I'm pretty sure this is a bad suggestion (There is warning about experimental functionalities in CppCheck).

Here is the code :

    /* Cutted beginning of the function ... */

    for ( const auto & program : m_programs )
    {
        if ( program->compare(vertexShader, tesselationControlShader, tesselationEvaluationShader, geometryShader, fragmentShader) )
        {
            TraceInfo(Classname, "A program has been found matching every shaders.");

            return program;
        }
    }

    return nullptr;
} /* End of the function */

And near the if condition I got : "Consider using std::find_if algorithm instead of a raw loop."

I tried to use it, but I can't get the return working anymore... Should I ignore this suggestion ?


Solution

  • I suppose you may need to use that finding function not once. So, according to DRY, you need to separate the block where you invoke an std::find_if algorithm to a distinct wrapper function.

    {
    
        // ... function beginning
    
        auto found = std::find_if(m_programs.cbegin(), m_programs.cend(), 
           [&](const auto& prog)
           {
               bool b = prog->compare(...);
               if (b)
                     TraceInfo(...);
               return b;
           });
    
        if (found == m_programs.cend())
           return nullptr;
    
        return *found;
    
    }
    
    

    The suggestion is good. An STL algorithm migth be able to choose an appropriate approach based on your container type.

    Furthermore, I suggest you to use a self-balancing container like an std::set.

    
    // I don't know what kind of a pointer you use. 
    using pProgType = std::shared_pointer<ProgType>;
    
    bool compare_progs(const pProgType &a, const pProgType &b)
    {
       return std::less(*a, *b);
    }
    
    std::set<std::shared_pointer<prog_type>, 
        std::integral_constant<decltype(&compare_progs), &compare_progs>> progs.
    
    

    This is a sorted container, so you will spend less time for searching a program by a value, given you implement a compare operator (which is invoked by std::less).

    If you can use an stl function, use it. This way you will not have to remember what you invented, because stl is properly documented and safe to use.