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

Koodikatselmointi #1

Closed
Eversor11 opened this issue Dec 1, 2013 · 0 comments
Closed

Koodikatselmointi #1

Eversor11 opened this issue Dec 1, 2013 · 0 comments

Comments

@Eversor11
Copy link

Ladattu 1.12.2013 klo 14:45

Tervehdys! Ota kommenteissa huomioon, että olen sivuaineopiskelija ja servletit sun muut on täysin uutta. Javaakaan en ole ennen tätä pahemmin koodannut, mutta toivottavasti täältä löytyy edes jotain sinulle.

Latasin reposta zipin ja buildasin Netbeansin kautta, kun Mavenia ei ole. Toimi hyvin sekin. Sain runnattua ohjelmaa, mutta jäi vaiheeseen: SelectChannelConnector@0.0.0.0:8080 STARTING,
joten todennäköisesti en vain osannut localhostia. Ajoin siis komentorivillä:

java -jar target/dependency/jetty-runner.jar --port 8080 target/jlab.war

Käyttöohjeessa tuon jälkimmäisen targetin edessä oleva slash on tilannekohtaisesti ylimääräinen? Ainakin itselläni herjasi ennenkuin sen poisti (git bashin kautta ajoin). Käyttöohje ei vaikuta kauhean projektikohtaiselta, joten sitä voisi täsmentää?

Yleisesti

Ohjelmakoodi näytti siistiltä. Nimeämiset olivat selkeitä eivätkä metodit olleet pitkiä.
Luokilla oli omat selkeät vastuut ja pakkausjako oli toimiva. Testit olivat selkeitä, eikä niitä tarvinnut sen kummemmin ihmetellä. Luokkakaavio ja sekvenssikaaviot olivat myös helpot ymmärtää. Kaikki testit menivät mukavasti läpi.

JavaDoc:sta

Koodi näytti hyvin dokumentoidulta, seuraavassa kehitysehdotuksia:

TwitterCache.java:n luokkakuvaus sisältää konstruktorin parametrin selvennystä, voisiko tämän siirtää konstruktorin kuvaukseen, jotta olisi selkeämpi?

HitEvaluator.java:n setUrl-metodin toimintaa voisi kuvata, että olisi helpompi ymmärtää mitä metodin kutsussa tarkasti ottaen tapahtuu.

TwitterEvaluator.java:n luokkakuvauksessa olevat ckey ja csecret eivät esiinny sellaisenaan luokassa taikka config.ini:ssä. Ne siis pitäisi vaihtaa? Konstruktorin toimintaa voisi kuvata, vaikka aika kuvaavia nuo metodikutsut ovat.

Loppusanat

Tekemistä näyttää vielä riittävän, mutta aihe on mielenkiintoinen. Omalle skillitasolleni liian huima projekti. Suosittelen https://www.websequencediagrams.com/ :n käyttöä sekvenssikaavioiden piirtämiseen. Ainakin itselläni toimi nopeammin kuin käsin vääntäminen, varsinkin jos virheitä sattuu. Mukavat Joulut!

@narkkil narkkil closed this as completed Jun 11, 2014
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

2 participants