HannuPekkaHonkanen / AritmetiikanHarjoittelua

Ohjelmoinnin harjoitustyö, touko/kesäkuu 2014
0 stars 0 forks source link

Koodikatselmointi #1

Open Termanty opened 10 years ago

Termanty commented 10 years ago

Koodi oli ladattu 1.6. klo 11.00.

Ohjelma on hyvin jäsennetty osiin. Koodi on selkeää lukea. Luokkien, metodien ja muuttujien nimet on hyvin valittuja. Metodit ovat sopivan mittaisia. Single responsibility periaatetta on koodaamisessa noudatettu. Koodi on jaettu hyvin ja loogisesti eri pakkauksiin. Luokka kaavio antaa selkeän kuvan ohjelman rakenteesta.

Testeissä voisit miettiä miten pääsisit eroon copypaste testien tekemisestä. Testaat oikeaa toimintaa, kannataisiko testata myös virheellistä toimintaa? Tosin se voi olla hieman aikaista, jos ohjelma ei hallitse virheellisiä tilanteita hyvin.

Toiminnallisuutta on toteutettu vielä varsin vähän, joten koodaamista on vielä runsaati edessä.

Parannus ehdotuksia: luokka Tekstikayttoliittyma, metodi kaynnista() Tätä metodia voisi vielä tiivistää. Esim. pilkkomalla useammaksi metodiksi.

luokka Laskutoimitustehdas, metodi uusilaskutoimitus() Tässä voisi käyttää switch-case rakennetta if-else if:n sijaan. Tätä metodin voisi jakaa pienempiin osiin.

luokka Laskutoimitustehdas, metodi arvoLuvut() Arpoja luokan instanssi tehdään joka kerta uudestaan. Tämä tuskin lienee tarpeellista. Jos Arpoja olisi luokka muuttuja niin sen voisi luoda konstruktorissa.

luokka Arpoja, metodi kokonaisluku() Tässä arvotaan kokonaislukuja väliltä -10 ja 10. Voiskohan metodin kirjoittaa niin että se olisi lukijalle itsestään selvää ja niin että metodista tulisi yleikäyttöisempi - voisi arpoa lukuja miltä tahansa väliltä.

luokka enum Laskutoimitustyyppi() Kannattaa opetella käyttämään enum luokkia. Luokan nimen voisi lyhentää Operaatioksi. Tällöin kutsuisit vain Operaatio.SUMMA

HannuPekkaHonkanen commented 10 years ago

Kiitos perusteellisesta palautteesta!

Paljon hyviä parannusehdotuksia. Osa ollut jo itsellä mielessä tai
saatu samanlaista palautetta ohjaajilta, mutta myös uusia ehdotuksia
joukossa.

Ja totta, koodaamista on vielä paljon edessä.

Hannu

Lainaus Tero Mäntylä notifications@github.com:

Ohjelma on hyvin jäsennetty osiin. Koodi on selkeää lukea. Luokkien,
metodien ja muuttujien nimet on hyvin valittuja. Metodit ovat
sopivan mittaisia. Single responsibility periaatetta on
koodaamisessa noudatettu. Koodi on jaettu hyvin ja loogisesti eri
pakkauksiin. Luokka kaavio antaa selkeän kuvan ohjelman rakenteesta.

Testeissä voisit miettiä miten pääsisit eroon copypaste testien
tekemisestä. Testaat oikeaa toimintaa, kannataisiko testata myös
virheellistä toimintaa? Tosin se voi olla hieman aikaista, jos
ohjelma ei hallitse virheellisiä tilanteita hyvin.

Toiminnallisuutta on toteutettu vielä varsin vähän, joten
koodaamista on vielä runsaati edessä.

Parannus ehdotuksia: luokka Tekstikayttoliittyma, metodi kaynnista() Tätä metodia voisi vielä tiivistää. Esim. pilkkomalla useammaksi metodiksi.

luokka Laskutoimitustehdas, metodi uusilaskutoimitus() Tässä voisi käyttää switch-case rakennetta if-else if:n sijaan. Tätä
metodin voisi jakaa pienempiin osiin.

luokka Laskutoimitustehdas, metodi arvoLuvut() Arpoja luokan instanssi tehdään joka kerta uudestaan. Tämä tuskin
lienee tarpeellista. Jos Arpoja olisi luokka muuttuja niin sen voisi
luoda konstruktorissa.

luokka Arpoja, metodi kokonaisluku() Tässä arvotaan kokonaislukuja väliltä -10 ja 10. Voiskohan metodin
kirjoittaa niin että se olisi lukijalle itsestään selvää ja niin
että metodista tulisi yleikäyttöisempi - voisi arpoa lukuja miltä
tahansa väliltä.

luokka enum Laskutoimitustyyppi() Kannattaa opetella käyttämään enum luokkia. Luokan nimen voisi
lyhentää Operaatioksi. Tällöin kutsuisit vain Operaatio.SUMMA


Reply to this email directly or view it on GitHub: https://github.com/HannuPekkaHonkanen/AritmetiikanHarjoittelua/issues/1