Statikus analízis eszköz futtatása és jelzett hibák átnézése.

[klenium]

Javaslat: SonarQube, FindBugs, VS Code Analyzer, Codacy, Coverity Scan

[klenium]

SonarQube lesz használva, azért is, mert képes megjeleníteni a JaCoCo által generált tesztlefedettség mérését is, sokkal jobb formában, és beépíthető a Travis folyamatába.

[klenium]

A JaCoCo-nak újabb verziója kell a JDK 12-höz, mint amit a laboron használtunk (7.9 -> 8.2), különben hibát dob.

A JaCoCo beállításai hasonlóak a laboron bemutatottéhoz.

[klenium]

A labor útmutatója alapján lefut a tetris-java projekten is a SonarQube localhost-on.

Jelenleg 12 bug, 24 code smell található, a code coverage 69.6%.

[klenium]

A 12 hibának érzékelt eset mindegyike a GraphicGameFrame.java fájlban található, és abból ered, hogy a JavaFX metódusok, pl szélesség, pozíció double típust vár, ellenben a kódban int típusú változók szorzása van a paraméter helyén, pl.:

boardCanvas.setWidth(gridSize.width * blockSize);

A SonarQube-nek igaza van abban, hogy általános esetben az implicit típuskonverziót érdemes kerülni aritmetikai műveleteknél, mert pontosságvesztés vagy túlcsordulás lehetne. Azonban a GraphicGameFrame.java fájlban lévő helyeken a szorzások mindkét operandusáról ismert, hogy kis értékeket vehetnek fel (1-100 közötti érték az ésszerű a rács felépítésére és megjelenítéséhez), így a szorzat se lesz túl nagy (ráadásul a double értékkészlete nagyobb is, mint az int típusé), és egész számok szorzásánál nem veszítünk a pontosságon, ha implicit helyett explicit konverzió lenne.

Ezért ezeket a hibákat hamis pozitívnak jelölöm.

Ennek ellenére már régen is felmerült bennem a probléma ezeknél a soroknál, de eltérő okból. A sok szorzás miatt túl hosszúak a sorok, ami rontja a láthatóságot, indokolatlan mindenhol kiírni az operandusokat, változtatás esetén túl sok helyen kell átírni, ráadásul többször is elvégzi ugyanazt a szorzást fölöslegesen. Felvettem a 0c59439 commitban egy canvasSize tulajdonságot a szorzás eredményének tárolására.

[klenium]

A többi eset javítása előtt integrálni kellene az ellenőrzési folyamatot a Travis rendszerébe, hogy ne kelljen a Maven parancsokat mindig kiadni, és ne localhost-on fusson (ami így csak képként mutatható be). Erre a célra egy saját szerver kellene, a Travis dokumentációja a SonarCould szolgáltatást ajánlja*, ami kis, nyílt projektekre ingyenes.

* Mint megtudtam, ez nem egy harmadik fél szolgáltatása, mint elsőnek feltételeztem, hanem ugyanannak a csapatnak egy másik típusú szolgáltatása a SonarQube egy lehetséges futtatási módjának biztosítására.

[klenium]

Egyelőre sikertelen:

[ERROR] Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:3.6.0.1398:sonar (default-cli) on project tetris: You're not authorized to run analysis. Please contact the project administrator.

[klenium]

Ellentétben a GitHub kulccsal, a SonarCould-ét nem kell külön kódolni a Travis parancssori programjával, ha nem a .travis.yml fájlba teszem, hanem a projekt beállításainál adom meg változóként...

Javítva a kódot már lefut a parancs, és elérhető lett a SonadCould oldalán, ami publikusan, bejelentkezés nélkül megtekinthető, így lehet hivatkozni az ottani adatokra.

[klenium]

Code smell-ek áttekintése:

• Add a private constructor to hide the implicit public one.
  Ez jó ötlet, C++-ban megszokott lépés, de Java-ban még nem is igen láttam. Javítás: 6173339
• Reorder the modifiers to comply with the Java Language Specification.
  Ez direkt volt így elrendezve, mert a rawData értéke túl hosszú, a fájl megnyitásakor a fontosabb információk kerülnek előre.
• Rename field "random".
  Az osztály kis mérete és adott funkcionalitása miatt félreértést ugyan nem okozna, de található neki jobb név. Javítás: 6173339
• Reorder the modifiers to comply with the Java Language Specification.
  Én is úgy szoktam meg, hogy elsőnek van a static, utána a final, átrendeztem. Az egységesség jó hibaelkerülésre is. Javítás: 6173339
• Make the enclosing method "static" or remove this set.
  Jogos lehetne későbbi kiterjesztésnél. A legelső változatban még singleton típusú volt a program felépítése, onnan maradhatott bent. Elvileg lehető kéne legyen egyszerre több Tetris, több ablak futtatása. Jelenleg egyszerűség miatt a MainApplication::createFrame is statikus. Annak idején próbálkoztam is ennek kiküszöbölésével. Most egy kis átalakítással a abeca6e commitban megoldottam. A program továbbra is csak egy darab ablakot futtat, ha több kellene, másképp kell átalakítani a kódot, és áttervezni a MainApplication::init metódust is.
• Swap these 2 arguments so they are in the correct order: expected value, actual value.
  Felcserélve mindegyik helyen. Érdekes, hogy a korábbi fejlesztésnél nem tűnt fel sehol a hibaüzenetek szövegében. Javítás: 002e064
• Remove this use of "Thread.sleep()".
  Ez régóta feltűnt. Lásd Remove Thread.sleep from PeriodicTask tests. #10 issue-t.
• End this switch case with an unconditional break, return or throw statement.
  Igen, már a többi nyelv (pl. Kotlin) implementációinál is szétbontottam azt a switch szerkezetet külön metódusokra, hogy jobban átlátható legyen, és ne függjön a sorrendtől (a többi nyelv nem is támogatja az ilyen "túlfutó" módszert). A 983582a commitban szétbontottam a Kotlin-os projekt mintája alapján.
• Convert this Map to an EnumMap.
  Nem használtam még EnumMap-ot, de ha már van, tényleg jobb így. Javítás: 3ac7602

A módosítások miatt az egységtesztek lefedettsége kissé romlott, már csak 53.8%-os.

[klenium]

A részfeladat elkészült, összefoglaló a Wikin.