Kod Analizi: Kurucu Kullanımı

Danışmanlık verdiğim yerlerde her zaman farklı yaklaşımlarla yazılmış kodlar görüyorum. Bu tür kodların ufkumu açtığında şüphe yok. Ama açıkçası her ufuk açmanın iyiye ve güzele doğru olduğunu söylemek zor.

Aşağıdaki kod da bu şekilde karşılaştığım kodlardan. Tabi ki ciddi miktarda kamufle edilmiş isimler içeriyor. Ama koddaki yaklaşım, arka taraftaki düşünce vs. tamamen korunmuş durumda. Kod şöyle:

public JTUser execute(RegistrationForm form, 
          HttpServletRequest request, Locale locale)    
          throws JTException {

	String uid = Utils.generateUUID();
	String salt = Utils.generateSalt(32);
	String accountId = Utils.generateUUID();

	JTUser user = new JTUser(form.getEmail().trim(), null, true, true, 
                          true, true, AuthorityUtils.NO_AUTHORITIES, uid, 
                          form.getEmail().trim(), salt, 
                          form.getName().trim(), form.getSurname().trim(), null, accountId, true,
		          true, form.getWelcomeMessage().trim(), true, new Date(), null);

	user.setStatus(JTConstants.PENDING);

	String servletPath = request.getServletPath();
	if (executeInner(form, locale, user)) {
	    auditLogDao.insert(new AuditLog(user.getUid(), user.getUsername(), Utils.getRemoteIp(), Utils.getSid(),
		               AuditType.REGISTRATION, AuditSubtype.PENDING, ResponseCode.SUCCESS, 
                               new Date(), null));
	    authenticateFastRegisterUser(form, request);
	}

    return user;
}

Bu metodu analiz etmek istersek, hem nesne-merkezli programlama hem de temiz kod gibi noktalardan inceleyebiliriz. Önce şekilden başlayıp sonra kodu yazanların sahip oldukları, en azından koda yansıttıkları anlayışa doğru gidebiliriz. Bu yüzden önce isimlerden başlayalım.

Mekanı kullanma

Yukarıdaki kodda düzgün olan nedir diye sorsam, olsa olsa “sadece paragraflar” denebilir çünkü isimlendirme bile problemli. Ama açıkçası bir kodun paragraflandırılmış olması, yani bir geliştirme aracının iki-üç tuşa basımlık bir işini meziyet olarak ortaya koymak mümkün değil. Öte taraftan JTUser kurucu ve insert metot çağrılarının, kodda hüküm süren yaklaşımdan dolayı mekanı zorunlu olarak iyi kullanamadıkları da bir gerçek.

İsimlendirme

Çoğu zaman kodu yazanların kafalarındaki karmaşıklığı verdikleri isimlerden çıkarabilirsiniz. Bu kod parçasında da execute ve executeInner metotları, bu metotları yazanların kafalarındaki karmaşıklığa bir örnektir. Biraz empati yapıp, execute metodunun Command tasarım kalıbından esinlendiği düşünülebilir. Bu durumda daha geniş bağlama bakıp hakikatten bu kalıbın kullanıp kullanılmadığına karar verilebilir. Diyelim ki Command kalıbı kullanıldı ve bu metodun bulunduğu sınıf, kalıptaki arayüzü gerçekleştiriyor ve bu yüzden metodun ismi, Command kalıbının geleneğine bağlı kalınarak execute olarak verilmiş olsun. Peki executeInner‘ı ne yapacağız? executeInner metodunun içinden çağrılan metotların isimlerini tahmin edebilir miyiz? executeIpInner, executeDahaDaInner ya da executeInner2 diye gidebilir 🙂  Bu kadar kötü ve ezbere bir isimlendirme olursa, daha fazla düşünce ve tecrübe gerektiren, daha zorlu algoritmik ve modelleme problemlerinde nasıl bir yaklaşım olabilir, tahmin etmek zor değil.

Bir diğer isimlendirme problemi de authenticateFastRegisterUser metodu için söz konusu. Bu metot hali hazırda sisteme kayıtlı olan kullanıcının hızlı olarak sisteme girmesini sağlıyor gibi duruyor. Bu durumda metot isminin authenticateFastRegisteredUser olması gerekirdi.

Yaklaşım

Bu kod parçasında temelde JTUser oluşturulup executeInner‘a geçiliyor ve dönen değer true olduğunda göre  audit bilgisi AuditLog nesnesi olarak oluşturulup, veri tabanına gönderiliyor ve user sisteme dahil ediliyor. Bu işler olurken kodda göze batan iki yer var, iki nesnenin oluşturulması. Bu iki nesne için tabi olarak ikş kurucu çağrısı yapılıyor ve bu çağrılar o kadar kötü kurgulanmış ki insan hayret ediyor. Şu çağrıya bakın örneğin:

	JTUser user = new JTUser(form.getEmail().trim(), null, true, true, 
                          true, true, AuthorityUtils.NO_AUTHORITIES, uid, 
                          form.getEmail().trim(), salt, 
                          form.getName().trim(), form.getSurname().trim(), null, accountId, true,
		          true, form.getWelcomeMessage().trim(), true, new Date(), null);

Bir kurucuya neden nesneyi doğrudan geçmek yerine parçalarını ayrı ayrı geçersiniz ki? Bu kurucuya form nesnesinin email, name, surname ve welcomeMessage alanları ayrı ayrı geçiliyor. Aslen sadece form nesnesinin geçilmesi gerekirdi. Bu durum nesne-merkezli programlamanın pek özümsenmediğini açıkça gösteriyor.

Öte taraftan, bir metoda neden new Date() ile o anki zamanı geçersiniz? İki şeyden dolayı, ya geçilen yerde o anki zamana ulaşamıyorsunuzdur,  yani kurucuda new Date() yazamıyorsunuzdur :), ya da o kadar ince hesaplarla çalışıyorsunuzdur ki, önemli olan execute metodundaki zamandır, bir nano saniye sonra JTUser kurucusundaki an işinize yaramıyordur 🙂 Açık ki burada bu iki durum da söz konusu değildir.

Başka ne gibi problemler var bu kurucu çağrısında? null geçilmesi problemdir. Üç parametre null olarak geçiliyor ve muhtemelen başka yerlerdeki aynı kurucu çağrısında null geçilmiyordur. Bu durumda da kurucuda devamlı null kontrolü yapılmalıdır. Aslen hep null geçiliyorsa bu daha büyük bir problemdir çünkü bu o nesnenin zaten bu kurucuya hiç geçilmemesi gerektiğini gösterir. Yok ilk durum geçerliyse yani bazen null geçiliyor bazen de değer geçiliyorsa bu zaten sıkıntılı bir yapı ortaya koyar çünkü null geçmek ya da null döndürmek hataya sebebiyet verir.

Kurucuya geçilen diğer üç parametre yani uid, salt, accountId, de geçilmemelidir çünkü bu üç değer zaten Utils sınıfından statik metotlarla alınabilir. Ne farkeder ki, ha orada ha burada demeyin, en az parametre geçecek şekilde kod yazmamız gerekli. Aksi taktirde bağımlılıkları gereksiz yere arttırmış oluyoruz.

Geriye bu kurucuya geçilen yedi tane true parametresi kalıyor. Bu parametreler belli ki JTUser nesnesinin belli bir durumda (state) oluşmasını sağlıyorlar. Ama bir nesnenin durumunda yedi tane boolean değişken yer alması biraz garip değil mi? Bu yedi değişken toplamda 2^7 = 128 farklı durumu temsil eder. Bu durumda JTUser nesnesinin en az 128 farklı hali var demektir. Örneğin yeni kayıt olmuş ama henüz teyit edilmemiş kullanıcı, yeni kayıt olmuş ve teyit edilmiş kullanıcı, vs. diye gidebilir. Problem şu: Her ne kadar biz metotları, onlara az parametre geçilecek şekilde tasarlayalım desek de kurucularda böyle yapmak imkansızdır. Yani kurucular nihayetinde oluşturdukları nesnelerin durumlarından sorumlu oldukları için genelde çok sayıda parametre alırlar. Bu türden kuruculara, parametrelerden dolayı uzun olduklarını ifade etmek için “teleskopik” denir. Biz yukarıda saydığımız sıkıntıları giderecek şekilde kodu tekrar yazsak bile kurucular hala çok parametre alır halde kalırlar. Örneğin JTUser kurucusunun 20 olan parametre sayısını 8’e indirmiş oluruz, ve bu hale gelir:

	JTUser user = new JTUser(form, true, true, true, true, true, true, true);

Garip değil mi? Garip olan şey bu kurucu metodun uzunluğu değil, çağrılmasındaki zorluk. Yedi tane arka arkaya gelen boolean parametrenin hangisinin ne olduğunu ve ne işe yaradığını bilmeniz gerekli. Muhtemelen bu kurucu metot şöyle tanımlanmıştır:

...

public JTUser(Form form, boolean verified, boolean authorized, boolean authenticated, ...){
   ...
}

...

Çok fazla sayıda parametre içeren metot yazmanın kaçınılmaz sonuçlarından birisidir bu durum. Normal metotlarda, metotları bölüp parçalayarak, sadece br işe odaklar hale getirip  bu durumdan kaçınabiliriz. Fakat metotların ismi olmasına karşın kurucuların ayırt edici isimleri yoktur. Ve özellikle de karmaşık nesnelerde hem pek çok kurucu vardır ama hangisinin, hem de kurucuların kendileri uzundur. Dolayısıyla hangi kurucunun çağrılacağı ve hangi parametrelerin geçileceği, zaman alan bir kodlama gerektirir ve hataya çok açıktır. Öyleyse kurucularda bu durumdan nasıl kaçınılabilinir?

Aslında bunun bir kaç yöntemi var. Örneğin yaratımsal (creational) kalıpları kullanmak. Ama nasıl bir çözüm önerilirse önerilsin yapılması gereken en temel şey, geçilen boolean parametreleri ortadan kaldırıp ya da en azından azaltıp, parametrenin rolünü metodun isminde ifade edecek şekilde kurgulanmış isimlere sahip, nesne oluşturan, gerekirse uzun isimli ama anlamlı metotlar yazmak. Eğer bu metotları ayrı bir sınıfta yazarsanız üretici metot (factory method) kalıbına varırsınız. Bu durumda bu sınıf şöyle olacaktır:

public class JTUserFactory{
	
	public JTUser produceVerifiedJTUser(...){ ... }

	public JTUser produceVerifiedJTUserUsingForm(Form form, ...){ ... }

	public JTUser produceVerifiedAndAuthorizedJTUserUsingForm(Form form, ...){ ... }

	...
}

(Üretici metot (factory method ) kalıbı için buraya bakabilirsiniz.)

Benzer ama bir başka çözüm, J. Bloch “Effective Java 2nd Ed.” isimli kitabının ilk maddesinde ifade edilmiştir. Burada tekli edilen şey, sınıfta kurucu kullanmak yerine statik factory metotlarının kullanılmasıdır. Yani yukarıdaki metotların statik olarak JTUser sınıfında olduğu durumdur. Ayrıca gerekirse bu metotları kullanmak için JTUser‘ın tüm kurucuları private yapılabilir.

Böyle bir yol izlendiğinde yukarıdaki kodumuz şu hale gelecektir:

public void authenticateUser(RegistrationForm registrationForm, 
                             HttpServletRequest request, 
                             Locale locale) throws JVTException {

   JVTUser user = JVTUser.createUnauthenticatedUser(registrationForm);

   if(authenticateUserUsingForm(registrationForm, locale, user)){
           
      authenticateFastRegisterUser(registrationForm, request);    
      
      mailSender.sendMail(user, locale);

      auditLogDao.insert(
          AuditLog.createAuditLogForUserAuthentication(user));
   }
}

Bu kodda, JTUser ve AuditLog sınıfları statik üretici metotlara sahip olacak şekilde kurgulandıklarından, nesnelerini oluşturmak createUnauthenticatedUser ya da createAuditLogForUserAuthentication gibi metotlarla çok daha rahat hale gelmiştir.

Kodun ilk halinin en berbat tarafı, sadece kötü yazılmış olması değildir. Çoğu zaman bu kodun sadece böyle yazılmış olmasına odaklanır esas problemi gözden kaçırırız. İlk haldeki gibi yazılmış karmaşık kodlar, copy-paste ile her yere dağılır. Her JTUser nesnesine ihtiyacı olan bu ve buna benzer kurucu çağrılarını, çağrıyı yapmak zor olduğundan, haklı olarak copy-paste ile alıp kullanma eğiliminde olur. Bu da bakımı çok zor bir kod yapısına götürür. Eğer bu çağrıyı yapmak için copy-paste’e başvuracak kişi, biraz akıllıca davranıp, yukarıda bahsedilen yollardan birisiyle, cut-paste kullanarak bu kodu iyileştirse (refactoring) idi çok güzel bir şey yapmış olurdu ve bu kod yapısı da iyileşerek büyürdü.

Burada kurucular üzerinden bir kod analizi yaparak nasıl daha temiz, nesne-merkezli ve bakımlanabilecek kod yazılabileceğini ele aldık. Ve yine temiz kod için iyileştirmeye (refactoring) ihtiyacımız oldu.

Rahat bakımlanabilir kodlar yazmak dileğiyle 🙂

Toplam görüntülenme sayısı: 1038