Добрый день.

Вот два куска кода:

раз

    // remove fields that dont exist anymore
    for (i=0; i<m_arrFieldObj.GetSize(); ++i)
    {
        for (int j=0; j<qr.names.GetSize(); ++j)
        {
            if(!qr.names[j].CompareNoCase(m_arrFieldObj[i].m_strName))
                break;
        }
        // field not found
        if (j<qr.names.GetSize())
            continue;
        m_arrFieldObj.RemoveAt(i);
        i--;
    }

два

    for(FieldVector::iterator it = m_arrFieldObj.begin();
        it != m_arrFieldObj.end();
        )
    {
        bool found = false;

        for (int j = 0; j < qr.names.GetSize(); ++j)
        {
            if(0 == qr.names[j].CompareNoCase((*it)->GetName()))
            {
                found = true;
                break;
            }
        }

        if(!found)
        {
            it = m_arrFieldObj.erase(it);
        }
        else
        {
            it ++;
        }
    }

Второй получен мной путем рефакторинга из первого.
Я считаю, что первый код ужасен и непонятен, второй еще более-менее нормален.
Человек, который писал первый код утверждает, что оба кода понятны и нормальны.

Скажите, я слишком критичен или нет?