2. lähdekoodin katselmoinnin huomiot

8.6.2016

Yhteenveto koodista

Timber-ryhmän tekemät muutokset:

• 23 muokattua tiedostoa
  • timApp/Dockerfile
  • timApp/containerLink.py
  • timApp/initdb2.py
  • timApp/modules/Haskell/cabal.sandbox.config
  • timApp/pluginControl.py
  • timApp/routes/answer.py
  • timApp/routes/view.py
  • timApp/static/all.scss
  • timApp/static/scripts/answerbrowser3.js
  • timApp/static/scripts/view_html.js
  • timApp/static/templates/answerBrowser.html
  • timApp/static/templates/parEditor.html
  • timApp/templates/document.html
  • timApp/templates/paragraphs.html
  • timApp/templates/view_content.html
  • timApp/templates/view_html.html
  • timApp/test_plugins.py
  • timApp/tim.py
  • timApp/timdb/documents.py
  • timApp/timdb/folders.py
  • timApp/timdb/timdb2.py
  • timApp/timdb/timdbbase.py
  • timApp/utils.py
• 31 lisättyä tiedostoa
  • assessment_area.py
  • modules/cs/comtest.jar
  • modules/cs/comtestcpp.jar
  • modules/cs/java/gui.jar
  • routes/annotation.py
  • routes/velp.py
  • schematimber.sql
  • static/scripts/controllers/reviewController.js
  • static/scripts/directives/annotation.js
  • static/scripts/directives/velpSelection.js
  • static/scripts/directives/velpSummary.js
  • static/stylesheets/velpSelection.scss
  • static/templates/annotation.html
  • static/templates/popOverTemplate.html
  • static/templates/reviewEditor.html
  • static/templates/velpSelection.html
  • static/templates/velpSummary.html
  • static/test_data/curltestdata.txt
  • static/test_data/markings.json
  • static/test_data/markings_old.json
  • static/test_data/phrases.json
  • static/test_data/tags.json
  • static/test_data/updateannotation.txt
  • static/test_data/velp example.txt
  • templates/velp_view.html
  • test_velp.py
  • timdb/annotations.py
  • timdb/icons.py
  • timdb/velpgrouplabels.py
  • timdb/velpgroups.py
  • timdb/velps.py

Yleisiä huomioita JavaScriptistä

• != -> !==, == -> ===, ks. tämä

• Objektien avaimien ei tarvitse olla lainausmerkeissä (muuta kuin joissain erikoistapauksissa).

  Siis esim. placeInfo["start"]["par_id"] -> placeInfo.start.par_id

• Minimoikaa globaalien muuttujien määrä Angularin ohjaimista ja direktiiveistä: jos tarvitsee käyttää esim. window-olion funktiota clearInterval, niin injektoikaa ohjaimeen $window ja sanokaa koodissa $window.clearInterval(...). Globaaleja ovat esim.

  • window
  • document
  • kaikki window:n alla olevat, esim. console

Yleisiä huomioita Pythonista

• Dokumentaatiokommentit aika hyvin kohdillaan; joitain pikkupuutteita.
• Muutamassa kohdassa pieniä PEP8-virheitä.
• abort-kutsuihin voisi laittaa alkuun return, jotta PyCharm ymmärtää, ettei metodin suoritus enää jatku. Muuten voi tulla turha varoitus variable might be referenced before assignment.

static/scripts/controllers/reviewController.js

• Funktion makePostRequest JSDocista puuttuu succesMethod. Huom. myös typo.
• Funktion addAnnotationToElement JSDoc: el vs. id
• getMarkingComments ei aina palauta arvoa.
• getMarkingComments: @returns {Array|*|string|boolean} ehkä kyseenalainen. Eikö comments ole aina taulukko?
• getNodeNumbers: aidFound käyttämätön
• makePostRequest: response käyttämätön
• Merkkijonot highlighted ja ANSWERBROWSER vakioiksi
• Find answer browser and isolate its scope -> ... and its isolate scope
• Jotkut rivit melko pitkiä

routes/velp.py

• return "" -> return okJsonResponse()
• Flaskin reittimääritykseen voi suoraan ilmoittaa, että kyseessä on int: "/<int:document_id>/get_default_velp_group", jolloin muunnosta ei tarvitse tehdä itse
• if xxx is True -> if xxx ja vastaavasti if xxx is False -> if not xxx
• # TODO outdated again - tarkenna, mihin tällä viitataan
• Typo: has_manage_access_access

timdb/velps.py

• update_velp_labels kannattaa muuttaa atomiseksi ottamalla ensimmäinen commit pois
• get_velp_label_ids_for_document: DocStringissä ylimääräinen parametri
• VelpInformation-taulua ei ole olemassa
• Muutama käyttämätön import

timdb/annotations.py

• seconds_since näyttää turhalta, koska muunnoksen voi tehdä kentän comment_time/creation_time avulla
• create_annotation DocString: velp_version_id vs. version_id

routes/annotation.py

• get_annotations: document_id -> doc_id?

test_velp.py

• Hyvännäköinen alustava testi, thumbs up!

• assertit voisi tehdä mahdollisimman tiukoiksi, esim. ei testaisi pelkästään pituutta vaan sisältöäkin

• Viimeinen assert ei mene läpi (tulee AssertionError: 400 != 403).

• Jos dokumentilla ei ole oletusvelppikokoelmaa, kannattaisiko palauttaa mieluummin null (Pythonissa None) eikä -1?

• Testikäyttäjän ryhmää ei kannata kovakoodata vaan hakea:

  gid = db.users. get_personal_usergroup_by_id(session['user_id'])

• format-kutsuissa ei tarvitse str-muunnosta