Kod Gözden Geçirmesi - Code Review

Kod Gözden Geçirmesi Neden Yapılır
Kod gözden geçirmesinde esas amaç hataları engellemek ve kodu doğrulamak için değildir. Esas amaç, kodun anlaşılır ve idame ettirilebilir olmasıdır.

If it cannot be reviewed, it cannot pass review.

You have to understand that code review isn't for finding bugs. That's what QA is for. Code review is to ensure that future maintenance of the code is possible. If you can't even follow the code now, how can you in six months when you're assigned to do feature enhancements and/or bug fixes? Finding bugs right now is just a side benefit.

Örneğin kod gözden geçirildi diye birim testi yapmamak olmaz. Aynı şekilde sistem testinden de vazgeçilemez.

Kod Gözden Geçirmesini Destekleyici Statik Kod Analizi

Mutlaka yapılmalı. Açıklaması şöyle

Code reviews are definitely necessary and useful. It's a way to impart knowledge, educate, control a task, improve code quality and formatting, fix bugs. Moreover, you can notice high-level errors related to the architecture and algorithms used. So it's a must-have practice, except that people get tired quickly. Therefore, static analysis perfectly complements reviews and helps to detect a variety of inconspicuous errors and typos.

Bazı Kazanımlar
Kod gözden geçirme faaliyetinin getirdiği bazı kazanımlar şöyle.

1. Geliştiricilerin eğitilmesi

• Ekip üyeleri kodun diğer yerlerini de öğrenme imkanı bulurlar. 
• Daha tecrübesiz olanlar, tecrübeli olanlardan teknik öğrenirler.

2. Kodun iyileştirilmesi

Çok ilerlemeden iyileştirme yapmak çok daha ucuzdur. Bazı kod iyileştirme örnekleri:
a ) Projede kullanılan Util, Helper gibi sınıflar aslında bazı sınıfların eksikliğine işaret eder ( yani code smell). Örneğin sadece bean veya collectionların olduğu bir yazılımda sağda solda static metodları olan Util sınıfları pörtlemeye başlar. Bu tür sınıfların kaldırılıp yerlerine daha işlevsel sınıflar koymak gerekir.

b) Kod ile açıklama (comment) arasında uyumsuzluk varsa bunu sadece insan yakalar. Statik analiz yapan bir araç bu tür şeyleri yakalamaz. Kod içinde yarım kalmış, unutulmuş işler bile olabilir.

// TODO by v55: Create migration to move constraints to new column
// TODO by v56: Create migration to drop old column

Extreme Programming gibi çiftle halinde çalışılan süreçlerde Kod Gözden geçirmesi diye bir ayrıca tanımlı bir faaliyet yok. Zaten sürekli kod gözden geçirmesi yapılıyor.

c) Duplicate Code

Otomatik araçlar bazen duplicate code'a itiraz ediyor. Aslında olmasa daha iyi ancak bu kodu temizlemek çok daha maliyetli ise kalması tercih edilebilir. Açıklaması şöyle

Duplication should be tolerated only when removing it would be overall more costly now for some other reason

Duplicate Code eğer soyutla işlemini yapmak çok zor ise kabul edilebilir. Açıklaması şöyle

duplication is far cheaper than the wrong abstraction

Yanlış soyutlamanın sonuçları şöyle

Sometimes finding the right abstraction is very difficult. In such cases it is tempting to just go for any abstraction to reduce duplication. But later you might find out that your abstraction doesn't hold for all cases. However, it is costly to change everything again and to go a different route

Kod Gözden Geçirmesi Ne Kadar Yapılmalı
Her faaliyette olduğu gibi bu faaliyetin de bir maliyeti var. Maliyeti ve kazanımı hesaplamak kolay bir iş değil. Kodun kaç defa gözden geçirilmesi gerektiği konusu da tamamen ekibe bağlı.

Buradaki bir yazıda kod gözden geçirme faaliyetinin işi yavaşlattığından bahsediliyor. Kod bazen 2-3 gün merge edilemiyormuş. Önerilen şey ise kod gözden geçirmesinin hafifletilmesi değil de daha verimli/hızı hale getirilmesi. Açıklaması şöyle.

Code review is just about the one thing that has been consistently shown to significantly improve code quality, and you're effectively proposing to stop doing it.

Instead, put your efforts into improving your code review process

Aslında kod gözden geçirmesinde bir yavaşlık varsa, sebeplerden birisi de kodun nasıl olması gerektiğine dair bir Kod Gözden Geçirme Listesi ve Kodlama Standardının olmaması olabilir. Açıklaması şöyle.

I'm starting to think you probably have bigger problems in your team than the code review process - it sounds like you don't have consensus on how to write software, and quite possibly your broken code review process is a consequence of that. You're going to need a strong leader to sort that out, and fixing the code review process should be at the end of that, not the start.

Kod Gözden Geçirmesinde Ön Koşullar
Bence gözden geçirmeye başlamadan önce kodun birim testlerinin bulunması önemli ön koşul. Yeterli kapsama yüzdesini sağlamayan kod için gözden geçirme bence başlamamalıdır!

Eğer projede statik kod analizi kullanılıyorsa, hatasız geçiyor olması ise ikinci ön koşul. Daha başka otomatik kontroller yapılıyorsa bu denetimlerin hepsinden hatasız geçmiş olması da ön koşullara dahildir.

Bazı Yanlışlar

Bazı projelerde sıkça gördüğüm bir yanlış, kod gözden geçirmesi görevinin ekipten rastgele seçilen tek bir kişiye atanması. Hatta öyle şeyler oluyor ki benim kodum sana, senin kodun ona, onun kodu bana atanıyor. Bireyler anlamadıkları kodları laf olsun diye gözden geçirmiş gibi yapıp, bir iki cümle bulgu yazıyorlar.

Bu amaçtan sapmaktır. Biten bir paket mümkünse tüm ekip, mümkün değilse yetkin daha fazla kişi tarafından anlayarak gözden geçirilmelidir. Özellikle SRS ile kod arasındaki uyuma dikkat edilmelidir. Bunun için de hangi SRS maddesinin, hangi kod içinde gerçekleştirildiği bilinmelidir. Bunun amaç için kullanılabilecek en basit yöntem kodun başına gerçekleştirilen SRS numarasını yazmaktır.

Kalite güvencenin bu tür sapmalara engel olması beklenir. İyi çalışan bir kalite güvence uzmanının, en azından kod içinde tüm SRS maddelerinin olup olmadığını kontrol etmelidir. Yani izlenebilirliğin sağlamasını yapmalıdır.

Kod Gözden Geçirmesi Metrikleri
Kod gözden geçirmesinde (code review) düzeltme (correction) ve doğrulama (verification) faaliyeti için toplam ne kadar zaman harcandığının metriğini toplayabilmek önemli. Bu metrik için geliştiricilerin Excel'e rakam girmelerini beklemek ilkel bir yöntem. Daha otomatik bir yöntem olmalı.

Kodun kendisine ait bazı metriklerin belli sınırlar içinde olması beklenir. Konuyla ilgili olarak Kod Metrikleri başlıklı yazıya göz atabilirsiniz.

Kod Gözden Geçirmesi Kontrol Listesi
Kod gözden geçirilirken bir kontrol listesi (check list) kullanılır. Checklist aslında projeye göre uyarlanması gereken yaşayan bir liste olmalı. Örneğin sıkça yapılan hataların bu listeye eklenmesi gerekiyor. Ancak bir çok projede kimse bu işle uğraşmak, firmanın çok eskiden hazırladığı kimsenin pek işine yaramayan liste kullanılır.

Listedeki maddelere aşağıdaki cümlelere benzer.
Bazı kontrol listelerinde anlamsız veya göreceli oldukları için tartışmalara sebep olan maddeler var.

Anlamsız olanlara örnek:

-Each file contains software CI ID

SDD ile kod arasında bağlantı kurulması istendiği için bu madde bazı listelerde bulunuyor. Kod ile tasarım arasında bu kadar sıkı bağ kurmak, geliştiricinin elini kolunu bağlar ve refactoring'e izin vermez.

- Source code is compatible and traceable to design 

Yukarıda açıklama ile aynı.

Göreceli olanlara örnek
1. Source code comments are sufficient :
Yazan cümleler genelde şöyle. İşte burada görecelilik ön plana çıkıyor.

• If there is a comment, does it explain why the code does what it does?
• Is each line of the code - in its context - either self-explanatory enough that it does not need a comment, or if not, is it accompanied by a comment which closes that gap?
• Can the code be changed so it does not need a comment any more?

Emniyet kritik bazı projelerde her satır için comment olması isteniyor. O zaman iş sanki biraz daha kolay. Sadece her satıra bakmak yeterli. Kod şöyle görünüyor.

/* Display an error message */
function display_error_message( $error_message )
{
  /* Display the error message */
  echo $error_message;

  /* Exit the application */
  exit();
}

/* -------------------------------------------------------------------- */

/* Check if the configuration file does not exist, then display an error */
/* message */
if ( !file_exists( 'C:/xampp/htdocs/essentials/configuration.ini' ) ) {
  /* Display an error message */
  display_error_message( 'Error: ...');
}

Yapılması gerekenlere bazı örnek

- Source code conforms to coding standard and is checked by automated tool
- Source code is checked manually by reviewer if automation is not possible
- Source code is checked for memory leaks by a dedicated tool
- Source code is compatible and traceable to SRS

Kodlama Standardı

Konuyu buraya taşıdım.