Programozási módszerek

Programozási módszerek

OKTV-s informatika versenyfeladat átalakítása 2. rész

2015. december 19. - Enpassant 2

Az előző részben megnéztünk egy OKTV-s informatika versenyfeladatot és azt tiszta és egyszerű kóddá igyekeztünk alakítani. A fiamtól kapott visszajelzések alapján ez nem sikerült maradéktalanul és nagyon hasznos észrevételeket tett. A következőkben az észrevételek alapján próbáljuk tovább tisztítani a kódot, növelve ezzel az olvashatóságot. Majd megnézzük, hogyan lehetne ezt a kódot kicsit továbbfejleszteni és az mekkora munkával jár.

Észrevételek és tisztogatás

Mi ez az IntervalLengthAndMax és miért ilyen hosszú a neve?

Teljesen jogos az észrevétel, nekem sem tetszett igazán, de elsőre azt gondoltam, hogy kifejezőbb, ha ott van a hossz és a maximum a nevében, de ezzel csak azt értem el, hogy olvashatatlanabb lett a kód. A Clean Code-olás egyik fő szabálya, hogy bátran nevezzük át az azonosítókat, ha azzal növeljük az olvashatóságot! Ha valaki tud frappánsabb elnevezéseket, akkor bátran írjon! Nevezzük át Interval-ra! Így is pontosan leírja, hogy mit ábrázol, az pedig, hogy konkrétan mit tárol, az általában nem érdekes, ha valahol kell, akkor megnézzük az osztály API-ját.

Zavaros ez a (List[IntervalLengthAndMax), IntervalLengthAndMax) páros.

Itt is elfogadtam az észrevételt, valóban zavaró egy kicsit, hogy most melyik mire is való. Ezért csak a List[Interval]-t hagyom meg, annak az első eleme (head) fogja tárolni az aktuális intervallumot, amin éppen dolgozunk.

Tisztogatás

Mivel készítettem egy egyszerű tesztet a kódomhoz, ezért nyugodtan állok az átalakításnak, mert jelezni fogja, ha valami hibát vétek.

Ez lett a kódom a tisztogatás után:

case class Interval(length: Int, max: Int) {
  def add(value: Int) = Interval(length + 1, math.max(max, value))
}
private val emptyInterval = Interval(0, 0)

private def calcNextState(intervals: List[Interval], value: Int) = {
  val currentInterval = intervals.head
  (currentInterval, value) match {
    case (`emptyInterval`, 0) => intervals
    case (_, 0) => emptyInterval :: intervals
    case _ => currentInterval.add(value) :: intervals.tail
  }
}

def calcIntervals(values: List[Int]): List[Interval] = {
  val initState = List(emptyInterval)
  val intervals = values.foldLeft(initState)(calcNextState)
  if (intervals.head == emptyInterval) intervals.tail.reverse
  else intervals.reverse
}

 Az egyszerűsége semmit sem változott, de az olvashatósága nagymértékben javult.

Továbbfejlesztés

Ne felejtsük el, hogy a programunk eleve többet tud, mint az eredeti, hiszen nem kellenek neki előfeltevések. Ugyanolyan jól működik, ha nem 0-val indul és/vagy végződik az értékek vektora.

 A tiszta és egyszerű kódnak az a jó tulajdonsága van, hogy könnyű átalakítani. Nézzük ezt meg!

Alakítsuk át a programunkat úgy, hogy ne csak a pozitív egészekre működjön, hanem tetszőleges egész számra. Ehhez egy új elemre lesz szükségünk az Int helyett, amivel jelezhetjük az intervallumok határát. Erre pontosan jó az Option konstrukció, aminek kétfajta értéke lehet, valamely érték (pl. Some(5)), vagy a semmi (None).

Mit kell változtatni a kódon? Először is a calcIntervals bemenő paraméterét változtassuk List[Option[Int]] típusúra, mentsük el és nézzük meg a fordító mit mond. A fordító figyelmeztet, hogy a calcNextState paraméterezése nem egyezik, mert a value paramétere Int, holott Option[Int]-et akarunk átadni. Az opcionális érték átadása jó, tehát a calcNextState paraméterezésén kell változtatnunk. A calcIntervals függvény törzsén nem kell semmit változtatni, a működése teljesen megegyezik a korábbival. Ez nagyon jó! A calcNextState függvény paraméterét változtassuk meg Option[Int]-re, majd nézzük a fordító mit mond. A fordító a három case sornál jelez hibát. Az első kettőnél a 0-kat kell csak lecserélnünk None-ra, a harmadiknál pedig az illeszkedést vizsgálatot kell átírni Some(value)-ra. Mentsünk, mit mond a fordító? Nincs hiba, remek!

Írjuk át a tesztet, és adjunk meg olyan intervallumot is, ahol csak negatív értékek vannak. Futtassuk a tesztet. Hibát jelez ott, ahol csak negatív értékek vannak. A maximum értéknek 0-át ad, holott negatív számot kellene számolnia. Hopp, át kell írnunk az üres intervallumunk minimum értékét Int.MinValue-ra. Így már ott sincs hiba! Nagyon jó! :)

Megállapíthatjuk, hogy könnyű volt a fejlesztést elvégezni, a fordítótól mindig jelentős támogatást kaptunk, hogy mit kell megvizsgálnunk, és mit kell változtatnunk. Ahol a fordító nem segített, ott a teszt segített. Házi feladatként megpróbálhatjuk ezt a kis fejlesztést az eredeti kódunkba is beépíteni. Nézzük meg a kész kódot is:

case class Interval(length: Int, max: Int) {
  def add(value: Int) = Interval(length + 1, math.max(max, value))
}
private val emptyInterval = Interval(0, Int.MinValue)

private def calcNextState(intervals: List[Interval], optValue: Option[Int]) = {
  val currentInterval = intervals.head
  (currentInterval, optValue) match {
    case (`emptyInterval`, None) => intervals
    case (_, None) => emptyInterval :: intervals
    case (_, Some(value)) => currentInterval.add(value) :: intervals.tail
  }
}

def calcIntervals(values: List[Option[Int]]): List[Interval] = {
  val initState = List(emptyInterval)
  val intervals = values.foldLeft(initState)(calcNextState)
  if (intervals.head == emptyInterval) intervals.tail.reverse
  else intervals.reverse
}

Végszó

Azt vehettük észre, hogy a tiszta és egyszerű kódot könnyű megérteni, könnyű módosítani, átalakítani, kevés benne a hibalehetőség, könnyű tesztelni. Azt is észrevehettük, hogy tiszta és egyszerű kódot nem egyszerű írni, főleg nem elsőre! Hacsak tehetjük, kérjük ki mások véleményét is, még jobb, ha van lehetőségünk páros programozásra.

Új projektnél mindig tartsuk tisztán és egyszerűen a kódot, hogy bármilyen felhasználói módosítási igényt gyorsan és plusz hibalehetőségek nélkül el tudjuk végezni.

Könnyű ezt mondani, hiszen mindannyian dolgoz(t)unk olyan projekten, amit az ember már bottal sem szívesen piszkál meg. Ilyenkor ahelyett, hogy módosítanánk a régi kódon, inkább lemásoljuk, nehogy a régi működés elromoljon és a másolaton módosítunk, ezzel is tovább merülve csúnya és bonyolult kód mocsarába. Az egész újraírására se idő, se pénz nincs, ráadásul folyamatosan jönnek az új igények is. Mit tegyünk ilyenkor? Tartsuk magunkat a cserkészek egyik szabályához:

A cserkész tisztábban hagyja el a táborozó helyet, mint ahogy kapta.

Tehát akármikor egy kódhoz hozzányúlunk és megértjük, akkor egy kicsit tegyük tisztábbá, egyszerűsítsünk rajta egy kicsit. Már egy azonosító átnevezése is sokat segíthet. Ha mindig tisztábban tesszük fel, mint ahogy kivettük, akkor szép lassan, de javulni fog a kódunk minősége és kellemesebb lesz dolgozni a kódunkkal.

OKTV-s informatika versenyfeladat átalakítása

Tiszta és egyszerű kóddá (Clean and Simple Code)

A 2013/2014-es Informatika OKTV verseny 3. szám feladatának forráskódját fogjuk átalakítani tiszta és egyszerű kóddá. Letölthető a feladatlap, és a javítási-értékelési útmutató.

Miért OKTV-s versenyfeladaton?

Ezek a feladatok szándékosan olvashatatlan és bonyolult módon vannak megírva, hogy a versenyzőknek nehezebb legyen a kódot megérteniük.

A feladat

3. feladat: Adatok (48 pont)
Az alábbi algoritmus N nemnegatív adatot kap bemenetként (N>2, X(1)=0, X(N)=0, X(i)≥0) az X vektorban, amelyekből több értéket számol ki:

Valami(N,X,A,B,C):
    A:=0; D:=0; E:=0
    Ciklus i=2-től N-ig
        Ha X(i-1)=0 és X(i)>0 akkor D:=0; E:=0
        Ha X(i)>0 akkor D:=D+1
        Ha X(i)>E akkor E:=X(i)
        Ha X(i)=0 és X(i-1)>0 akkor A:=A+1; B(A):=D; C(A):=E
    Ciklus vége
Eljárás vége.

Ez Tiszta és Egyszerű kódnak tűnik, nem az?

Igen elsőre tisztának és egyszerűnek tűnik, a valóságban azonban nem az. Talán nem is a legjobb szó a tiszta (clean) és az egyszerű (simple), ha helyettük inkább az olvasható és érthető kifejezéseket használjuk, akkor talán jobban megértjük ezen programozási módszerek tartalmát.

Tiszta (olvasható) a kód?

Sajnos nem nagyon! Nem lehet tudni, hogy mit jelentenek az N,X,A,B,C,D,E jelek. Azt sem, hogy az eljárásunknak mik a bemenő, mik a kimenő paraméterei. Az olvashatóság szempontjából a pozitívum csupán a rövidsége és a formázása.

Egyszerű (érthető) a kód?

Sajnos ez sem igaz! Rengeteg az eljárásban a módosuló változó (mutable variable), ezek állapotait nehéz követni (fejben tartani). Rengeteg a hibalehetőség az eljárásban. 7 helyen is gond lehet a vektorok indexelésével, X(i) és X(i-1), könnyen indexelhetünk olyan vektor elemre, ami nem létezik és olyankor hibával le fog állni a program. A bemeneti értékekre előfeltevés van, ami fontos az eljárás szempontjából, ha azok nem igazak, akkor hibásan fog futni a program. Ezekre a feltételekre az eljárás nem ellenőriz rá.

Első kísérlet (Scala  nyelven)

case class IntervalLengthAndMax(length: Int, max: Int) {
  def add(value: Int) = IntervalLengthAndMax(length + 1, math.max(max, value))
}
private val emptyInterval = IntervalLengthAndMax(0, 0)

def calcIntervals(values: List[Int]): (List[IntervalLengthAndMax]) = {
  val initState = (List.empty[IntervalLengthAndMax], emptyInterval)
  val (intervals, currentInterval) = values.foldLeft(initState) {
    case ((intervals, currentInterval), value) =>
      (currentInterval, value) match {
        case (`emptyInterval`, 0) => (intervals, currentInterval)
        case (_, 0) => (currentInterval :: intervals, emptyInterval)
        case _ => (intervals, currentInterval.add(value))
      }
  }
  if (currentInterval != emptyInterval) (currentInterval :: intervals).reverse
  else intervals.reverse
}
 

Tiszta (olvasható) a kód?

Igen! :) Minden azonosítónak (változónak, eljárásnak, osztálynak) szép, beszédes neve van. Pontosan lehet tudni, hogy a függvényünknek mik a bemenő, mik a kimenő paraméterei. A függvény viszonylag rövid és formázott.

Egyszerű (érthető) a kód?

Ez is nagyjából igaz! Egyetlen módosuló változónk sincs (mutable variable). Egyetlen hibalehetőség sincs a programban. Nincs a bemeneti értékekre semmilyen előfeltevés. A program működése viszonylag könnyen megérthető.

Tudunk ezen még egyszerűsíteni? Igen, az állapotváltozást megadó részt kiemelhetjük egy külön függvénybe, így a két függvény megértése külön-külön sokkal egyszerűbb lesz, mint így együtt. Egy mélységgel kisebb is lesz a fő függvényünk, ami az olvashatóságot tovább fogja javítani.

Végeredmény

case class IntervalLengthAndMax(length: Int, max: Int) {
  def add(value: Int) = IntervalLengthAndMax(length + 1, math.max(max, value))
}
private val emptyInterval = IntervalLengthAndMax(0, 0)

private def calcNextState(
  state: (List[IntervalLengthAndMax], IntervalLengthAndMax),
  value: Int) =
{
  val (intervals, currentInterval) = state
  (currentInterval, value) match {
    case (`emptyInterval`, 0) => state
    case (_, 0) => (currentInterval :: intervals, emptyInterval)
    case _ => (intervals, currentInterval.add(value))
  }
}

def calcIntervals(values: List[Int]): (List[IntervalLengthAndMax]) = {
  val initState = (List.empty[IntervalLengthAndMax], emptyInterval)
  val (intervals, currentInterval) = values.foldLeft(initState)(calcNextState)
  if (currentInterval != emptyInterval) (currentInterval :: intervals).reverse
  else intervals.reverse
}

Tiszta (olvasható) a kód?

Igen! :) Minden azonosítónak (változónak, eljárásnak, osztálynak) szép, beszédes neve van. Pontosan lehet tudni, hogy a függvényünknek mik a bemenő, mik a kimenő paraméterei. A függvények rövidek és jól formázottak.

Egyszerű (érthető) a kód?

Ez is igaz! :) Egyetlen módosuló változónk sincs (mutable variable). Egyetlen hibalehetőség sincs a programban. Nincs a bemeneti értékekre semmilyen előfeltevés. A függvények működése könnyen megérthető.

Állapotváltozást megadó rész:

  • Ha az aktuális intervallum üres és a vizsgált érték 0, akkor nincs változás az állapotban
  • Különben, ha a vizsgált érték 0, akkor az intervallumok elejére beszúrjuk az aktuális intervallumot és az új aktuális intervallumot üresre állítjuk
  • Különben az intervallumok nem változnak és az aktuális intervallumhoz hozzáadjuk az értéket

Fő függvényünk:

  • Kiindulási állapotunk egy üres intervallum listából és egy üres intervallumból áll
  • Az értékeket (values) a kiindulási állapot és az állapotváltozást megadó rész alapján összesíti (foldLeft)
  • Az így megkapott állapot alapján, ha az aktuális intervallum nem üres, akkor azt is hozzáadja, majd megfordítja a listát, mert fordított sorrendben gyűjtöttük.

A teljes forrás kód itt elérhető.

Folytatás következik.

süti beállítások módosítása