Code Smell Kataloğu: C# Kodunda Kokuyu Tanımak

Sürüm notu: Bu yazıdaki kodlar C# 14 ve .NET 10 ile derlenip denenmiştir.

Önce Tahmin Et: Bu Method'da Kaç Koku Var?

Seri boyunca temizlediğimiz legacy OrderService.cs'ten 25 satırlık bir method. Siparişi iptal ediyor:

public bool CancelOrder(int id, bool refund, bool notify)
{
    try
    {
        Order o = GetOrder(id);
        if (o == null) return false;
        if (o.Status == 3 || o.Status == 4) return false;
        if (o.Status == 9) return false;
        if (refund)
        {
            var r = pc.Refund(o.PaymentRef, (long)(o.Total * 100));
            if (r.Code != "00") { LastError = r.Message; return false; }
        }
        foreach (var oi in o.Items) oi.Book.Stock += oi.Qty;
        o.Status = 4;
        o.Customer.Points -= (int)(o.Total / 10);
        if (o.Customer.Points < 0) o.Customer.Points = 0;
        if (notify) Mailer.Send(o.Customer.Email, "Order #" + o.Id + " cancelled", "Your order has been cancelled.", false);
        return true;
    }
    catch (Exception)
    {
    }
    return false;
}

Okumaya devam etmeden önce say: burada kaç ayrı problem görüyorsun ve her birine bir isim verebiliyor musun? "Kötü kod" demek kolay; "iki flag argument, yutulan exception, magic number olarak status, static bir hata field'ı, başka bir object'in verisini değiştiren hesap" demek ise ekipte konuşulabilir, sıralanabilir ve düzeltilebilir bir liste. Code smell kataloğu tam olarak bu isimleri verir. Sayını not et; yazının sonunda aynı method'a tabloyla döneceğiz.

Code Smell Nedir, Ne Değildir?

Terim Kent Beck'ten geliyor. Martin Fowler'ın anlatımına göre Beck, Fowler'ın Refactoring kitabına (Refactoring: Improving the Design of Existing Code, 1999) yardım ederken bu ismi buldu. Fowler'ın 2006 tarihli bliki yazısındaki tanımı kısa: code smell, genellikle sistemde daha derin bir probleme karşılık gelen yüzeysel bir işarettir. Tanımın iki parçası da önemli. "Yüzeysel" çünkü koku hızlı fark edilir: 127 satırlık bir method'u görmek için domain'i bilmen gerekmez. "Genellikle" çünkü koku her zaman problem demek değildir; Fowler'ın kendi örneğiyle bazı uzun method'lar gayet iyidir. Koku bir teşhis değil, bakılması gereken yeri gösteren bir işaret.

Bu ayrım pratikte şunu söylüyor: smell listesi bir linter kural seti değildir. "Long Function gördüm, böldüm" refleksi iki yazı sonra konuşacağımız over-decomposition problemine götürür. Doğru soru "bu koku burada hangi değişikliği pahalı yapıyor?" sorusu.

Refactoring'in 1999'daki ilk baskısında 22 smell vardı. 2018'deki ikinci baskıda liste 24'e çıktı ve bazı isimler değişti: Long Method artık Long Function, Switch Statements artık Repeated Switches, Lazy Class artık Lazy Element, Inappropriate Intimacy artık Insider Trading. Mysterious Name, Global Data, Mutable Data ve Loops yeni geldi. Bu yazı ikinci baskının isimlerini kullanıyor; eski isimleri hâlâ birçok kaynakta göreceksin.

Beş Grup: Smell'leri Haritalamak

Fowler smell'leri gruplamaz, düz bir liste olarak verir. Bugün en çok kullanılan gruplama Mika Mäntylä'nın: önce 2003'teki yüksek lisans tezinde, sonra Casper Lassenius'la birlikte 2006'da Empirical Software Engineering dergisinde yayımladıkları çalışmada, 22 smell'i ortak etkilerine göre beş gruba ayırdılar. İkinci baskıda gelen dört yeni smell'i ve Comments'i bu yazıda bir gruba koymadım; aşağıdaki haritada "Diğer" kutusundalar.

Grupların değeri şu: aynı gruptaki smell'ler genelde aynı tür bedeli öder ve benzer refactoring'lerle çözülür. Bloaters okuma maliyetini artırır, OO (object-oriented) Abusers inheritance ve polymorphism'i yanlış yerde kullandırır, Change Preventers değişiklik maliyetini, Couplers bir class'ı değiştirince başka class'ların da değişmesini. Aşağıdaki tablo 24 smell'in her birini OrderService.cs'te arıyor:

Tabloda iki dürüst "yok" var. Her legacy dosyada her smell bulunmaz; kataloğu bir checklist gibi doldurmaya çalışmak da bir tuzak. Refactoring adları Fowler'ın refactoring.com'daki kataloğundakilerle aynı; adımların kendisi ilerideki Refactoring serisinin konusu.

Bloaters: Büyüyüp Yönetilemez Hâle Gelen Kod

Bloaters bir gecede oluşmaz; açılıştaki changelog comment'leri gibi, her ekleme "küçük bir değişiklik"tir. En görünür olanı Long Function: ProcessData 127 satır ve dört seviye iç içe if taşıyor. Bunu fonksiyonlar yazısında guard clause'larla ve Extract Function'la açtık.

Long Parameter List ile Data Clumps çoğu zaman birlikte gelir:

public int CreateCustomer(string name, string email, string street, string city, string zip, string country, string phone, string type, bool newsletter)

Dokuz parametrenin dördü (street, city, zip, country) her zaman birlikte geziyor. Birlikte gezen veri, adı konmamış bir kavramdır. Çağıran taraf city ile zip'in yerini karıştırırsa derleyici seni uyarmaz, ikisi de string. Çözüm o kavrama isim vermek; 14. yazıdaki temiz versiyonda bu iki record oldu:

public sealed record Address(string Street, string Zip, string City, string Country);

public sealed record NewCustomer(string Name, string Email, Address Address, CustomerTier Tier = CustomerTier.Normal, string Phone = "", bool Newsletter = false);

Primitive Obsession aynı sorunun tip seviyesindeki hâli: Customer.Type'ın "vip" olduğunu string karşılaştırmasıyla anlamak, seriyi açan Türkçe I bug'ını üretti. tr-TR kültüründe "vip".ToUpper() sonucu "VİP" olduğu için VIP indirimi sessizce kayboluyordu. enum ya da value object kullanınca bu karşılaştırma ortadan kalkar; ayrıntısı object ve data structure yazısında.

OO Abusers: OOP'yi Yarım Kullanmak

OO Abusers grubu OOP'nin (Object-Oriented Programming) araçlarının yanlış ya da yarım kullanıldığı yerleri toplar. Repeated Switches dosyada şöyle duruyor:

// ProcessData içinde
if (b.Type == "ebook")
    oi.Price = b.Price * 0.8m;
else if (b.Type == "audio")
    oi.Price = b.Price * 0.9m;
else
    oi.Price = b.Price;

// GetQuote içinde
decimal p = b.Price;
if (b.Type == "ebook") p = b.Price * 0.8m;
tmp += p * kv.Value;

Kitap tipine göre karar iki yerde veriliyor ve ikisi zaten ayrışmış: GetQuote sesli kitap indirimini unutmuş. Fowler'ın ikinci baskıda ismi "Switch Statements"tan "Repeated Switches"a çevirmesinin nedeni bu: tek bir switch sorun değil, aynı koşulun birden fazla yerde tekrarlanması sorun. Kataloğun önerdiği Replace Conditional with Polymorphism, tip başına davranış çoksa doğru yol. Burada davranış tek bir fiyat çarpanı olduğu için 14. yazıda daha küçük bir adım yeterli oldu: kararı tek bir yere toplamak.

public static decimal UnitPrice(Book book) => book.Format switch
{
    BookFormat.Ebook => book.Price * PricingRules.EbookPriceFactor,
    BookFormat.Audio => book.Price * PricingRules.AudioPriceFactor,
    _ => book.Price,
};

Refused Bequest bu dosyada yok, çünkü dosyada inheritance yok. Ama .NET'in kendi kütüphanesinde ders kitabı örneği var. ReadOnlyCollection<T>, IList<T>'yi implement ediyor ve miras aldığı sözleşmenin bir kısmını reddediyor:

using System.Collections.ObjectModel;

IList<string> isbns = new ReadOnlyCollection<string>(["978-0441013593", "978-0141439518"]);

Console.WriteLine(isbns.IsReadOnly);
try
{
    isbns.Add("978-0062316097");
}
catch (NotSupportedException ex)
{
    Console.WriteLine($"{ex.GetType().Name}: {ex.Message}");
}

True
NotSupportedException: Collection is read-only.

IList<T> üzerinden çalışan bir method, eline geçen listeye ekleme yapabileceğini varsayar ve çalışma zamanında NotSupportedException ile karşılaşır. BCL (Base Class Library) bunu IsReadOnly property'si ile yarı yarıya belgeliyor; ilerideki SOLID (Single Responsibility, Open/Closed, Liskov Substitution, Interface Segregation ve Dependency Inversion prensipleri) ve Design Principles serisinin Liskov Substitution yazısı tam olarak bu NotSupportedException'larla ilgili.

Change Preventers ve Dispensables

Change Preventers'ın iki üyesi birbirinin aynadaki görüntüsü. Divergent Change'de tek bir modül birçok farklı nedenle değişir: ProcessData'nın başındaki dört changelog satırı kupon, express kargo, VIP indirimi ve hafta sonu indirimi için aynı method'un değiştirildiğini kayda geçirmiş. Shotgun Surgery'de ise tek bir değişiklik birçok modüle dokunur: yeni bir kupon eklemek ProcessData ile GetQuote'u ayrı ayrı değiştirmeyi gerektiriyor ve birini unutmak, 1. yazıda gördüğümüz "sepet sayfasında başka, ödemede başka fiyat" bug'ını üretiyor. İkisinin ortak çaresi değişme nedenlerini ayırıp her nedeni tek bir yere toplamak; 14. yazıda fiyatlama OrderPricing class'ına bu yüzden taşındı.

Dispensables grubu silinse kimsenin aramayacağı kodları toplar:

// eski indirim hesabı - kullanılmıyor ama silme!
private decimal OldCalcDiscount(decimal total, int itemCount)
{
    if (itemCount > 3) return total * 0.07m;
    return 0;
}

//public int ProcessDataV2(Order o)
//{
//    var e = new PricingEngine();
//    return e.Run(o);
//}

// ileride farklı sipariş tipleri için
public interface IOrderProcessor<T> where T : class
{
    int Process(T item, bool flag);
}

Üç farklı koku tek ekranda. OldCalcDiscount ölü kod; "silme!" uyarısı nedenini söylemiyor, Git zaten geçmişi saklıyor. Yorum satırına alınmış ProcessDataV2 ölü kodun daha kötü bir hâli, çünkü derlenmediği için bozulduğunu kimse fark etmez. Dead Code ikinci baskının 24 smell'i arasında ayrı bir madde olarak yok, ama katalogda karşılığı olan bir refactoring var: Remove Dead Code. IOrderProcessor<T> ise Speculative Generality: "ileride" diye yazılmış, bir implementation'ı bile olmayan bir abstraction. Simple design yazısındaki dördüncü kural bu tür element'leri soruyordu.

Couplers ve Diğerleri

Couplers bir class'ın başka bir class'ın içini fazla bildiği yerlerdir. Feature Envy'nin en kısa tarifi şu: method, kendi class'ının değil başka bir class'ın verisiyle ilgileniyor. Açılıştaki CancelOrder müşterinin puanını kendisi hesaplayıp kendisi sınırlıyor; aynı işi Customer'ın kendisi yapabilir. Insider Trading de aynı yerde: OrderService başka bir object'in field'ını dışarıdan azaltıyor. Temiz versiyonda bu kural Customer'a taşındı:

public void ReturnPointsFor(Order order) =>
    Points = Math.Max(0, Points - (int)(order.Total / PricingRules.CurrencyPerPoint));

Message Chains, Law of Demeter ihlalinin koku hâlidir:

string label = o.Customer.Name + "\n" + o.Customer.Address.Street + "\n" + o.Customer.Address.Zip + " " + o.Customer.Address.City.Name + "\n" + o.Customer.Address.City.Country;

Kargo etiketi müşterinin adresinin şehrinin adına kadar iniyor. Address yapısı değişirse etiketi üreten kod da değişmek zorunda. Hide Delegate ile zinciri kısaltınca ve City class'ı adresin bir property'sine dönüşünce etiket kendi yerine çekildi:

public static class ShippingLabel
{
    public static string For(Customer customer, bool express)
    {
        Address address = customer.Address;
        string label = $"{customer.Name}\n{address.Street}\n{address.Zip} {address.City}\n{address.Country}";
        return express ? "*** EXPRESS ***\n" + label : label;
    }
}

Middle Man ise tersi bir problem: bir class'ın tek işi başka bir class'a iş devretmek. OrderService.SendMail yalnızca Mailer.Send'i çağırıyor ve bir de exception'ı yutuyor; çağıranlar doğrudan Mailer'ı kullansa hiçbir şey kaybedilmez.

"Diğer" kutusundakiler bu dosyanın her yerinde. Mysterious Name naming yazısının, Comments comment'ler yazısının konusuydu. Global Data seri boyunca en pahalı koku çıktı: static Db ve LastError yüzünden 14. yazıdaki characterization test'leri paralel çalışamadı. Mutable Data'nın en sinsi örneği IsValid: adı bir sorgu gibi okunuyor ama siparişin Status'unu değiştiriyor, yani Command-Query Separation'ı bozuyor.

Araçlar Hangi Kokuyu Görür?

Smell'lerin bir kısmını derleyici seviyesinde yakalamak mümkün. Aynı legacy dosyayı bir console projesine koyup SonarAnalyzer.CSharp NuGet paketini ekledim ve varsayılanda kapalı olan birkaç kuralı açtım:

root = true

[*.cs]
dotnet_diagnostic.CA1502.severity = warning
dotnet_diagnostic.CA1505.severity = warning
dotnet_diagnostic.CA1506.severity = warning
dotnet_diagnostic.S3776.severity = warning
dotnet_diagnostic.S107.severity = warning
dotnet_diagnostic.S138.severity = warning

Derleme 72 uyarı verdi. Smell'lerle eşleşenler:

OrderService.cs(31,20): warning CA1502: 'ProcessData' has a cyclomatic complexity of '27'. Rewrite or refactor the code to decrease its complexity below '26'.
OrderService.cs(31,20): warning S138: This method 'ProcessData' has 127 lines, which is greater than the 80 lines authorized. Split it into smaller methods.
OrderService.cs(31,20): warning S3776: Refactor this method to reduce its Cognitive Complexity from 92 to the 15 allowed.
OrderService.cs(252,41): warning S1168: Return an empty collection instead of null.
OrderService.cs(324,34): warning S107: Method has 9 parameters, which is greater than the 7 authorized.
OrderService.cs(439,25): warning S1144: Remove the unused private method 'OldCalcDiscount'.
OrderService.cs(446,9): warning S125: Remove this commented out code.
OrderService.cs(473,38): warning S3246: Add the 'in' keyword to parameter 'T' to make it 'contravariant'.

S3776 (Cognitive Complexity), S138 (method uzunluğu) ve CA1502 (cyclomatic complexity) Long Function'ı, S107 (parametre sayısı) Long Parameter List'i, S1144 (kullanılmayan private member) ve S125 (yorum satırındaki kod) ölü kodu buldu. S1168 (koleksiyon yerine null dönmek) bir smell değil ama error handling yazısının konusu. Geri kalan uyarıların büyük kısmı public field'lar ve static state ile ilgili: S1104 (public field) tek başına 21 kez çıktı, yani Data Class ve Global Data dolaylı olarak görünüyor.

Görmediklerine bak: Feature Envy, Shotgun Surgery, Divergent Change, Message Chains ve Duplicated Code için tek bir uyarı yok. En öğretici olanı S3246 (generic type parameter'ı contravariant yapma önerisi): analyzer, hiç implementation'ı olmayan IOrderProcessor<T>'yi silmeyi değil, daha iyi bir interface yapmak için in keyword'ünü eklemeyi öneriyor. Araçlar yerel kokuları, yani tek bir method'un ya da satırın içinde görünenleri iyi yakalar; tasarım seviyesindeki kokular birden fazla dosyaya ve değişim geçmişine bakmayı gerektirir. Kuralların kurulumu, eşikleri ve SonarQube ile ekip seviyesinde kullanımı ilerideki Statik Analiz serisinde.

Sık Düşülen Tuzaklar

Kokuyu teşhis sanmak. "Long Function" uyarısı method'u bölmen gerektiğini söylemez, bakman gerektiğini söyler. Bazı uzun method'lar düz ve okunaklı bir algoritmadır; bölmek okuyucuyu method'lar arasında zıplatır.

Kataloğu doldurmaya çalışmak. Tablodaki "belirgin örnek yok" satırları bilinçli. Var olmayan bir smell'i bulmak için kodu zorlamak, gerçek kokulardan zaman çalar.

Araç uyarısı sayısını hedef yapmak. 72 uyarıyı sıfıra indirmek Shotgun Surgery'yi çözmez. Uyarılar yerel kalitenin iyi bir göstergesi, tasarımın değil.

İsmi bilip nedeni bilmemek. Her smell'in arkasındaki soru aynı: bu yapı, gelecekteki hangi değişikliği pahalı ya da riskli hâle getiriyor? Cevap "hiçbirini" ise koku zararsız olabilir.

Smell'i bulup hemen düzeltmeye girişmek. Testi olmayan kodda en güvenli ilk adım çoğu zaman düzeltme değil, mevcut davranışı test'le sabitlemek. 14. yazı bununla başlıyor.

Küçük Bir Deneme

★☆☆ Açılıştaki CancelOrder'a geri dön ve tablodaki isimlerle kokuları listele. Kaç tane buldun? İlk tahminine göre hangisini kaçırmıştın?

★★☆ LEGACY_ORDERSERVICE.cs'i kendi projene koy, yukarıdaki .editorconfig'i ekle ve uyarıları rule ID'sine göre say. Hangi smell'ler için hiç uyarı çıkmadı?

★★★ Report(DateTime from, DateTime to, bool csv) method'unda en az üç smell bul ve her biri için katalogdan bir refactoring seç. Hangisini önce uygulardın ve neden?

Bu yazıda kokulara isim verdik. Bir sonraki yazıda bu isimlerin arkasındaki kuralların kendisini sorguluyoruz: John Ousterhout'un Martin'le 2024–2025'teki tartışması, Casey Muratori'nin performans eleştirisi ve kuralların nerede bittiği.