Kokouspöytäkirja 1.4.2002 Ohjelmistotuotantoprojektiryhmä 1. Tapahtumapohjaisten järjestelmien jonomallien visualisointi Kevät 2002 KOODIKATSELMUSKOKOUS Aika Torstai 25.3.2002 klo 17.12. - 18.50 Paikka Helsingin yliopisto, TKTL Teollisuuskatu 25-27, B436 Paikalla Juha Taina, asiakas klo 17.12. - 17.25 Lauri Aarnio, puheenjohtaja Kimmo Airamaa Esko Kupiainen Liisa Länkä, sihteeri Janne Petäjä, klo 17.33 -> Heikki Tuominen Johannes Ukkonen Kokouksessa käytiin läpi TFlow-luokan metodi drawRecArc sekä luokka StateHandler. Projektiryhmä oli etukäteen tutustunut koodiin sekä Eskon laatimaan matemaattiseen aineistoon, mitä tarvitaan rekursiivisten kaarten piirtoon. Kokouksessa käytettiin dataprojektoria, jotta kaikki pystyivät seuraamaan koodia samanaikaisesti. Puheenjohtaja muistutti kokouksen aluksi, että kokouksessa katselmoidaan koodia eikä koodin laatijoita. Koodi drawRecArc käytiin läpi kahteen kertaan: ensimmäisellä kierroksella katselmoitiin tyyliasu, toisella kierroksella toiminnallisuus. StateHandler ehdittiin käydä läpi vain kertaalleen. 1. Kokouksen avaus Puheenjohtaja avasi kokouksen klo 17.12. 2. drawRecArc tyyliasun tarkistus: kommentit, asettelu, nimeäminen Katselmuksessa tuli esille seuraavia muutos- ja parannusehdotuksia: - Otsikko-osassa kannattaa kertoa muutamalla lisärivillä laajemmin siitä, mitä metodi tekee. - Rivi 4: s ei ole kuvaava muuttujan nimi, parempia vaihtoehtoja olisivat: server, vertex, cell. Heikki totesi, että muuttujien nimet olisi kannattanut heti koodauksen alussa suunnitella paremmin, koska niitä on nyt käytetty ohjelman eri osissa epäyhtenäisesti. Sama ongelma on myös muuttujien arc/vector/tFlow/edge kohdalla. - Rivi 8: returnin perässä Poin on turha. - Rivi 10: metodin nimenä olisi voinut ollut kuvaavampi esimerkiksi DrawRecusiveTransmissionArc. Myös muuttujat s.dir ja g kannattaisi nimetä kuvaavammin. - Rivi 13: muuttuja arc_angle nimetty tyyliohjeen vastaisesti. - Rivi 16: muuttuja start_angle nimetty tyyliohjeen vastaisesti. - operaattorin ympäriltä puuttuu välilyönnit. Vastaavia muuttujan nimeämisiä sekä epäyhtenäisyyttä välilyönneissä -,+,/,* -merkkien ympärillä on koodissa muuallakin. Niitä ei käyty katselmuksessa kaikkia yksittäin läpi. - Rivi 19:: muuttuja height_half nimetty tyyliohjeen vastaisesti. - Rivi 20:: muuttuja width_half nimetty tyyliohjeen vastaisesti. - Rivi 22, 23: muuttuja gamma kannattaisi kommentoida sanallisesti laajemmin ja viitata esimerkiksi projektimapista löytyvään piirrokseen, mistä muuttujan käyttötarkoitus käy ilmi. - Rivi 26,28: muuttujat y_e, x_e nimetty tyyliohjeen vastaisesti ja ovat myös liian lyhyitä - kannattaa nimetä kuvaavammilla nimillä. - Rivi 32: muuttuja rad_short nimetty tyyliohjeen vastaisesti, ja nimessä etäisyys olisi kuvaavampi termi kuin säde. - Rivi 60: muuttuja recArcsBefore nimeä parempi olisi getArcNumber. - Rivi 62: Cell center point korvataan Server center point. - Rivi 63: muuttujan nimi serverCenterPoint olisi parempi kuin p - Rivi 65,66: hakasulkeiden ympärillä turhia välilyöntejä. - Rivi 70: arvo 2.25 saatu kokeilemalla. Vakio olisi parempi kuin magic number - Rivi 90: jos radius on vakio, niin käytettävä isoja kirjaimia nimeämisessä. - Rivi 128: kertoimesta 0,65 yritetään päästä eroon. Se on saatu kokeilemalla, mutta on ratkaistavissa matemaattisesti laskentakaavaa tarkistamalla. Esko on oikeastaan ratkaissut jo ongelman. 3. drawRecArc koodin tekninen sisältö Esko esitteli metodin muuttujat ja vakiot. Projektimapissa on kaaviota ja kuvia, joista muuttujat ja niiden käyttötarkoitus käy ilmi. Lisäksi käytiin läpi DrawSelf -metodia sen verran, että nähtiin miten käsiteltävään metodiin päädytään. Koska koodi sisältää paljon matemaattisia kaavoja, on sen teknistä sisältöä vaikea kommentoida. Joitain parannusehdotuksia tuli esille: - Kertoimesta 0,65 tulisi päästä eroon - Käsin reititetyn rekursiivisen kaaren piirrossa on virhe: Kaaria voi piirtää rajattoman määrän: ehtoa enintään 5 kaarta pääilmansuuntaa kohti ei tarkisteta. - Rivi 70,71,128: kokeilemalla saadut piirroksen ulkonäköön vaikuttavat numeroarvot kannattaa muuttaa esimerkiksi luokkavakioiksi. - Rivit 76 ja 77 turhia, arvo laskettu aiemmin riveillä 32 ja 33. - Rivi 97: Esko huomasi, että lausekkeen arc_start_x + width_half voi korvata muuttujalla p.x (ks. Rivi 85) 4. StateHandler koodin tyyliasu Luokan todettiin pääosin olevan hyvää koodia ja tyyliohjeen mukainen. Muuttujat on nimetty kuvaavasti, siksi ne ovat toisinaan huomattavan pitkiä. Koodissa on tästä johtuen joissain paikoin rivitysongelmia. Katselmuksessa ei ollut koodin suhteen huomautettavaa. Seuraavia parannusehdotuksia tuli esille: - Kaikkien koodia muokanneiden henkilöiden nimet eivät näy otsikkoteksteissä. - Rivi 13-15: turhia rivejä, jotka poistetaan. - Rivi 19: sana Inner ei pidä enää paikkaansa: poistetaan. - Rivi 30,114,135: kokonainen rivi *-merkkejä. Tähtien väliin voisi kirjoittaa tilanteen mukaan, esimerkiksi End of States Section - Järjestystä public, protected, private ei olla noudatettu systemaattisesti - Rivi 191,473,475,480,482,483: 80 merkin enimmäismäärä on mennyt rikki muutaman kerran. - Rivi 218-230: ikonia kuvaavat server.gif, qserver.gif,vrctor.gif,begin.gif js end.gif olisi parempi korvata esimerkiksi muuttujilla selected ja notSelected. - Rivi 309: metodista puuttuu kommentti. - Rivi 336,494: sisennettävä käsin, koska emacsin automaattisisennys ei ole tyyliohjeen mukainen. Vastaavanlainen ongelma toistuu muuallakin koodissa. - Rivi 375,434,544: tyhjä rivi ennen komentoa break on epäyhtenäinen. Tyhjä rivi mieluummin break-komennon jälkeen. - Koodin seuraamista hankaloittaa hieman se, että switch case ei mene Staten vaan Actionin mukaan. - Rivi 435: case-lause on pitkä. Voisiko lausetta yrittää jakaa lyhyempiin osiin? - Rivi 474: piirtoon vaikuttava 6000 pixeliä kommentoitava ja mielellään asetettava vakioksi. - Jokaisen case-lauseen alkuun kannattaa lisätä kuvaus siitä, mitä tehdään. - Kommentteja kannattaa kirjoittaa enemmän. - Rivi 635: Virhetilanne, johon ohjelman suorituksen ei tulisi koskaan päätyä. Ilmoitus muutettava tilanteeseen sopivammaksi. 5. StateHandler koodin tekninen sisältö Kommentoitavaa tai huomautettavaa ei löytynyt. 6. Kokouksen päättäminen Puheenjohtaja päätti kokouksen klo 18.50. Pöytäkirjan vakuudeksi Liisa Länkä