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

These are the current permissions for this document; please modify if needed. You can always modify these permissions from the manage page.