Govnokodu.Net

Пример применения рефакторингов «извлечение класса», «перемещение метода», «излечение метода»

Исходный код

typedef enSceneObjectsManager::tObjects::const_iterator It;
if (objects.size() != 0)
    for (It it1(objects.begin()); it1 != objects.end() - 1; ++it1)
        if ((*it1)->GetObjectType() == enSceneObject3D::GetClassType())
            for (It it2(it1 + 1); it2 != objects.end(); ++it2)
                if (((enSceneObject3D*)(enSceneObject*)(*it1))->GetResource().asInt() == 
                    ((enSceneObject3D*)(enSceneObject*)(*it2))->GetResource().asInt())
                    if ((*it1)->GetTransform() == (*it2)->GetTransform())
                        if (FindObject(theList, (*it2)) == -1)
                            InsertObject(theList, (*it2));

Как можно отрефакторить исходный код

Полноценно улучшить этот код можно лишь хорошо зная, что находится в обрабатываемом массиве, но кое-что можно сделать и вслепую.

На помощь опять приходит рефакторинг «извлечение метода».

Но начну я с извлечения класса и перемещения метода.

Два вызова методов FindObject и InsertObject издают явственный запах завистливых методов. Чтобы его устранить, нужно сделать переменную theList экземпляром нового класса с методами FindObject(), InsertObject() и объединяющим два предыдущих AddObject(). Тогда последние две строчки исходного кода станут выглядеть как theList->AddObject(*it2).

void AddObject(const enSceneObjectsManager::tObjects* object)
{
    if (FindObject(object) == -1)
        InsertObject(object);
}

Сравнение ресурсов и трансформаций стоит извлечь в отдельные методы, и добавить к ним ещё один, их объединяющий (над приведением типа не смеяться — это всего лишь пример извлечения метода):

bool IsSimilar(const enSceneObjectsManager::tObjects* object)
{
    returnIsEqualResource(object) && IsEqualTransform(object);
}
bool IsEqualResource(const enSceneObjectsManager::tObjects* object)
{
    returnGetResourceInt() == object->GetResourceInt();
}
int GetResourceInt()
{
    return ((enSceneObject3D*)(enSceneObject*)this)->GetResource().asInt();
}
bool IsEqualTransform(const enSceneObjectsManager::tObjects* object)
{
    returnGetTransform() == object->GetTransform();
}

Соответственно, последние шесть строк исходного кода будут выглядеть так:

if ((*it1)->IsSimilar(*it2))
    theList->AddObject(*it2);

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

Теория

Сергей Галанин