Come evitare di cambiare la firma su un metodo ben utilizzato?

Ho deciso di spostare il metodo TextInCell (e un paio di altri metodi strettamente correlati) in un oggetto che memorizza l'opzione per il colore (blu, verde) come stato. Poi ho aggiunto un'istanza di questa nuova classe come variabile readonly privata della classe FormDesigner , che è responsabile del 99,8% di tutti gli usi. Dato che i colori non sono mai confusi all'interno di un modulo, ha senso: il progettista di moduli ha un oggetto di supporto che "sa" di quale colore è l'ombreggiatura. La maggior parte delle modifiche sono state apportate da ricerca e sostituzione globali e l'intera soluzione era pronta per il commit dopo 2 ore.

Il fatto che un design sia difficile da cambiare in un modo particolare non è sempre un problema, ma in questo caso penso che ci sia un design superiore che faciliti questa modifica.

Se il metodo anziché prendere una mezza dozzina di parametri facoltativi richiedeva un singolo ParameterObject , si potrebbe facilmente aggiungere un campo colore a quell'oggetto parametro senza influire sui chiamanti esistenti. In questo caso è possibile introdurre il design superiore mantenendo la compatibilità con le versioni precedenti introducendo un nuovo metodo prendendo un oggetto parametro, riscrivendo il vecchio metodo per inoltrarlo al nuovo e quindi deprecare il vecchio metodo.

In questo caso specifico , immagino che tu possa andare via senza un sovraccarico aggiuntivo o cambiando significativamente la firma - almeno non cambiandolo in un modo che il compilatore si lamenterà di esso.

Se l'uso tipico di CellStyle è solo utilizzando valori come CellStyle.SmallRed come valore letterale nel codice (e non molte posizioni nel codice assumendo implicitamente o esplicitamente questo è un enum obbligatorio), puoi anche provare se puoi sostituire CellStyle enum di una CellStyle classe con tre parametri FontSize (una nuova enumerazione con valori Piccolo, Grande, Normale), Color (un enum con tutti i colori necessari) e FontStyle (un enum con valori come Grassetto, Corsivo, Standard). Quindi, fornire alcuni membri predefiniti statici come

  class CellStyle
  {
      public static readonly CellStyle SmallRed 
         = new CellStyle(){
            FontSize=FontSize.Small, 
            Color=Color.Red
            FontStyle=FontStyle.Standard};
      // ...
  }

in sostituzione di tutti i tuoi 15 valori enum (Attenzione, questo non è un "codice aereo" non verificato, sono abbastanza sicuro che non verrà compilato, ma spero che tu abbia ottenuto l'idea).

Se sei fortunato, il 99% del tuo codice verrà compilato immediatamente, poiché la sintassi per l'utilizzo di queste costanti statiche è esattamente la stessa utilizzata per l'utilizzo dei valori enumerati . I pochi punti in cui il compilatore si lamenta possono probabilmente essere risolti con uno sforzo minimo. Ovviamente, l'unico punto in cui devi modificare il codice è l'implementazione del tuo metodo TextInCell , ma ciò è chiaramente necessario anche per qualsiasi altra soluzione.

Secondo me, la firma del metodo stessa è un odore di codice. Se mi ritrovo ad avere più di due o forse tre argomenti in una firma, di solito è perché quei parametri dovrebbero essere sotto forma di proprietà di classe invece di un singolo metodo.

Una volta che sei al punto che sei (con 1k + chiamate al metodo), le tue buone opzioni sono limitate. Il mezzo più sicuro (in termini di non violazione del codice esistente) consiste nell'aggiungere parametri opzionali alla fine della firma in modo tale che i chiamanti precedenti possano continuare a lavorare senza alcuna modifica. Si noti che questo non fa nulla per garantire che la logica all'interno di quel metodo rimanga corretta, però. E naturalmente, mentre questo è il modo più sicuro per evitare di rompere il contratto di chiamata, non è una buona soluzione a lungo termine in quanto si aggiunge semplicemente al problema originale.

Se fossi in me, guarderei il refactoring di qualsiasi cosa in quella cosa nella propria classe. Poi vedrei come posso fare una ricerca / sostituzione all'interno dei chiamanti esistenti per convertirlo. Quindi avrei un risultato molto migliore al costo di una conversione molto più difficile.

Un'altra opzione potrebbe essere l'aggiunta di parametri facoltativi (senza modificare nulla nei parametri esistenti), quindi utilizzare il metodo come wrapper legacy. Significa, all'interno di quel metodo, istanziare una nuova classe correttamente codificata e applicare i parametri alle proprietà, a quel punto il metodo stesso esiste realmente solo ai fini dei chiamanti legacy.

Non sono sicuro di come questo giochi con la tua classe di FormDesigner, ma osservandolo dall'ambito del metodo stesso, ci sono alcune idee almeno su come potresti affrontarlo.

1 è rischioso per rompere i chiamanti esistenti.

2 potrebbe funzionare, come indicato nel mio testo sopra ma potrebbe non essere pulito a lungo termine.

3 Non preferirei: ogni volta che hai un metodo che deve essere chiamato dopo un altro metodo per funzionare correttamente, di solito c'è un modo migliore per farlo.

4 sembra che non sia del tutto corretto, dato che in realtà dovresti avere una sola unità di codice per gestire questa roba, no?

5 è un sacco di lavoro, ma sembra essere il metodo che ti guiderà verso un codice più gestibile in futuro.

Does option (4) constitute a professional solution ?

Non ci andrei mai. L'introduzione dello stato globale è cattiva; è accettabile quando anche le alternative sono cattive, ma qui non è così.

It is obviously the easiest option.

Non lo è. Un semplice sovraccarico è altrettanto facile e non dovrai mai preoccuparti dello stato globale.

IIUYC, il tuo enum sta crescendo da 15 membri a 18 ea causa della struttura, non vuoi semplicemente lasciarlo crescere. È sensato. Aggiungere ancora un altro parametro a un metodo così completo sarebbe pazzesco poiché ce ne sono già troppi.

Soluzione conservativa

Il problema è che il tuo vecchio% enunciato CellStyle non è abbastanza espressivo, e questo può essere risolto in pochi semplici passaggi:

1. Hai bisogno di CellStyleAndShadingColor (questo è un nome stupido, ma rinominiamolo più tardi). Questa nuova classe è composta da due parti come dice il nome. Per ShadingColor usa un enum se vuoi e usa Colore o altro.

2. Ora riscrivi TextInCell in modo che funzioni con CellStyleAndShadingColor al posto di CellStyle semplicemente estraendo CellStyle e lasciando tutto il resto (complimenti, hai 1k errori).

3. Quindi aggiungi un sovraccarico con gli stessi argomenti del vecchio metodo e delegando a quello nuovo (ora, gli errori 1k scompaiono).

4. Ora, tutto dovrebbe funzionare esattamente come prima. Questo potrebbe essere un grande vantaggio quando era necessario un test manuale, ma abbiamo fatto solo un cambiamento molto banale. Quindi è il momento di implementare l'ombreggiatura.

Soluzione a lungo termine

Il problema dell'essere conservatori è che le cose non migliorano quasi mai. Vorrei non andare per un singolo oggetto parametro in quanto ciò avrebbe (almeno in parte) spostare la bruttezza sull'oggetto parametro. Un metodo di estensione chiamato come

Cell.SetText("text")
  .Style('BoldHeading', 'Blue')
  .Align('ParagraphAlignment.SPECIAL');

il raggruppamento dei parametri correlati insieme potrebbe essere il migliore (supponendo che ci sia un buon raggruppamento logico, altrimenti è già stato proposto). Invece del metodo di estensione, potresti iniziare con un'istanza che contiene l'ombreggiatura (la tua soluzione 5). In ogni caso, un metodo che inoltra le chiamate legacy ti evita di dover toccare i siti di chiamata esistenti.

Passa a uno schema di metodo di estensione compatibile con le versioni precedenti che supporta la sintassi fluente

Questo è molto più semplice di quanto sembri e non richiede modifiche al tuo codice legacy.

Dato che il tuo metodo assomiglia a questo:

public static Cell TextInCell(  Cell cell, 
    string text, 
    CellStyle style = CellStyle.Normal, 
    int mergeRight = 0, 
    bool underline = false, 
    ParagraphAlignment? horizontalAlignment = null, 
    VerticalAlignment? verticalAlignment = null)
{
    /*
        Main code goes here
    */
}

È maturo per essere trasformato in una estensione metodo , come questo:

public static Cell TextInCell(this Cell cell,  //All I did was add "this"
    string text, 
    CellStyle style = CellStyle.Normal, 
    int mergeRight = 0, 
    bool underline = false, 
    ParagraphAlignment? horizontalAlignment = null, 
    VerticalAlignment? verticalAlignment = null)
{
    /*
        Main code goes here
    */
}

Ed è maturo per essere utilizzato in un sintassi fluente :

public static Cell TextInCell(this Cell cell, Etc etc)
{
    /*
        Main code goes here
    */
    return cell;  //<-- this is what enables the fluent syntax
}

Ciò significa che puoi scrivere altri metodi di estensione su Cell e concatenarli. Ad esempio, per soddisfare i tuoi nuovi requisiti, potresti scrivere:

static Cell Background(this Cell cell, Color color)
{
    cell.BackgroundColor = color;
    return cell;
}

Quindi chiamalo con:

HelperClass.TextInCell(cell, "My text").Background(Color.Green);

Oppure (questo fa la stessa cosa):

cell.TextInCell("My text").Background(Color.Green);

Puoi continuare con qualsiasi altro metodo che desideri aggiungere, rendendo potenzialmente la tua API molto più flessibile (e molto più interessante):

cell.TextInCell("My text")
    .Style(Styles.Plain)
    .Background(Color.Green)
    .Alignment(Alignment.Left)
    .PetAnimal(Animals.Goat);

Così puoi mantenere il tuo vecchio codice e introdurre nuove funzionalità concatenando le chiamate, proprio come faresti con, per esempio, con jquery.

Bonus: la modifica del metodo statico esistente in un metodo di estensione non interromperà le chiamate esistenti. Puoi chiamarlo con

cell.TextInCell(....)

o

HelperClass.TextInCell(cell, ...);

Entrambi si compilano e funzionano esattamente allo stesso modo. Quindi questo schema è retrocompatibile al 100% e non dovrai affatto scimmiottare con il tuo vecchio codice. Potrebbe essere necessario aggiungere un'istruzione using nella parte superiore di alcuni file di codice e questo è tutto.