Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vertaisarviointi #1

Open
mhaapakangas opened this issue Jun 5, 2019 · 0 comments
Open

Vertaisarviointi #1

mhaapakangas opened this issue Jun 5, 2019 · 0 comments

Comments

@mhaapakangas
Copy link

mhaapakangas commented Jun 5, 2019

Projekti ladattu: 5.6. klo 21

Algoritmin suoritus onnistui, ja se tuotti esimerkkikuvista selvästi tarkemman version. Se, miten tarkan kuvan algoritmi tuottaa, riippuu kuitenkin kuvan sisältämistä väreistä sillä algoritmi huomioi ainoastaan vihreän värikanavan.

Ohjelman logiikkaa oli suurimmaksi osaksi helppo ymmärtää, mutta siitä saisi vielä selkeämmän jakamalla koodiin pienimpiin metodeihin ja luokkiin. Esimerkiksi FocusStacking-luokan Stack-metodin voisi pilkkoa pienempiin osiin. Lisäksi jäi hieman epäselväksi, mitä osia algoritmista on vielä toteuttamatta koska sen kulku oli erilainen kuin Toteutus-dokumentin kulkukaaviossa ja projekti sisälsi poiskommentoitua koodia. Esimerkiksi ylipäästösuodatusta en löytänyt projektista.

MyArrayList-luokan käyttöä voisi optimoida lisäämällä konstruktorin, jolla voi asettaa luokan sisäisen taulukon alkukapasiteetin. Alustamalla taulukon koon oikeaksi vältytään datan kopioimiselta uudelleen aina kun taulukon kokoa pitää kasvattaa. Taulukon haluttu koko on tiedossa jo etukäteen, sillä se riippuu kuvan koosta, minkä ansiosta MyArrayList-luokan voisi luultavasti myös korvata Javan omalla taulukolla.

Yksi mahdollinen koodiparannus olisi käyttää MyArrayList-luokan sisäisessä taulukossa geneeristä luokkaa E Object-luokan sijaan jolloin vältyttäisiin tyyppimuunnoksilta (cast). FFT-luokan fft-metodissa käytettiin Javan omia Math.cos ja Math.sin metodeja. Kuuluisiko nämä korvata omilla implementaatiolla? Jos Math-kirjaston käyttö on sallittua, ei luultavasti muitakaan apumetodeja (sqrt, abs, hypot) tarvitsisi toteuttaa itse.

Koodia oli melko helppo ymmärtää, mutta sen luettavuutta voisi vielä parantaa kiinnittämällä enemmän huomiota metodien nimiin. Jos nimet ovat tarpeeksi kuvaavia, on myös kommenttien tarve pienempi. Esimerkiksi kohdasta

 // Compute FFT
 Complex[][] fft = FFT.fft2(window);

voisi tehdä hieman selkeämmän vaikka seuraavasti:

 Complex[][] fft = FFT.compute2dFft(window);

Jos koodia haluaa hioa, voi siihen tehdä pieniä tyyliparannuksia. Esimerkiksi joidenkin metodien nimet alkoivat isolla kirjaimella.

Projektin testikattavuus oli yksinkertaisempien luokkien osalta melko hyvä, mutta esimerkiksi FocusStacking-luokan logiikka oli vielä testaamatta. Koska Stack-metodi sekä lataa kuvat, käsittelee ne että lopussa tallentaa uuden kuvan, sen testaaminen on haastavaa. Myös tästä syystä koodi kannattaisi jakaa pienempiin metodeihin ja useampiin luokkiin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant