Refactoring Kataloğu 1: Extract/Inline Function, Extract/Inline Variable, Change Function Declaration, Rename ve Replace Magic Literal

Sürüm notu: Bu yazıdaki kodlar C# 14 ve .NET 10 (SDK 10.0.404) ile, testler xUnit.net v3 4.0.3 ve Verify.XunitV3 32.0.0 ile derlenip denenmiştir.

Önce Tahmin Et: Math.Round'a Geçmek Davranışı Değiştirir mi?

Sitedeki serilerin ortak örnek uygulaması Bookstore'un (bir online kitapçı) ödeme adımında sepet toplamını CheckoutService.Calculate hesaplıyor: ara toplam, satır bazında KDV (katma değer vergisi), kargo ücreti ve genel toplam. Kod yıllardır çalışıyor ve testi yok:

public class CheckoutService
{
    public CheckoutTotals Calculate(List<CartLine> lines, bool exp)
    {
        decimal t = 0;
        decimal v = 0;
        bool p = false;
        foreach (CartLine l in lines)
        {
            decimal lt = l.UnitPrice * l.Quantity;
            t += lt;
            if (l.IsEbook)
            {
                v += Math.Floor(lt * 0.20m * 100 + 0.5m) / 100;
            }
            else
            {
                v += Math.Floor(lt * 0.10m * 100 + 0.5m) / 100;
                p = true;
            }
        }

        decimal s = 0;
        if (p)
        {
            s = GetShippingBase(t);
            s = AddExpress(s, exp);
        }

        CheckoutTotals result = new(t, v, s, t + v + s);
        return result;
    }

    private decimal GetShippingBase(decimal t)
    {
        if (t >= 300)
            return 0;
        return 49.90m;
    }

    private decimal AddExpress(decimal s, bool exp)
    {
        if (exp)
            return s + 35;
        return s;
    }
}

KDV satırlarındaki Math.Floor(x * 100 + 0.5m) / 100 ifadesi elle yazılmış bir yuvarlama. Ekipten biri bunu Math.Round(x, 2) ile değiştirmek istiyor: "aynı şey, sadece daha okunur." Sence bu değişiklik bir refactoring mi, yoksa bir müşterinin faturası değişecek mi? (Örnekteki oranlar yalnız örnek için seçildi, güncel vergi oranları değil.)

Katalog Bloğu: Ne İşlenecek, Nasıl İşlenecek?

Serinin ilk dokuz yazısı refactoring'i bir yolculuk olarak ele aldı: güvenlik ağı, küçük adımlar, legacy kodda dependency kırmak, büyük değişiklikleri planlamak ve teknik borcu önceliklendirmek. O yazılarda Extract Function, Rename ya da Introduce Parameter Object gibi adlar bir vakanın içinde geçti, ama tek tek ele alınmadı. Bu yazıyla başlayan blok, Martin Fowler'ın Refactoring kitabının ikinci baskısındaki (2018) kataloğu aile aile işliyor. Katalogdaki 66 refactoring'in adları refactoring.com/catalog sayfasında listeleniyor; adları oradaki gibi İngilizce kullanıyoruz, açıklamalar ve örnekler ise bizim, hepsi Bookstore üzerinde.

Her refactoring için aynı soruları soracağız: hangi code smell'e cevap veriyor (smell adları Clean Code serisinin "Code Smell Kataloğu: C# Kodunda Kokuyu Tanımak" yazısındakiyle aynı), mekanik adımları ne, C#'ta önce ve sonra nasıl görünüyor, IDE bunu otomatik yapabiliyor mu ve tersi hangi refactoring. Son soru önemli; katalogdaki birçok refactoring'in bir tersi var ve hangi yönün doğru olduğu koda göre değişir:

Bu yazı en sık kullanılan grubu ele alıyor: Rename Variable ve Rename Field, Inline Variable, Inline Function, Replace Magic Literal, Replace Inline Code with Function Call, Extract Function, Extract Variable ve Change Function Declaration. Hepsini yukarıdaki Calculate üzerinde, üçüncü yazıdaki ritimle uyguluyoruz: tek adım, derle, test et, yeşilse commit.

İkinci yazıdaki gibi işe güvenlik ağıyla başlıyoruz. Calculate'in girdisi bir sepet ve bir ekspres kargo bayrağı; beş sepet ve bayrağın iki hâli tek bir Verify snapshot'ına sığıyor. Sepetlerden birini bilerek seçtim: 12,45 TL'lik bir kitabın KDV'si tam yarım kuruşa denk geliyor.

private static readonly Dictionary<string, List<CartLine>> Carts = new()
{
    ["empty"] = [],
    ["one paperback"] = [new("9780441013593", "Dune", 12.45m, 1, false)],
    ["two hardcovers"] = [new("9780261103344", "The Hobbit", 149.90m, 2, false)],
    ["ebook only"] = [new("9780141439587", "Emma", 59.90m, 1, true)],
    ["mixed"] = [new("9780261103344", "The Hobbit", 189.50m, 1, false), new("9780141439549", "Middlemarch", 129.90m, 1, true)]
};

[Fact]
public Task Calculate_for_all_carts() =>
    Combination()
        .Verify(
            (string cart, bool isExpress) => new CheckoutService().Calculate(Carts[cart], isExpress),
            Carts.Keys,
            [false, true]);

  one paperback , False: {
    Subtotal: 12.45,
    Vat: 1.25,
    Shipping: 49.90,
    Total: 63.60
  },

Snapshot onaylandıktan sonra ilk commit yalnız bu test. Bundan sonraki her adım bu testi bir saniyenin altında çalıştırıyor.

Rename Variable ve Rename Field: Önce İsimler

Rename Variable, Mysterious Name smell'ine verilen ilk cevap. t, v, p, s, l ve lt; subtotal, vat, hasPrintedBooks, shipping, line ve lineTotal oluyor. Mekanik adımları basit: değişkenin bütün kullanımlarını bul, hepsini birlikte değiştir, derle. Değişkenin kapsamı ne kadar darsa adım o kadar güvenli; bir method'un içindeki local değişkeni IDE tek hamlede ve hatasız değiştirir.

p'nin yeni adına biraz düşünmek gerekti. Değişken "en az bir basılı kitap var mı" sorusunu tutuyor; physical da olabilirdi, ama kargonun neden hesaplandığını hasPrintedBooks daha iyi anlatıyor. İyi bir isim bulmak kodu anladığını gösteren ilk işaret; bulamıyorsan o değişken muhtemelen birden fazla iş yapıyordur (Split Variable, bir sonraki yazıda).

Rename Field aynı işin class seviyesindeki hâli. Private bir field için risk local değişkenle aynı. Public bir field ya da property ise başka bir hikâye: adı JSON alanına, Entity Framework Core (EF Core; .NET'in veritabanı erişim kütüphanesi) kolonuna ya da bir Razor view'daki asp-for ifadesine dönüşmüş olabilir. Üçüncü yazıda rename'in JSON contract'ını sessizce bozduğunu, yedinci yazıda da kod içi adı dış contract'tan nasıl ayıracağımızı gördük; burada tekrar etmiyoruz. Kural aynı: public bir isim değişiyorsa önce o ismin kod dışında nerede yaşadığını bul.

Inline Variable ve Inline Function: Önce Geri Toplamak

Extract ile başlamak doğal görünür, ama bu method'da önce ters yöne gitmek gerekiyor. result değişkeni hiçbir şey anlatmıyor: oluşturulup bir sonraki satırda döndürülüyor. Inline Variable, ifadesi zaten kendi adı kadar açık olan bir değişkeni kaldırır: değişkenin tek atandığından emin ol, kullanımlarını ifadenin kendisiyle değiştir, tanımı sil.

Kargo hesabı daha ilginç. Mantık iki küçük method'a bölünmüş: GetShippingBase eşiği kontrol ediyor, AddExpress ekspres ücretini ekliyor. İki method da tek başına anlamsız, birlikte okununca bir anlam taşıyor; okur kargo kuralını görmek için üç yer arasında gidip geliyor. Inline Function, gövdesi adından daha açık olan ya da kötü bölünmüş fonksiyonları çağrıldıkları yere geri koyar. Mekanik adımları: method'un polymorphic olmadığını (override edilmediğini) kontrol et, her çağrı noktasını gövdeyle değiştir, her değiştirmeden sonra test et, sonunda tanımı sil.

         decimal shipping = 0;
         if (hasPrintedBooks)
         {
-            shipping = GetShippingBase(subtotal);
-            shipping = AddExpress(shipping, exp);
+            if (subtotal >= 300)
+                shipping = 0;
+            else
+                shipping = 49.90m;
+            if (exp)
+                shipping = shipping + 35;
         }

Bu adımın amacı kodu güzelleştirmek değil; kargo kuralını tek yerde, bütün hâliyle görmek. Birazdan doğru sınırlarla yeniden çıkaracağız. Kötü bölünmüş bir kodu düzeltmenin en kısa yolu çoğu zaman bu: önce hepsini inline et, sonra yeniden extract et.

Replace Magic Literal ve Replace Inline Code with Function Call

Gövdede beş çıplak sayı var: 0.20m, 0.10m, 300, 49.90m ve 35. Replace Magic Literal her birine bir ad verir: EbookVatRate, PrintedBookVatRate, FreeShippingThreshold, StandardShippingFee, ExpressSurcharge. Burada private const yeterli. İki uyarı: Math.Round(x, 2)'deki 2 ya da * 100 gibi anlamı kendinden belli sayılara ad vermek kodu uzatır ama açıklamaz. Ayrıca C#'ta public bir const, onu kullanan assembly'lerin içine derleme anında kopyalanır; değeri değiştirdiğinde tekrar derlenmeyen çağıranlar eski değerle çalışmaya devam eder. Değişmesi muhtemel bir iş kuralıysa (vergi oranı gibi) static readonly ya da configuration daha doğru yer.

Sonra tahmin sorusundaki adım geliyor. Replace Inline Code with Function Call, aynı işi yapan bir fonksiyon zaten varken elle yazılmış kodu o fonksiyonun çağrısıyla değiştirir; çoğu zaman bu fonksiyon .NET'in kendisinde vardır. Elle yazılmış yuvarlama Math.Round'un işini yapıyor gibi görünüyor. Değiştirip testi çalıştırıyoruz:

6c6
<     Vat: 1.25,
---
>     Vat: 1.24,
8c8
<     Total: 63.60
---
>     Total: 63.59

Cevap: Yarım Kuruş ve MidpointRounding

Snapshot'ta tek kitaplık sepetin iki kombinasyonu kırmızı; yukarıdaki fark ekspressiz olanınki, ekspresli olanda da aynı bir kuruş var. 12,45 TL'nin yüzde 10'u 1,245. Elle yazılmış kod sıfırdan uzağa yuvarlıyor ve 1,25 veriyor. Math.Round(decimal, int) ise varsayılan olarak MidpointRounding.ToEven kullanır: tam ortadaki değeri çift rakama, yani 1,24'e yuvarlar. Bir müşterinin faturası bir kuruş değişiyordu; bu bir refactoring değil, davranış değişikliği olurdu.

Doğru çağrı, eski kuralı açıkça söyleyen overload:

vat += Math.Round(lineTotal * PrintedBookVatRate, 2, MidpointRounding.AwayFromZero);

Test yeşile dönüyor. Ders Replace Inline Code with Function Call'un kendisiyle ilgili: fonksiyonun aynı işi yaptığına adından değil, davranışından karar verilir. Snapshot'a tam yarım kuruşa denk gelen bir fiyat koymasaydık bu fark üretimde, bir müşteri şikâyetiyle ortaya çıkardı. İkinci yazıda gördüğümüz gibi ağın değeri, kenarları ne kadar iyi taradığıyla ölçülür.

Extract Function ve Extract Variable

Artık kod geri toplanmış, isimleri ve sabitleri yerinde. Extract Function, Long Function ve Duplicated Code smell'lerinin ana cevabı: bir kod parçasına niyetini anlatan bir ad verip kendi fonksiyonuna taşımak. Mekanik adımları: yeni fonksiyonu niyetine göre adlandır (nasıl yaptığına göre değil), kodu kopyala, parçanın kullandığı local değişkenleri parametre yap, parçanın değiştirdiği değişkeni dönüş değeri yap, orijinal yeri çağrıyla değiştir, test et.

Üç fonksiyon çıktı: satır tutarını hesaplayan LineTotal, satırın KDV'sini veren VatFor ve kargo kuralını tek yerde toplayan ShippingFee. VatFor'un içindeki iki dallı if'i Extract Variable ile sadeleştirdim: oran ayrı bir değişkene çıkınca iki yuvarlama satırı bire indi. Aynı şekilde ShippingFee'deki eşik karşılaştırması qualifiesForFreeShipping adını aldı. Extract Variable, karmaşık bir ifadenin bir parçasına ad vererek okura "bu ne demek" sorusunun cevabını verir; ifade class'ın başka yerlerinde de anlamlıysa değişken yerine bir method ya da property daha iyi olabilir (Replace Temp with Query, bir sonraki yazıda).

public CheckoutTotals CalculateTotals(IReadOnlyList<CartLine> lines, bool isExpress)
{
    decimal subtotal = 0;
    decimal vat = 0;
    bool hasPrintedBooks = false;
    foreach (CartLine line in lines)
    {
        subtotal += LineTotal(line);
        vat += VatFor(line);
        if (!line.IsEbook)
            hasPrintedBooks = true;
    }

    decimal shipping = hasPrintedBooks ? ShippingFee(subtotal, isExpress) : 0;

    return new CheckoutTotals(subtotal, vat, shipping, subtotal + vat + shipping);
}

private static decimal ShippingFee(decimal subtotal, bool isExpress)
{
    bool qualifiesForFreeShipping = subtotal >= FreeShippingThreshold;
    decimal fee = qualifiesForFreeShipping ? 0 : StandardShippingFee;
    return isExpress ? fee + ExpressSurcharge : fee;
}

private static decimal LineTotal(CartLine line) => line.UnitPrice * line.Quantity;

private static decimal VatFor(CartLine line)
{
    decimal rate = line.IsEbook ? EbookVatRate : PrintedBookVatRate;
    return Math.Round(LineTotal(line) * rate, 2, MidpointRounding.AwayFromZero);
}

Döngü hâlâ üç iş yapıyor: tutar topluyor, KDV topluyor ve basılı kitap arıyor. Onu bilerek bıraktım; döngüyü bölmek ve LINQ'e (.NET'in collection'lar üzerinde sorgu yazma API'si) çevirmek on üçüncü yazının konusu. Ne kadar extract edileceği de bir karar: her satırı bir method'a çıkarmak okuru bu kez method'lar arasında gezdirir. Clean Code serisinin "Clean Code Tartışması: Ousterhout, Muratori ve Kuralların Sınırları" yazısı bu tartışmayı ayrıntılı ele alıyor; pratik ölçü, çıkardığın fonksiyonun adının gövdesinden daha çok şey anlatıp anlatmadığı.

Change Function Declaration: İmza Bir Contract'tır

Son adım imza: Calculate(List<CartLine> lines, bool exp), CalculateTotals(IReadOnlyList<CartLine> lines, bool isExpress) oluyor. Katalogdaki Change Function Declaration; method'u yeniden adlandırmayı, parametre eklemeyi, çıkarmayı ya da değiştirmeyi aynı başlıkta toplar ve iki yol tanımlar: basit yol (bütün çağıranları tek seferde değiştir) ve migration yolu (yeni imzayı ekle, eskisini ona yönlendir, çağıranları taşı, eskisini sil).

Basit yolun nerede kırıldığını görmek için yalnız parametrenin adını değiştirdim, exp yerine isExpress. Kulağa en zararsız değişiklik gibi geliyor. Ama test projesinde ekspres kargoyu sınayan bir test, method'u named argument ile çağırıyordu:

$ dotnet build
tests/Bookstore.Ordering.Tests/CheckoutServiceTests.cs(27,81): error CS1739: The best overload for 'Calculate' does not have a parameter named 'exp' [tests/Bookstore.Ordering.Tests/Bookstore.Ordering.Tests.csproj]

Aynı solution içinde IDE'nin rename'i bu çağrıyı da günceller. Ama method bir NuGet paketindeyse ya da başka bir ekibin repo'sundan çağrılıyorsa, parametre adı da public contract'ın parçasıdır ve onu değiştirmek o çağıranların build'ini kırar. Bu yüzden public method'larda migration yolunu kullanıyoruz; üçüncü yazıda ayrıntılı gördüğümüz parallel change'in aynısı:

[Obsolete("Use CalculateTotals(IReadOnlyList<CartLine>, bool) instead.")]
public CheckoutTotals Calculate(List<CartLine> lines, bool exp) => CalculateTotals(lines, exp);

public CheckoutTotals CalculateTotals(IReadOnlyList<CartLine> lines, bool isExpress)
{
    // ... gövde

$ dotnet build --no-incremental
tests/Bookstore.Ordering.Tests/CheckoutServiceTests.cs(20,50): warning CS0618: 'CheckoutService.Calculate(List<CartLine>, bool)' is obsolete: 'Use CalculateTotals(IReadOnlyList<CartLine>, bool) instead.' [tests/Bookstore.Ordering.Tests/Bookstore.Ordering.Tests.csproj]
tests/Bookstore.Ordering.Tests/CheckoutServiceTests.cs(27,33): warning CS0618: 'CheckoutService.Calculate(List<CartLine>, bool)' is obsolete: 'Use CalculateTotals(IReadOnlyList<CartLine>, bool) instead.' [tests/Bookstore.Ordering.Tests/Bookstore.Ordering.Tests.csproj]

Parametre tipinin List'ten IReadOnlyList'e genişlemesi de bilinçli: method listeyi değiştirmiyor, yalnız okuyor. Daha geniş bir tip kabul etmek çağıranları kırmaz; tersi, yani daha dar bir tip istemek, kırar. Bütün adımlar şu commit'lerde yapıldı; hiçbiri kırmızı commit edilmedi:

$ git log --oneline
40ca750 refactor: remove obsolete Calculate (contract)
9055a06 refactor: tests call CalculateTotals (migrate)
6b56c2a refactor: introduce CalculateTotals, obsolete Calculate (expand)
3d2acde test: express surcharge above free shipping threshold
bee2752 refactor: extract ShippingFee and variable qualifiesForFreeShipping
5aae24e refactor: extract variable rate in VatFor
aad204b refactor: extract LineTotal and VatFor
69e44e3 refactor: replace hand-written rounding with Math.Round(AwayFromZero)
b9a85af refactor: replace magic literals with named constants
43b2616 refactor: inline GetShippingBase and AddExpress
c25ad02 refactor: inline variable result
6ed7c6f refactor: rename locals in Calculate
15558c4 test: characterize CheckoutService.Calculate (10 combinations)

IDE Ne Yapar, Ne Yapamaz?

Aşağıdaki tablo bu yazıdaki dokuz refactoring'in IDE karşılıklarını özetliyor. IDE sütunları Microsoft Learn ve JetBrains'in resmi yardım sayfalarına dayanıyor; bu yazı için IDE'leri değil, yalnız komut satırını çalıştırdım. Visual Studio'da bütün menülerin girişi Ctrl+. (Quick Actions and Refactorings); belgelerde Rename için Ctrl+R, Ctrl+R, Extract Method için Ctrl+R, Ctrl+M ve Change Signature için Ctrl+R, Ctrl+V kısayolları geçiyor. Rider'da hepsi "Refactor This" menüsünde; kısayollar seçili keymap'e göre değişiyor. Visual Studio Code'da C# Dev Kit aynı Roslyn refactoring'lerini Ctrl+. ve Ctrl+Shift+R ile açıyor; resmi sayfası Inline method, Inline temporary variable ve Introduce local variable gibi adları listeliyor. Visual Studio belgeleri bir literal için yalnız local değişken seçeneğini anlatıyor; Rider'ın Introduce Field'ı ise "Introduce constant" seçeneğiyle doğrudan const üretebiliyor.

IDE'nin bu refactoring'leri güvenli yapmasının nedeni Roslyn'in semantik modeli: Extract Method parçanın hangi değişkenleri okuyup yazdığını hesaplar, Inline method çağrı noktasındaki argümanları doğru yere koyar. Göremediği şeyler de üçüncü yazıdakiyle aynı: reflection ile string üzerinden bulunan method'lar, asp-action="Calculate" gibi Razor tag helper'larında string olarak duran adlar ve solution dışındaki çağıranlar. Replace Inline Code with Function Call'un hiçbir IDE'de genel bir karşılığı yok, çünkü iki kodun aynı işi yaptığına karar vermek semantik değil, davranışsal bir karar; bunu ancak testler söyler.

Sık Düşülen Tuzaklar

Extract Function'ı hedef hâline getirmek. "Hiçbir method 10 satırı geçmeyecek" diye başlanan iş, her biri iki satırlık ve adı gövdesinden bir şey fazla söylemeyen method'larla biter. Bu yazıda önce iki küçük method'u inline etmemiz tam olarak bu sorunun düzeltmesiydi.

Benzer görünen fonksiyonun aynı olduğunu varsaymak. Math.Round ile elle yazılmış yuvarlama, string.Equals ile == (culture ve case farkı), DateTime.Now ile DateTime.UtcNow. Replace Inline Code with Function Call'dan önce kenardaki değerleri (tam ortadaki sayı, boş string, null) ağa koy.

Public bir imzayı tek commit'te değiştirmek. Method adı, parametre tipi, hatta parametre adı (named argument kullanan çağıranlar için) public contract'ın parçası. Solution içinde IDE hepsini düzeltir, solution dışında ise [Obsolete] ile genişlet, taşı, daralt.

Magic literal'i yanlış yere taşımak. Public bir const çağıranlara kopyalanır; değişebilen iş kurallarını const yapmak, değer değiştiğinde iki farklı davranış üretir.

Rename sonrası düz metin aramayı atlamak. Değişkeni değil de public bir field ya da property'yi yeniden adlandırdıysan eski adı solution genelinde metin olarak ara: JSON, CSV (virgülle ayrılmış düz metin dosya) başlıkları, log alanları ve view'lar.

Küçük Bir Deneme

★☆☆ Kendi kodunda elle yazılmış bir yuvarlama, aralık kontrolü ya da string birleştirme bul. .NET'te aynı işi yapan bir method var mı? Varsa, tam ortadaki ya da sınırdaki bir değerle ikisini karşılaştıran küçük bir test yaz.

★★☆ Bu yazıdaki eski Calculate'i kendi projende kur ve snapshot'ı onayla. GetShippingBase ve AddExpress'i inline etmeden doğrudan Extract Function ile başlamayı dene. Kargo kuralı tek yerde toplanabildi mi, kaç adım sürdü?

★★★ Ekibinin bir NuGet paketindeki public method'unda bir parametre adını değiştirmen gerektiğini düşün. Named argument kullanan çağıranları bulmak için hangi aramayı yaparsın? Migration yolunu (yeni imza, [Obsolete], taşıma, silme) kaç sürüme yayacağını bir issue'ya yaz.

CalculateTotals artık okunuyor, ama sepet satırları hâlâ dışarıdan değiştirilebilen bir List, ISBN (kitapların uluslararası standart numarası) hâlâ bir string ve toplamlar her çağrıda yeniden hesaplanıyor mu yoksa bir field'da mı saklanıyor, koda bakmadan bilemiyorsun. Bir sonraki yazıda veriyi kapsülleyen refactoring'lere geçiyoruz: Encapsulate Variable, Encapsulate Record ve Encapsulate Collection, Replace Primitive with Object, Replace Temp with Query ve value ile reference arasındaki geçiş.