TIM - The Interactive Material. Jyväskylän yliopisto

Sovellusprojekti Timppa, 2. lähdekoodin katselmointi

##

Yhteenveto koodista

##

Olennaiset muutokset viime katselmointiin nähden:

  • 10 muokattua tiedostoa
    • timApp/static/scripts/view_html.js
    • timApp/static/scripts/wallView/wallController.js
    • timApp/templates/lectureinfo_menu.html
    • timApp/templates/question.html
    • timApp/templates/view_html.html
    • timApp/tim.py
    • timApp/timdb/lectures.py
    • timApp/timdb/messages.py
    • timApp/timdb/questions.py
  • 22 lisättyä tiedostoa
    • timApp/static/scripts/controllers/lectureInfoController.js
    • timApp/static/scripts/controllers/questionAnswerController.js
    • timApp/static/scripts/controllers/questionShowAnswersController.js
    • timApp/static/scripts/createDialog.js (onko kirjasto?)
    • timApp/static/scripts/createLectureCtrl.js
    • timApp/static/scripts/popUpDialog.js
    • timApp/static/scripts/showChartDirective.js
    • timApp/static/scripts/sidebarMenuCtrl.js
    • timApp/static/scripts/smallMenuCtrl.js
    • timApp/static/scripts/statisticsView/statisticsController.js
    • timApp/static/scripts/timApp.js
    • timApp/static/templates/answersFromStudents.html
    • timApp/static/templates/lectureEnding.html
    • timApp/static/templates/lectureOptions.html
    • timApp/static/templates/questionAskedStudent.html
    • timApp/static/templates/questionNote.html
    • timApp/static/templates/showQuestionTeacher.html
    • timApp/static/templates/start_lecture.html (siirretty)
    • timApp/templates/answers.html
    • timApp/templates/join_lecture.html
    • timApp/templates/lectureInfo.html
    • timApp/templates/lectureinfo_menu_min.html
    • timApp/templates/matrix.html (uudelleen nimetty)
    • timApp/templates/questioninfo.html
    • timApp/timdb/lectureanswers.py
  • lisätyt JS-kirjastot
    • timApp/static/scripts/Chart.min.js
    • timApp/static/scripts/jsDatePick.full.1.3.js
    • timApp/static/scripts/jsDatePick.min.1.3.js
  • TIM-repon mergen yhteydessä tulleet muutokset
  • poistettuja tiedostoja:
    • timApp/templates/matriisi.html (uudelleen nimetty)
    • timApp/templates/start_lecture.html (siirretty)

Yleisiä huomioita HTML:stä

##
  • Tyylit mieluummin CSS-tiedostoon, ei style-attribuuttiin.

  • label-elementtien käyttö joissain kohdissa vielä pielessä. Siis ei näin:

    <label>Vastausaika:</label>
<input type="radio" name="vastausaika" value="0">Ei

    vaan näin:

    <label><input type="radio" name="vastausaika" value="0">Ei</label>

    Toinen (mutta hankalampi) tapa for-attribuutilla:

    `<label for="inputName">Some text</label>`.
  • br-elementtejä ei tulisi käyttää välien saamiseksi, vaan CSS:ää (esim. margin-ominaisuus).
  • Pitkät rivit hyvä jakaa useammalle riville.

  • Muottitiedostoissa joihinkin tiedostoihin viitataan tyyliin ../../static/windowstyle.css mutta parempi olisi url_for-funktio.

Yleisiä huomioita JavaScriptistä

##
  • Puuttuvia puolipisteitä.
  • Globaali console pois (console -> $window.console).
  • Miten tarkistetaan, onko muuttujaa määritelty? Koodin ehtolausekkeissa esiintyy ainakin:
    • singleAnswer == "undefined"
    • scope.question.question == undefined
    • $scope.endDate !== undefined
    • typeof userName === 'undefined' (oikein)
    • angular.isDefined(question.DATA.ROWS) (oikein, paras)
  • Vertailuissa mielellään === eikä ==.

Onhan JSHint PyCharmista päällä?

Yleisiä huomioita Pythonista

##
  • Dokumentaatiokommentteja puuttuu joistain funktioista.
  • Oikeuksien tarkistus puuttuu joistain reiteistä.
  • Parametrien oikeellisuustarkistus.

timApp/tim.py

##
  • get_lecture_answers():
    • Pollauslogiikka hämärä ainakin itselleni.
      • Toimiiko monen prosessin tapauksessa?
      • for-silmukan tarkoitus?
      • wait-kutsuun tarvinnee ainakin timeout-parametrin, ja paluuarvon tarkistus.
    • request.args.get('time') kutsutaan kahdesti, mutta kerta riittää.
    • Tarvitseeko erikseen noAnswer-tapausta?
    • Tarvitseeko erikseen latestAnswer-avainta?
    • Jos parametri question_id, lecture_id tai doc_id muita kuin kokonaislukuja, tulee Internal Server Error (status 500).
    • Kommentit.
  • answer_to_question():
    • Miten tiedetään, että avain (question_id, lecture_id) on olemassa __pull_answer-kokoelmassa?
    • Oikeuksien tarkistus.
    • Jos parametri question_id tai lecture_id muita kuin kokonaislukuja, tulee Internal Server Error (status 500).
    • Kommentit.

timApp/static/scripts/wallView/wall_controller.js

##
  • Funktion document.getElementById("wall") käyttäminen ei ole "Angularmaista". Paras ratkaisu: Wallille oma direktiivi, jolle voisi sanoa luennon id:n ja hoitaisi viestien yms. hakemisen.
  • Nimeäminen
    • wall_controller -> lecture_controller?
    • answer.answers?
  • parseInt-kutsuihin kantaluku, ks. MDN.
  • extendLecture-funktiossa voisi käyttää valmiita päivämäärämetodeja oman toteutuksen sijaan. Esim. nyt karkausvuosia ei huomioida.

timApp/static/scripts/showChartDirective.js

##
  • canvas on turhaan erillisenä elementtinä. Direktiivi voisi luoda sen itse.
  • Parametriksi dataset, joka näytetään.
  • basicSets-taulukon määrittelyssä hieman toistoa. Voisi luoda yhteiset osat silmukassa ja sen jälkeen asettaa värit.
  • replace: "true" -> replace: true
  • transclude turha?
  • Moduuli, johon direktiivi kuuluu, pitäisi hakea Angularilta. Nyt se on globaali muuttuja (timApp).

timApp/static/scripts/createLectureCtrl.js

##
  • getElementById:stä eroon
  • Funktio clearForm:
    • Silmukka ei tee $scopen muuttujille mitään.
    • Miksi $scope.$emit("closeLectureForm");?
  • Funktio submitLecture:
    • Toistoa.
    • Pitkä rivi.

timApp/templates/view_html.html

##
  • createLectureCtrl.js on linkitetty 2 kertaa.
  • ng-class on tarpeeton, jos luokan nimi on vakio. Esim. ng-class="['menu']" -> class="menu".
  • Miksi 3 tyhjää diviä peräkkäin (<div></div><div></div><div></div>)?
  • Typo: useAswers.
  • Luentoihin liittyviä osia voisi laittaa omiin muottitiedostoihin.

timApp/templates/lectureinfo_menu.html

##
  • Rivinvaihtoja oudoissa paikoissa, esim. Show current<br>lectures
  • ../../../ -> url_for(...)

timApp/templates/question.html

##
  • Muotti matriisi.html sisällytetään 4 kertaa -> paljon toistoa selaimelle tulleessa HTML:ssä.

timApp/templates/matrix.html

##
  • textarea-elementissä type="text" on turha.

timApp/static/scripts/view_html.js

##
  • QuestionController ja ShowQuestionController omiin tiedostoihinsa.
  • //TODO use JSON.stringify - samaa mieltä. Eikö Angular muutenkin hoida merkkijonomuunnoksen automaattisesti $http-palvelussa?
  • Typo: answerFiledType.

timApp/static/templates/start_lecture.html

##
  • label-elementtien for-attribuutit väärin; pitäisi viitata id-attribuuttiin eikä name-attribuuttiin. Mutta mieluummin niin, että input on labelin sisällä.

timApp/timdb/(lectures|messages|questions|lectureanswers).py

##
  • __author__ = 'localadmin'?
  • commit-parametrilla voisi aina olla oletusarvo true.
  • Question-taulun sarake on question_id, ei id.
  • "string" -> "str"
  • Merkkijonoissa "" vai ''?

timApp/static/scripts/controllers/questionAnswerController.js

##
  • HTML:n muodostusta merkkijonoilla - voisiko Angularin muotteja käyttää?

timApp/static/scripts/controllers/questionShowAnswersController.js

##
  • Ohjaimen nimi hieman eri kuin tiedoston.