CODEWITHSUNDEEP
c++11mfcstdmap
Andrew Truckle
Andrew Truckle

Reputation: 19157

Is it possible to simplify this code which iterates a std map?

So I have this definition:

using HistoryDataItemList = list<CHistoryDataItem>;
HistoryDataItemList m_listHistoryItems;

And this method:

bool CHistoryData::HasHistoryForAssignment(ASSIGN_TYPE_E eAssignType)
{
    bool bHasHistoryForAssignment = false;
    auto iter = m_listHistoryItems.begin();

    for (; iter != m_listHistoryItems.end(); iter++)
    {
        if (iter->GetAssignmentType() == eAssignType)
        {
            bHasHistoryForAssignment = true;
            break;
        }
    }

    return bHasHistoryForAssignment;
}

I know that with languages like C# you can simplify the code a bit but can that also be done with MFC when using the newer C++11? Or is what I have written pretty much as simple as it can be?

Thank you.

Upvotes: 0

Views: 49

Answers (1)

Vittorio Romeo
Vittorio Romeo

Reputation: 93324

The most obvious improvement is using a C++11 "range-for" loop:

bool CHistoryData::HasHistoryForAssignment(ASSIGN_TYPE_E eAssignType)
{
    bool bHasHistoryForAssignment = false;

    for (auto& item : m_listHistoryItems)
    {
        if(item.GetAssignmentType() == eAssignType)
        {
            bHasHistoryForAssignment = true;
            break;
        }
    }

    return bHasHistoryForAssignment;
}

But thinking about your code, what you really want is to check if any of the items matches a particular predicate. There's an <algorithm> for that!

bool CHistoryData::HasHistoryForAssignment(ASSIGN_TYPE_E eAssignType)
{
    return std::any_of(std::begin(m_listHistoryItems),
                       std::end(m_listHistoryItems),
                       [](const CHistoryDataItem& item) 
                       {
                           return item.GetAssignmentType() == eAssignType;
                       });
}

Alternatively, using a for with no extra state is also easy to read and understand:

bool CHistoryData::HasHistoryForAssignment(ASSIGN_TYPE_E eAssignType)
{
    for (auto& item : m_listHistoryItems)
    {
        if(item.GetAssignmentType() == eAssignType)
            return true;
    }

    return false;
}

Upvotes: 5

Related Questions