Refactoring Kataloğu 4: Move Function/Field, Move ve Slide Statements, Split Loop, Replace Loop with Pipeline ve Remove Dead Code

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, sorgular Entity Framework Core 10.0.15 (SQL Server provider) ile derlenip denenmiştir.

Önce Tahmin Et: Kuralı Entity'ye Taşıyınca Sorgu Ne Olur?

Katalog bloğunun dördüncü yazısı bir şeyi bir yerden başka bir yere taşıyan refactoring'leri ele alıyor: Move Function, Move Field, Move Statements into Function, Move Statements to Callers, Slide Statements, Split Loop, Replace Loop with Pipeline ve Remove Dead Code. Adlar refactoring.com/catalog'daki gibi; açıklama ve örnekler bizim. Sahne, sitedeki serilerin ortak örnek uygulaması Bookstore'un (bir online kitapçı) Inventory module'ündeki stok raporu:

public class StockReport(Dictionary<string, int> reorderLevels)
{
    private const bool UseLegacyHeader = false;
    private int _lastItemCount;

    public string Build(List<StockItem> items)
    {
        StringBuilder sb = new();
        decimal totalValue = 0;
        List<StockItem> reorder = [];
        Dictionary<string, int> perWarehouse = [];
        _lastItemCount = items.Count;
        foreach (StockItem item in items)
        {
            totalValue += item.Quantity * item.UnitCost;
            if (IsBelowReorderLevel(item))
                reorder.Add(item);
            if (!perWarehouse.ContainsKey(item.Warehouse))
                perWarehouse[item.Warehouse] = 0;
            perWarehouse[item.Warehouse] += item.Quantity;
        }

        if (UseLegacyHeader)
            sb.AppendLine("STOCK REPORT");
        sb.AppendLine(FormattableString.Invariant($"Stock value: {totalValue:0.00}"));
        foreach (string warehouse in perWarehouse.Keys.Order())
            sb.AppendLine($"{warehouse}: {perWarehouse[warehouse]} copies");
        sb.AppendLine("Reorder:");
        foreach (StockItem item in reorder)
            sb.AppendLine("! " + FormatLine(item));
        return sb.ToString();
    }

    public string ReorderEmailBody(List<StockItem> items)
    {
        StringBuilder sb = new();
        sb.AppendLine("Please reorder:");
        foreach (StockItem item in items.Where(IsBelowReorderLevel))
            sb.AppendLine("! " + FormatLine(item));
        return sb.ToString();
    }

    private bool IsBelowReorderLevel(StockItem item)
    {
        int level = reorderLevels.TryGetValue(item.Isbn, out int configured) ? configured : 5;
        return item.Quantity < level;
    }

    private static string FormatLine(StockItem item) =>
        $"{item.Isbn} {item.Title} ({item.Warehouse}): {item.Quantity}";

    private static string FormatLineWithCost(StockItem item) =>
        FormattableString.Invariant($"{item.Isbn} {item.Title}: {item.Quantity} x {item.UnitCost:0.00}");
}

Yazının ortasında yeniden sipariş kuralını rapordan alıp StockItem'ın kendisine, NeedsReorder adlı bir property'ye taşıyacağız. Stok ekranı da bu kuralı veritabanında kullanmak istiyor: db.StockItems.Where(i => i.NeedsReorder). Bu sorgu Entity Framework Core (EF Core; .NET'in veritabanı erişim kütüphanesi) ile SQL Server'da çalışır mı? Çalışmazsa, hata mesajının önerdiği düzeltme ne yapar?

Move Statements into Function ve Slide Statements

Characterization testleri iki Verify snapshot'ı: raporun ve e-postanın tam metni. İlk adım küçük: iki çağıran da FormatLine'ı çağırmadan hemen önce aynı işi yapıyor, satırın başına "! " ekliyor. Move Statements into Function, bir fonksiyonun her çağrısından önce (ya da sonra) hep aynı satır geliyorsa o satırı fonksiyonun içine alır: tekrar eden satırı fonksiyona taşı, çağıranlardan sil, fonksiyonun adını yeni davranışa göre güncelle. Sonuç FormatReorderLine oldu ve "! " tek bir yerde yaşıyor. Tekrar eden satır bir fonksiyon çağrısının parçası olduğu için bu, Duplicated Code smell'inin en küçük hâli.

Slide Statements ise birbiriyle ilgili satırları yan yana getirir. Build'in başında dört değişken birden tanımlanıyor ve StringBuilder ancak döngüden sonra kullanılıyor. Bir değişkenin tanımını ilk kullanımının yanına kaydırmak hem okuru rahatlatır hem de sonraki adımları (Split Loop, Extract Function) mümkün kılar; birbirine karışmış kod parçaları bölünemez. Kaydırırken tek kural var: kaydırdığın satır, üzerinden geçtiği satırlarla bir yan etki paylaşıyorsa (aynı değişkeni yazıyorlarsa, biri bir method çağırıyor ve o method diğerinin okuduğu state'i değiştiriyorsa) sıra davranışın parçasıdır. Visual Studio'da "Move declaration near reference" bunun değişken tanımı için otomatik hâli.

Split Loop: Bir Döngü, Üç İş

Tek döngü üç ayrı şey hesaplıyordu: stok değeri, yeniden sipariş listesi ve depo başına adet. Split Loop döngüyü kopyalar ve her kopyada yalnız bir işi bırakır:

decimal totalValue = 0;
foreach (StockItem item in items)
    totalValue += item.Quantity * item.UnitCost;

List<StockItem> reorder = [];
foreach (StockItem item in items)
{
    if (IsBelowReorderLevel(item))
        reorder.Add(item);
}

Dictionary<string, int> perWarehouse = [];
foreach (StockItem item in items)
{
    if (!perWarehouse.ContainsKey(item.Warehouse))
        perWarehouse[item.Warehouse] = 0;
    perWarehouse[item.Warehouse] += item.Quantity;
}

Kod uzadı ve liste üç kez dolaşılıyor; ilk bakışta geriye bir adım gibi duruyor. Kazanç, her döngünün artık tek bir anlamı olması: her birini ayrı bir fonksiyona çıkarabilir ya da bir sonraki adımda olduğu gibi tek bir ifadeye çevirebilirsin. Split Loop'tan önce döngünün gövdesindeki işlerin birbirinden bağımsız olduğundan emin ol; bir iş öbürünün ara sonucunu kullanıyorsa (örneğin break ile erken çıkılıyorsa ya da bir sayaç diğer işin koşulunda okunuyorsa) bölmek davranışı değiştirir.

Replace Loop with Pipeline ve Performans

Her biri tek işe inen döngüler LINQ'e (.NET'in collection'lar üzerinde sorgu yazma API'si) çevrilmeye hazır. Replace Loop with Pipeline, döngünün yaptığı filtreleme, dönüştürme ve toplama işlerini bir dizi operasyona çevirir; Clean Code serisinin "Code Smell Kataloğu" yazısındaki Loops smell'inin cevabı:

decimal totalValue = items.Sum(i => i.Quantity * i.UnitCost);
List<StockItem> reorder = items.Where(IsBelowReorderLevel).ToList();
Dictionary<string, int> perWarehouse = items
    .GroupBy(i => i.Warehouse)
    .ToDictionary(g => g.Key, g => g.Sum(i => i.Quantity));

Mekanik adımlar: döngünün üzerinde gezdiği collection için bir değişken aç, döngü gövdesindeki her işi sırayla bir operasyona çevir (if ise Where, dönüşüm ise Select, toplama ise Sum ya da Aggregate), her operasyondan sonra test et, döngüyü sil. Visual Studio'nun foreach döngüsünü LINQ'e çeviren bir refactoring'i var (Ctrl+. menüsünde "Convert to LINQ" ve "Convert to Linq (call form)"); belgesi aynı zamanda LINQ'in genelde bir foreach döngüsünden daha az verimli olduğunu not ediyor.

Bu not önemli. Yukarıdaki kod listeyi üç kez dolaşıyor, delegate çağırıyor ve GroupBy ara bir yapı kuruyor. Bir stok raporu için bu fark ölçülebilir bir şey değil: rapor günde birkaç kez, birkaç bin satır üzerinde çalışıyor. Ama her HTTP isteğinde çalışan ve günde milyonlarca kez çağrılan bir hesaplamada, yani hot path'te (kodun en sık çalışan, performansı belirleyen kısmı) durum farklı olabilir. Kararı tahminle değil ölçümle ver; LINQ'in maliyetinin nerede önemli olduğunu ve BenchmarkDotNet ile nasıl ölçüleceğini C# LINQ serisinin "LINQ Performansı: Çoklu Enumeration, Materialize ve BenchmarkDotNet ile Ölçmek" yazısı anlatıyor. Bu yazı için benchmark çalıştırmadım; rapor kodu okunurluk için pipeline'da kaldı.

Move Field ve Move Function: Veri ve Davranış Aynı Yere

IsBelowReorderLevel yalnız StockItem'ın verisiyle çalışıyor; tek yabancı veri, raporun constructor'ına verilen ve ISBN'e (kitapların uluslararası standart numarası) göre tutulan seviye sözlüğü. Bu, Clean Code serisindeki Feature Envy smell'i: method başka bir class'ın verisine kendi class'ınınkinden daha çok ilgi duyuyor. Ama önce veriyi taşımak gerekiyor.

Move Field, bir alanı onu en çok kullanan ya da onunla birlikte değişen class'a taşır. Yeniden sipariş seviyesi bir kitabın stok kaydının özelliği, raporun değil. Mekanik adımlar: hedef class'ta alanı aç (varsayılan değeriyle; sözlükte olmayan kitaplar için 5), kaynağı dolduran yerleri hedefi de dolduracak şekilde değiştir, okuyanları hedefe çevir, kaynağı sil. Gerçek uygulamada bu bir EF Core migration'ı da demek: StockItems tablosuna yeni bir kolon ve eski configuration'daki değerlerin bu kolona taşınması. Yani Move Field kodda bir dakika, veride ise yedinci yazıdaki expand-contract adımlarıyla yürütülen bir iş.

Veri taşındıktan sonra Move Function doğal: method'u hedef class'a kopyala, kaynakta ona yönlenen ince bir çağrı bırak, test et, çağıranları hedefe çevir, kaynaktakini sil.

public class StockItem
{
    public int Id { get; set; }
    public string Isbn { get; set; } = "";
    public string Title { get; set; } = "";
    public string Warehouse { get; set; } = "";
    public int Quantity { get; set; }
    public decimal UnitCost { get; set; }
    public int ReorderLevel { get; set; } = 5;

    public bool NeedsReorder => Quantity < ReorderLevel;
}

Rapor artık items.Where(i => i.NeedsReorder) diyor ve constructor parametresi kalmadı. Snapshot'lar değişmedi. Sırada stok ekranının sorgusu var.

Cevap: Önce Exception, Sonra Bütün Tablo

Sorguyu bir veritabanına bağlanmadan denemek için EF Core'un ToQueryString() method'unu kullandım; SQL'i üretir ama bağlantı açmaz. Sonuç:

System.InvalidOperationException : The LINQ expression 'DbSet<StockItem>()
    .Where(s => s.NeedsReorder)' could not be translated. Additional information: Translation of member 'NeedsReorder' on entity type 'StockItem' failed. This commonly occurs when the specified member is unmapped. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

EF Core sorguyu C# olarak değil, bir expression tree olarak görür ve SQL'e çevirir. NeedsReorder'ın gövdesi derlenmiş bir getter; EF Core içine bakamaz ve kolon olarak map edilmemiş bir property'yi SQL'e çeviremez. Move Function, C# tarafında saf bir refactoring'di; sorgu tarafında ise davranışı değiştirdi.

Hata mesajının önerdiği ikinci yol, AsEnumerable() ya da ToList() ile filtrelemeyi istemciye almak. Bu çalışır, ama nasıl çalıştığına bakmak gerek. AsEnumerable()'dan önceki kısmın ürettiği SQL'de filtre yok, bütün tablo istemciye geliyor:

SELECT [s].[Id], [s].[Isbn], [s].[Quantity], [s].[ReorderLevel], [s].[Title], [s].[UnitCost], [s].[Warehouse]
FROM [StockItems] AS [s]

Kuralı sorgunun içine, bir lambda olarak yazınca ise filtre SQL'e iniyor:

SELECT [s].[Id], [s].[Isbn], [s].[Quantity], [s].[ReorderLevel], [s].[Title], [s].[UnitCost], [s].[Warehouse]
FROM [StockItems] AS [s]
WHERE [s].[Quantity] < [s].[ReorderLevel]

İlki geliştirme ortamındaki yüz satırlık tabloda fark edilmez; üretimdeki yüz binlerce satırlık stok tablosunda her ekran açılışında bütün tabloyu uygulama sunucusuna taşır. Cevap ikisi de değil: kuralı EF Core'un okuyabileceği bir expression olarak tek yerde tanımlayıp hem sorguda hem property'de kullanmak.

public static readonly Expression<Func<StockItem, bool>> NeedsReorderExpression =
    i => i.Quantity < i.ReorderLevel;

private static readonly Func<StockItem, bool> NeedsReorderRule = NeedsReorderExpression.Compile();

public bool NeedsReorder => NeedsReorderRule(this);

// Sorgu tarafı
string sql = db.StockItems.Where(StockItem.NeedsReorderExpression).ToQueryString();

Testler üç şeyi sabitliyor: expression'lı sorgunun WHERE [s].[Quantity] < [s].[ReorderLevel] üretmesi, property'li sorgunun "could not be translated" ile reddedilmesi ve property ile expression'ın sınırda (adet seviyeye eşitken) aynı cevabı vermesi. SQL'i ToQueryString() ile ürettim; sorguyu gerçek bir SQL Server'da çalıştırmadım. Aynı tuzak on birinci yazıdaki Order.Total için de geçerli: sorguya dönüşen bir property'yi entity'ye taşımadan önce o property'nin bir sorguda kullanılıp kullanılmadığını ara.

Move Statements to Callers: Yeni Bir Çağıran Gelince

Depo ekibi bir toplama listesi istedi: bir depodaki kitaplar, aynı satır biçimiyle ama "!" işareti olmadan. FormatReorderLine bu çağıran için yanlış bir şey yapıyor. Move Statements to Callers, Move Statements into Function'ın tersi: çağıranlar artık aynı şeyi istemiyorsa, fonksiyonun sonundaki ya da başındaki satırı çağıranlara geri dağıt. Fonksiyona bir bool addMark parametresi eklemek daha kısa görünür, ama bu dördüncü yazıda böldüğümüz türden bir flag parametresi olurdu.

Birinci yazıdaki iki şapka kuralı burada da geçerli. Önce davranışı değiştirmeyen bir refactor: commit'i: "! " iki çağırana geri döndü, fonksiyonun adı yine FormatLine. Sonra ayrı bir feat: commit'i yeni özelliği ekledi:

public string PickList(IReadOnlyList<StockItem> items, string warehouse)
{
    StringBuilder sb = new();
    sb.AppendLine($"Pick list for {warehouse}:");
    foreach (StockItem item in items.Where(i => i.Warehouse == warehouse))
        sb.AppendLine(FormatLine(item));
    return sb.ToString();
}

Özelliği eklemeden önce yapılan bu küçük hazırlık, Kent Beck'in sık alıntılanan fikrinin günlük hâli: istenen değişikliği önce kolaylaştırmak, sonra kolaylaşan değişikliği yapmak.

Remove Dead Code: Analyzer, Coverage ve Reflection

Raporda üç ölü parça var: hiç çağrılmayan FormatLineWithCost, yazılıp hiç okunmayan _lastItemCount ve false bir sabitle korunan eski başlık. Derleyici yalnız sonuncusunu varsayılan olarak söylüyor (CS0162). İlk ikisi için IDE0051 ve IDE0052 kuralları var, ama bunlar code style kuralları ve dotnet build'de çalışmaları için iki ayar gerekiyor: .editorconfig'te önem derecesi ve EnforceCodeStyleInBuild. İkisini açınca (satırlar kısaltıldı):

$ dotnet build --no-incremental -p:EnforceCodeStyleInBuild=true
src/Bookstore.Inventory/StockReport.cs(22,13): warning CS0162: Unreachable code detected
src/Bookstore.Inventory/StockReport.cs(53,27): warning IDE0051: Private member 'StockReport.FormatLineWithCost' is unused
src/Bookstore.Inventory/StockReport.cs(8,17): warning IDE0052: Private member 'StockReport._lastItemCount' can be removed as the value assigned to it is never read

Ayarı sonra repo kökündeki Directory.Build.props'a taşıdım, böylece her build bu kuralları çalıştırıyor. Düzeltmeyi de komut satırından denedim. dotnet format style --diagnostics IDE0051 IDE0052, IDE0051 için Roslyn'in code fix'ini uygulayıp method'u sildi; IDE0052 için ise şunu söyledi:

$ dotnet format style src/Bookstore.Inventory --diagnostics IDE0051 IDE0052
Unable to fix IDE0052. No associated code fix found.

Bu mantıklı: okunmayan bir field'ı silmek, ona yazan satırı da silmeyi gerektirir ve o satırın bir yan etkisi olabilir. Field'ı, atamayı ve sabitle korunan dalı elle sildim. Remove Dead Code'un mekanik adımları kısa: kodun gerçekten çağrılmadığını doğrula, sil, test et. Silinen kod git geçmişinde duruyor; "belki lazım olur" diye yorum satırına almak gerekmiyor.

Zor olan, doğrulama adımı. IDE0051 yalnız private üyeleri bilir. Public bir method ya da class için "kullanılmıyor" demek, onu reflection ile, convention tabanlı DI (dependency injection; bağımlılıkları dışarıdan veren mekanizma) kaydıyla, bir Razor view'dan, bir serializer'dan ya da başka bir repo'dan kimsenin çağırmadığını bilmek demek. Üçüncü yazıda adı "Rule" ile bitmeyen bir class'ın DI container'ından sessizce düştüğünü görmüştük; aynı class'ı "referansı yok" diye silmek de aynı sonucu verir. İkinci yazıdaki coverage raporu burada yardımcı: hiç çalışmayan satırlar aday, ama kesin değil, çünkü testlerin kapsamadığı her kod ölü değildir. Üretimdeki çağrıları görmek için loglar ya da metrikler daha güvenilir. Feature flag kalıntıları da tipik ölü kod: flag her yerde açıldıktan sonra kapalı dal, UseLegacyHeader gibi, kimse okumadan yıllarca yaşar.

IDE Desteği ve Bütün Adımlar

IDE sütunu Microsoft Learn ve JetBrains belgelerine dayanıyor; IDE'leri bu yazı için çalıştırmadım. Analyzer ve dotnet format satırları yukarıdaki gerçek çıktılardan.

$ git log --oneline
a611871 refactor: StockReport accepts IReadOnlyList (CA1002)
371be39 refactor: remove dead code (IDE0051, IDE0052, CS0162)
acb84f8 feat: warehouse pick list
1b4f2c6 refactor: move the '! ' prefix back to the callers
8a63ec4 refactor: reorder rule as an expression EF Core can translate
ff584ac refactor: move IsBelowReorderLevel to StockItem.NeedsReorder
333a09b refactor: move reorder level from StockReport to StockItem
187644b refactor: replace the three loops with LINQ pipelines
65d00ad refactor: split the stock loop into three loops
e4ae4f4 refactor: slide statements in Build (StringBuilder next to the output)
3ec9a3e refactor: move the '! ' prefix into FormatReorderLine
3e83e1a test: characterize StockReport (legacy)

Son commit, CA1002 uyarısına cevap olarak Build ve ReorderEmailBody'nin parametresini List'ten IReadOnlyList'e genişletiyor; onuncu yazıdaki Change Function Declaration'ın küçük bir tekrarı.

Sık Düşülen Tuzaklar

Move Function'da sorgunun istemciye kayması. Bir koşulu entity'nin property'sine ya da bir method'a taşıyınca EF Core onu SQL'e çeviremez; hata mesajının önerdiği AsEnumerable() çoğu zaman bütün tabloyu belleğe çeker. Kural sorguda da kullanılıyorsa bir Expression olarak tanımla ve testte ToQueryString() ile SQL'i doğrula.

Dead code sanılan kodu silmek. Reflection, DI convention'ı, serializer, Razor view ya da başka bir repo üzerinden çağrılan public kod hiçbir analyzer'da "kullanılmıyor" görünmez. Silmeden önce adı string olarak ara ve üretim loglarına bak.

Replace Loop with Pipeline'ı her yere uygulamak. Okunurluk kazancı çoğu döngüde gerçek, ama sık çağrılan bir hesaplamada ölçmeden çevirmek performans kaybı riskini körlemesine almak demek. Tersini de yapma: ölçmeden "LINQ yavaştır" deyip her şeyi döngüde bırakmak.

Slide Statements'ta yan etkiyi gözden kaçırmak. İki satırın sırası, aynı state'i değiştiriyorlarsa davranışın parçasıdır. Bir log satırını ya da bir sayaç artışını yukarı kaydırmak çıktıyı değiştirebilir.

Move Field'ı yalnız kodda yapmak. Taşınan alan bir kolonsa migration, veri taşıma ve eski okuyucular da işin parçası; yedinci yazıdaki expand-contract burada da geçerli.

Küçük Bir Deneme

★☆☆ Kendi projende .editorconfig'e IDE0051 ve IDE0052'yi warning olarak ekle, EnforceCodeStyleInBuild'i aç ve build al. Kaç ölü üye çıktı? dotnet format style --diagnostics IDE0051 bunlardan kaçını otomatik silebildi?

★★☆ Birden fazla iş yapan bir döngü bul. Önce Slide Statements ve Split Loop uygula, sonra her döngüyü LINQ'e çevir. Her adımdan sonra testleri çalıştır. Hangi adımda bir şey kırıldı?

★★★ Bir entity'de hesaplanan bir property'nin bir EF Core sorgusunda kullanıldığı bir yer ara. ToQueryString() ile ürettiği SQL'e bak. Filtre SQL'de mi, yoksa sorgunun bir yerinde AsEnumerable() ya da ToList() ile istemciye mi kayıyor?

Taşıma ailesi de bitti: veri, davranış ve satırlar doğru yerde. Katalogda geriye üç aile kalıyor. Bir sonraki yazı koşulları sadeleştiren refactoring'lerle devam ediyor: Decompose Conditional, Consolidate Conditional Expression, Replace Nested Conditional with Guard Clauses, Replace Conditional with Polymorphism ve Introduce Special Case.