07 października 2013

Czytelność kodu a przegląd kodu w moim zespole w Citi

Nie miałem jakiejkolwiek wątpliwości, że przejście do Citi będzie dla mnie wyjątkowe pod wieloma względami. Ot, chociażby dzisiejsze spotkanie zespołu celem przeglądu kodu jednego z nas - nazwisko ominę (wybacz Artur).

W jeden sali miałem okazję zasiąść z (kolejność przypadkowa) Marcinem, Grześkiem, Tomkiem, Arturem, Pawłem i innymi, mniej lansującymi się w Sieci, ale wciąż światłymi osobami (określenie "lansowanie" używam tutaj wyłącznie w pozytywnym znaczeniu). Wielu ich zna, niektórzy nawet osobiście, ale niewielu może z nimi pracować na co dzień. Ja taką okazję mam i nie zamierzam jej marnować, więc uczestniczę we wszystkich spotkaniach. Uważam, że obcowanie z mądrym sprawia, że sam się nim stanę (?), a co najmniej zbliżę się do takiego poziomu. Dziękuję Panowie!

Podczas pierwszego przeglądu kodu, wyniknęła dyskusja o czytelności kodu. Początkowo sądziłem, że to kolejne marnowanie czasu, ale wystarczyło wstrzymać się z dalszymi ocenami, aby przekonać się, jak bardzo się myliłem. Dyskusja rozgorzała, kiedy pojawił się poniższy kawałek kodu:
  val wasStateChanged = prevCMStatusState match {
    case Some(prevState) => prevState != currentState
    case None => true
  }
Ten kawałek został zganiony za powtórzenie implementacji metody Option.fold. Korzystając z niej możnaby powyższe zapisać następująco:
  val wasStateChanged = prevCMStatusState.fold(true)(_ != currentState)
I tu się zaczęło.

Kiedy zobaczyłem fold zacząłem rozkładać prevCMStatusState na elementy (zakładając, że to kolekcja). To był błąd, bo prevCMStatusState to scala.Option.

Zajęło mi chwilę, co mogłoby to robić, ale kiedy dowiedziałem się, spodobało mi się. Jakieś takie geek'owe :) I tu cały pies pogrzebany. Może faktycznie geek'owe, albo po prostu moja nieznajomość (ignorancja) podstawowych klas i ich metod w Scali sprawiła, że odniosłem takie wrażenie. Nieistotne, bo szybko stałem się zwolennikiem Option.fold. Nie wszyscy.

Inny przykład mógłby być taki:
  case class X(age: Int)
  Some(X(40)).fold(false)(_.age > 30)
Odczytanie tej linijki z Option.fold zajmuje mi tyle samo czasu, co odczytanie pattern matching wyżej, a pisze się krócej, więc obstaję za wersją krótszą.

Ale czy krócej to piękniej? I czy nie przekraczamy cienkiej granicy czytelności kodu nad jego zwięzłością? I czy czytelność kodu jest cechą kodu czy relacją między kodem a czytelnikiem. Sądzę, że to drugie.

Dla mnie czytelny kod nie musi implikować jego czytelności dla Ciebie czy Twoich znajomych. Dyskusja sprowokowała mnie do zastanowienia się nad (lekko wyświechtanym) określeniem "czytelność kodu". Dla mnie, czytelność kodu jest wypadkową doświadczenia czytelnika, a nawet nastroju. Dzisiaj jestem w stananie czytać między liniami, ale czy jutro również będzie mi to dane? Zdecydowanie obstaję nad pełną znajomością standardowej biblioteki języka (w tym przypadku Scali), więc jeśli mamy Option.fold w Scali, to będę z tego korzystał (zamiast pisać implementację za każdym razem, gdzie Option.fold mógłby być zastosowany).

A jak z Twoją definicją czytelności kodu? Czy powyższy przykład mógłby stanowić podstawę do takiej dyskusji z Tobą? Czy znasz inne, kontrowersyjne przykłady?