search

xanxan / Matrix-calculator

Matrix calculator (a data structures and algorithms course work). Not completed version.
0 stars 0 forks source link

Koodikatselmointi #1

Open jakaarl opened 9 years ago

jakaarl commented 9 years ago

Koodikatselmointi

Koodi päivitetty reposta to 12.2. klo 20.

Projektikonfiguraatio

Projekti ei käänny Mavenilla komentoriviltä tai Eclipseen tuotuna ilman muutoksia, koska Mavenin oletus-JDK on versio 1.5. Ongelma on helposti korjattavissa lisäämällä Mavenin kääntäjälle asianmukainen konfiguraatio, tyyliin:

<build>
  <plugins>
    <plugin>
      <artifactId>maven-compiler-plugin</artifactId>
      <version>2.0.2</version>
      <configuration>
        <source>1.7</source>
        <target>1.7</target>
      </configuration>
    </plugin>
  </plugins>
</build>

Koodin jäsennys

Ohjelmakoodi ei jäsentelyn, nimeämisen tai muunkaan kannalta tarjoile juurikaan yllätyksiä, mikä on ilman muuta hyvä asia ("principle of least astonishment"). Metodit ovat riittävän lyhyitä, kuten myös luokat.

Koodin tyyli

Omaan silmääni "suomenkielinen" koodi sattuu, mutta se lienee makuasia. Suomenkielisissäkin muuttujien ja metodien nimissä tulisi silti noudattaa ohjelmointikielen koodaustapaa; Javassa tämä tarkoittaa "camelCaseNimeämistä".

Matriisi-rajapinnassa ja sen toteutuksessa monet metodit palauttavat matriisin kaksiulotteisena taulukkona - Javalle ja olio-ohjelmoinnille luontevampaa olisi palauttaa uusi matriisi-olio, samaan tapaan, kuin esimerkiksi java.lang.Stringin metodit palauttavat uuden Stringin. Oliomaisen tyylin lisäksi tälläisestä on etuna metodien ketjutettavuus: edellisen metodin tulokselle voidaan kutsua metodia. Matriisi ei ole muokattavissa ("immutable"), mikä on hyvä suunnitteluratkaisu ja seurailee mainitun Stringin esimerkkiä.

Paikoitellen koodi on tarpeettoman koukeroista: esimerkiksi Matriisin metodi koontarkistus() palauttaa booleanin, jonka ollessa false haaraudutaan heittämään poikkeus - olisi selkeämpää ja vähäsanaisempaa, jos koontarkistus() booleanin palauttamisen sijaan heittäisi suoraan poikkeuksen.

Testikoodissa vilisee ei-kuvaavia muuttujanimiä, ja se on muutenkin luettavuudeltaan varsinaista sovelluskoodia kehnompaa. Metodit on nimetty tyyliin "testXyz()" - historiallisesti testimetodit on nimetty siten, mutta nimeämiskäytäntö on jäänyt tarpeettomaksi Test-annotaation myötä. Yleinen ja hyvä käytäntöä on nimetä testimetodit siten, että ne kuvaavat testattavaa asiaa, esim. "shouldThrowExceptionIfInvalidParam()", "shouldReturnEmptyListIfNoResults()".

xanxan commented 9 years ago

Kiitos palautteesta, siitä on paljon hyötyä ja tulee tarpeeseen! Osan mainitsemistasi puutteista olenkin jo ehtinyt korjata, mutta muutamia juttuja en olisi itse varmaan hoksannut. "Suomenkielinen" koodi tulee itseltä niin automaattisesti että olin jo ehtinyt kirjoittaa puolet ohjelmasta ennnenkuin mietin että olisi ehkä kannattanut nimetä muuttujat, metodit ja luokat englanniksi. Onhan se kieltämättä hieman epäjohdonmukaista, kun tuloksena on väistämättä koodi jossa esiintyy sekä suomea että englantia.