Come faccio a sapere quando effettuare il refactoring delle chiamate globali alle query del database e quando lasciarle da sole?

5

Ho le seguenti chiamate di istruzioni globali sparse in tutta la mia base di codice legacy PHP:

$jobnumber = db_quick($sql);
db_query($sql);

Cioè, quelle chiamate di funzione sono state inizializzate nello scope globale e esse stesse richiamano un oggetto globale MySQL precedentemente inizializzato, cioè:

//initializes MySQL connection object in the global scope
$dblink = MySqlConnector::getMySql();

function db_query($sql)
{
    //previously initialized MySQL connection object
    global $dblink;  

    //native MySQL API call:
    return mysqli_query($dblink, $sql);
}

Domanda

Ci sono dei vantaggi nel rifattorizzare queste affermazioni globali, oltre a dire semplicemente "Le ho rimosse dall'ambito globale"?

Lo spirito della mia domanda va ... quelli sono già nella base di codice, e funzionano. Devo scherzare con loro? Finora [3 anni di lavoro con il codice] non ci sono stati danni apparenti. Anche con la transizione da deprecato mysql a mysqli, è stata una transizione indolore. La conservazione di questi come globale causerà danni misurabili in seguito? Sono abbastanza buoni da essere lasciati così com'è?

Cosa succede se refactoring?

A proposito di refactor, sto pensando di usare un pattern Singleton per l'istanza del link al database e poi chiamarlo tramite una classe istanziata, che chiama Singleton. Mi piace così:

//in a calling class:
$repository = new GenericRepository();
$repository->getMySql()->persist($sql);
$value = $repository->getMySql()->paramQuery($sql, $param);

//GenericRepository makes a static call to the actual connector
class GenericRepository 
{
    final public function getMySql()
    {
        if ($this->link === null)
            $this->link = MySqlConnector::getMySql();

            return $this->link;
    }
}

//Connector calls MySQL API
class MySqlConnector
{

    public static function getMySql()
    {
        if (null === static::$instance)
        {
            include 'include/config.php';

            //class MySql extends mysqli library
            static::$instance = new MySql(DBHOST, DBUSER, DBPASS);
        }

        return static::$instance;
    }
}

Quindi, in sostanza, refactoring, rimuoverò i metodi del database globale e li sostituirò con chiamate istanziate al pattern Singleton. Il cambio di codice sarà piuttosto esteso, poiché ci sono un paio di migliaia di chiamate al database globali.

Effetti del precedente Refactoring

Attualmente posso chiamare i metodi "da qualsiasi luogo" - sono globali.

db_query($sql);    

Dopo il refactoring potrò ancora chiamarli "da qualsiasi luogo", ma usando una stringa di connessione più lunga.

(new \Namespace\GenericRepository())->getMySql()->persist($sql);

Sembra un po 'un danno - una stringa più lunga, più da digitare, più da copiare e amp; incolla. Ci sono davvero dei benefici in questo?

    
posta Dennis 27.12.2016 - 17:51
fonte

2 risposte

2

C'è una ragione per cui db_query è nello scope globale, e questo perché hai bisogno che sia visibile ovunque, così puoi chiamarlo da qualsiasi luogo. Quando usi jQuery, $ è nell'ambito globale, ed è per questo che.

Prima di passare a un refactoring complesso solo per rimuovere una funzione dall'ambito globale, considerare i motivi per cui l'ambito globale è problematico. Penso che scoprirai che non è un ambito globale che è di per sé negativo, ma piuttosto variabili globali.

Inoltre, nota che le variabili globali non sono sempre cattive. Se ne stai usando uno in modo da non dover ripetere una stringa di connessione all'infinito nel tuo programma, è una buona cosa.

    
risposta data 27.12.2016 - 18:03
fonte
2

Il refactoring non è fine a se stesso, è un mezzo per un fine. E l'obiettivo è rendere il codice più testabile, manutenibile ed evolutivo.

Nel tuo caso, l'uso della stessa funzione globale $db_query che accede allo stesso oggetto globale $dblink durante l'intera base di codice impedisce di utilizzare facilmente questa funzione con due connessioni di database diverse durante un'esecuzione di esecuzione. La restrizione "una e una sola connessione db per istanza dell'applicazione" si manifesta nell'architettura. Qualsiasi codice creato sul $dbquery dovrà convivere con questa ipotesi e non può essere facilmente riutilizzato in un contesto diverso in cui l'ambiente db avrà un aspetto diverso o inserito in un contesto di test senza alcun database.

Questo può essere perfettamente ok . Per molte applicazioni, l'architettura "one db connection at a time" è tutto ciò di cui hanno bisogno durante tutta la loro vita. Se la tua applicazione è di questo tipo e non noti alcun vero svantaggio da questa restrizione, nemmeno per i test, lasciala così com'è.

Tuttavia, se questa architettura inizia a intralciare l'implementazione di nuove funzionalità o test, allora un tipo di refactoring di cui hai bisogno è un refactoring in cui il $dblink verrà fornito in modo dipendente dal contesto, o il database dietro $dbquery può essere facilmente deriso. Ciò dimostra che il tuo refactoring Singleton è probabilmente l'approccio sbagliato, perché non risolve le principali limitazioni della tua architettura.

    
risposta data 28.12.2016 - 10:14
fonte

Leggi altre domande sui tag