Refactoring: riduzione della duplicazione senza creare flag

1

Non so come refactoring un pezzo di codice che differisce da altro nel numero di condizioni controllate su una clausola if . Lascia che ti mostri un esempio del mondo reale che sto affrontando proprio ora.

L'unica differenza tra i due metodi è che all'interno elseif , controlla una condizione aggiuntiva sul nuovo parametro.

Metodo 1:

public function getKeysToInvalidateOnCreation($entity, $allKeys)
{
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY || $key_type == self::CUSTOM_KEY) {
                    $result[] = $key;
                } elseif (strpos($key, 'id:') === false) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

Metodo 2:

public function getKeysToInvalidateOnModification($entity, $allKeys, $modifiedId)
{
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY || $key_type == self::CUSTOM_KEY) {
                    $result[] = $key;
                } elseif (strpos($key, 'id:') === false ||
                          strpos($key, 'id:'.$modifiedId) !== false) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

Quello che vorrei fare è estrarre tutto il codice interno in un nuovo metodo protetto che avrebbe il terzo parametro come facoltativo, e poi fare il secondo controllo solo se è impostato quel parametro. Ma per me ha una specie di cattivo odore: sembra come aggiungere una bandiera al metodo.

Quale sarebbe la strategia di refactoring appropriata?

    
posta antonienko 31.12.2014 - 12:27
fonte

2 risposte

6

PHP non è un linguaggio con cui ho molta esperienza, ma, a quanto ho capito, entrambi i loop stanno cercando di filtrare il set di chiavi, quindi puoi estrarre la struttura del ciclo in un altro metodo che accetta le condizioni per il filtraggio come un argomento sotto forma di una funzione:

private function getKeysWhere($entity, $allKeys, $condition)
{
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY ||
                    $key_type == self::CUSTOM_KEY ||
                    $condition($key)) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

Questo richiede l'uso di PHP 5.3 o versioni successive, per fare cose come passare le funzioni come argomenti e dichiarare funzioni anonime.

Questo è molto più generale e potente di un flag booleano, perché invece di scegliere tra due condizioni, puoi spostare la logica per includere la chiave in una funzione separata e separare la preoccupazione di filtrare le chiavi dalla preoccupazione di quali condizioni filtrarli per.

Ora definiamo quali sono le condizioni per una chiave da invalidare in caso di modifica e di creazione come metodi di supporto privati:

private function keyIsInvalidOnCreation($key) {
    return strpos($key, 'id:') === false;
}

private function keyIsInvalidOnModification($key, $modifiedId) {
    return strpos($key, 'id:') === false || strpos($key, 'id:'.$modifiedId) !== false;
}

E infine, possiamo definire i nostri due metodi per ottenere chiavi non valide in termini di questo metodo generale e di questi due predicati:

public function getKeysToInvalidateOnCreation($entity, $allKeys) {
    return getKeysWhere($entity, $allKeys, 'keyIsInvalidOnCreation');
}

public function getKeysToInvalidateOnModification($entity, $allKeys, $modifiedId) {
    return getKeysWhere($entity, $allKeys, function($key) use ($modifiedId) {
        return keyIsInvalidOnModification($key, $modifiedId)
    });
}

Il caso per la modifica deve essere racchiuso in una funzione anonima al fine di passare correttamente in $modifiedId al predicato e trasformarlo in una funzione di un argomento per lavorare con getKeysWhere . Se si utilizza una libreria con supporto per strumenti di programmazione funzionale comuni come curry e applicazione parziale, è possibile utilizzarla qui come alternativa più pulita al wrapper della funzione anonima. Non sei sicuro di quanto sia possibile che alcune cose siano in PHP.

Questo è abbastanza pesantemente decomposto e potrebbe essere difficile da ingannare se non hai familiarità con i modelli di programmazione funzionale, ma riduce la duplicazione e generalizza bene.

    
risposta data 31.12.2014 - 13:08
fonte
3

Quindi, solo per altri tipi di PHP che potrebbero essere alle prese con la programmazione funzionale, voglio registrare qui i passi che ho seguito seguendo la risposta di Jack.

Per prima cosa ho rifattorato ogni metodo per estrarre la condizione in modo da avere una base di codice identica in entrambi (nota come la condizione è stata estratta in entrambi):

public function getKeysToInvalidateOnCreation($entity, $allKeys)
{
    $condition = function($key) {
        return strpos($key, 'id:') === false;
    };
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY || $key_type == self::CUSTOM_KEY || $condition($key)) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

public function getKeysToInvalidateOnModification($entity, $modifiedId, $allKeys)
{
    $condition = function($key) use ($modifiedId) {
        return strpos($key, 'id:') === false || strpos($key, 'id:' . $modifiedId) !== false;
    };
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY || $key_type == self::CUSTOM_KEY || $condition($key)) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

private function keyIsInvalidOnModification($modifiedId, $key)
{
    return strpos($key, 'id:') === false || strpos($key, 'id:' . $modifiedId) !== false;
}

private function keyIsInvalidOnCreation($key)
{
    return strpos($key, 'id:') === false;
}

In questo momento, tutti i test sono ancora verdi, quindi posso procedere ad estrarre il codice comune in un metodo privato:

public function getKeysToInvalidateOnCreation($entity, $allKeys)
{
    $condition = function($key) {
        return strpos($key, 'id:') === false;
    };
    return $this->getKeysToInvalidate($entity, $allKeys, $condition);
}

public function getKeysToInvalidateOnModification($entity, $modifiedId, $allKeys)
{
    $condition = function($key) use ($modifiedId) {
        return strpos($key, 'id:') === false || strpos($key, 'id:' . $modifiedId) !== false;
    };
    return $this->getKeysToInvalidate($entity, $allKeys, $condition);
}

private function getKeysToInvalidate($entity, $allKeys, callable $condition)
{
    $result = [];
    foreach ($allKeys as $key) {
        $entities_in_key = explode('+', substr($key, 0, strpos($key, '_')));
        $key_type = substr($key, strrpos($key, '_'));
        foreach ($entities_in_key as $entity_in_key) {
            if ($entity_in_key == $entity) {
                if ($key_type == self::GET_ALL_KEY || $key_type == self::CUSTOM_KEY || $condition($key)) {
                    $result[] = $key;
                }
            }
        }
    }
    return $result;
}

E tutto ancora verde. Grazie Jack

    
risposta data 31.12.2014 - 14:18
fonte

Leggi altre domande sui tag