Code review 1
Deze code review is uitgevoerd door Joost Laurman op 02-10-2018
Code review
Orginele bestand: hier
- De lang is aparte folder?
- Wat doet de
http
folder,nodeRequest
? - Waarom staat
Platform
inutils
map RiverScoutUtils
staat in route, terwijl dit eenutil
isRoute
is een aparte map, dit zoulogic
moeten zijn. Alle logica staat in deze manier. HernoemRoutePlanner
naarRoutePlannerLogic
- Voeg ook wat algemene informatie toe aan de
javadoc
bovenaan de bestanden met korte beschrijving waarvoord it bestand dient. - Vergeet ook niet de
javadocs
boven alle methodes, ook de private methodes! - Meer commentaar in de code; over een maand weet je niet meer wat je hebt geschreven en is ook makkelijker te begrijpen voor iemand die nieuw met het project begint.
println()
zal niet in de logs terecht komen. Kijk dus even naar een logging library (Logger
,KLogging
)
RouteController
Requestmapping
hoort nietget
te zijn, maarroutes
- Bij de
getRoute
kan je deRequestmapping
weghalen - Probeer op te vangen als er velden niet zijn ingevuld. Nu komt er een 500 error.
Platform.kt
- Veel in
Platform.kt
kan worden onderverdeeld in bijvoorbeeldShipUtils
,RouteUtils
etc. - Nog geen unit tests in
core
project
RWSRISReader
PATH
,DELIMITER
enSTART_FROM
zijn constants- Let op de character limit
MyJavaHopper
- Hernoemen
MyJavaHopper
Todo
zonder wat er moet gebeuren; dan gewoon weghalen.
Acties
- Gebruik van
http
folder ennodeRequest
zijn uitgelegd. (Wordt alleen gebruikt voor het handmatig testen) - Bestanden zijn verplaatst en hernoemd zodat deze correct zijn.
- RoutePlanner
=>
RoutePlannerLogic - MyJavaHopper
=>
RouteCreator - Etc.
- RoutePlanner
println()
zijn vervangen door loggen metKotlinLogging
Requestmapping
is veranderd naarroutes
- Error messages bevatten meer informatie en er wordt gebruik gemaakt van de juiste HTTP-statuscodes
PATH
,DELIMITER
enSTART_FROM
zijn veranderd naar constants- Character limits in bestanden worden niet meer overschreden
- Alle bestanden bevatten een
javadoc
alsheader
en alle functies zijn gedocumenteerd en zijn voorzien van comments (als dat nodig was). - Unit tests zijn geschreven voor alle modules;
core
,graph
engraphhopper
- Onderdelen die niet nodig waren uit
platform
zijn verwijderd; alleenShipUtils
voor het berekenen van CEMT-klasse zijn bewaard. - Alle
TODO's
zijn voorzien van commentaar.