Talk:Kirves: Difference between revisions

From ICO wiki
Jump to navigationJump to search
Poras (talk | contribs)
Aakke (talk | contribs)
No edit summary
Line 36: Line 36:
<br>
<br>
Võib väita, et meeskond on püstitatud ülesandega edukalt ning korrektselt hakkama saanud, täidetud on kõik ettemääratud kriteeriumid.
Võib väita, et meeskond on püstitatud ülesandega edukalt ning korrektselt hakkama saanud, täidetud on kõik ettemääratud kriteeriumid.
==Teenuse ja klientrakenduse retsensioon meeskonna "Vargamäe" poolt==
Avades solutioni tekkis kohe arusaamatus, kuna klientrakendus ja teenus on ühes projektis koos(WebApiCocain), kuigi projektinime järgi ütleks, et tegemist on API’ga. Siinkohal tekkis kohe küsimus, et mis põhjusel on need kaks projekti koos, kui oleks saanud ka need täiesti lahus hoida, sest kui struktuuri vaadata, siis kliendi (MVC) osa selles projektis kasutab API’d nii nagu tavaks on (läbi HttpClienti) – ehk siis ei leia ühtegi mõjuvat põhjust, et need kaks projekti koos hoida.
===Veebiteenus===
Käivitades meeskonna poolt loodud veebiteenuse ja klientrakenduse (kuna need on ühes projektis koos, tuli seda paratamatult teha), tekkis esmalt soov näha dokumentatsiooni API kohta, et milliseid meetodeid on võimalik kasutada, et saada üldist aimu teenusest – kuid vastu vaatas klientrakendus. Sellest segamata sai leitud üles dokumentatsiooni lehekülg, kuid sealt ei olnud kokkuvõttes võimalik välja lugeda seda, et millist informatsiooni teenus vastu võtab ja tagastab (mõnes kohas oli)  - see on selle pärast kehv, kuna kui keegi teine peaks hakkama teie teenuse vastu klienti ehitama siis nad ei tea, millist informatsiooni nad saatma peavad või tagasi saavad. Tagasi saadavat informatsiooni saab muidugi GET päringuga kontrollida ja kui muutujate nimed on mõistlikud, siis ei tohiks tekkida ka küsimust, mis midagi näitab.
Sellest tulenevalt, et teenuse dokumentatsioon kohati näitas vastuvõetavat ja tagasisaadetavat informatsiooni sai uurima hakatud API kontrollereid. Viga oli ilmselge, kohati tagastas teenus lihtsalt kollektsiooni objektidest (nt lihtlabane GET päring, mis tagastas kogu informatsiooni), teine kord aga HttpResponseMessage’it või IhttpActionResult’i. Kuid korrektne oleks alati kasutada tagastuseks üht formaati – üldpilt oleks selgem. Selleks, et vältida seda API dokumentatsiooni tagastusväärtuste puudumisviga oleks lihtsalt tulnud HelpPageConfig.cs’i kirjutada vastavad read koodi või kasutada meetodi peal attribuuti ResponseType’i, mille abil saab määrata tagastatava informatsiooni tüübi (näiteks string või teie teenuse puhul VehicleModelDTO).
Vaadates API kontrollereid jäi kohe silma see, et kasutatakse ära eraldi loogika kihti, et kontrolleris oleks minimaalselt koodi. See on väga positiivne kuid kohati on sellest mööda hiilitud ning on otse kontrollerisse kirjutatud koodiloogikat. Kui natuke koodi kirjutada ühte kohta ja natuke teisse kohta, siis see teeb olukorra veel segasemaks kui lihtsalt kirjutada kõik loogika kontrollerisse.
Teenuse POST ja PUT meetodite kohapealt jäi silma täielik valideerimise puudumine ning lisaks on jäetud võimalus kaasa anda ID. Veelgi enam puudub kontroll selle üle, kas POST meetodiga sissetulev ID, mis viitab teisele tabelile reaalselt eksisteerib või mitte. Kasutatud on küll DTO’sid kuid neid ainult andmete tagastamiseks. Üldiselt on ikka tavaks kasutada DTO’sid ka andmete vastuvõtmiseks, sest sageli ei ole vaja anda võimalust saata teenusesse kõiki andmeid (näiteks just see ID koht POST meetodi juures). Eriti halb on näiteks selline olukord, kus nende andmetega on võimalik muuta iseenda konto õigusi.
Teenust uurides tuli päevavalgel üks väga kohutav viga, mida üldse teha võib. Nimelt puuduvad igasugused piirangud andmete muutmiseks. Üks registreerunud kasutaja võib teha kõike – näiteks on võimalik kellegi teise auto enda nimele kirjutada.
Lisaks jäi veel silma, et igas kontrolleris on ülekirjutatud Dispose meetod, kus sees pannakse kinni Uow. Kuid kui vaadata Ninjecti konfiguratsiooni, siis seal on Uow juures kirjas InRequestScope. Kui dokumentatsiooni lugeda Ninjecti kohta, siis sealt tuleb välja, et selle abil luuakse iga päringuga uus instants ning kui sellel instantsil on olemas Dispose meetod, siis pannakse see automaatselt kinni.
Kogu selle kriitika varjus, mis siiani on olnud saab rääkida ka positiivsetest külgedes. Vast kõige positiivsem külg on see, et on olemas andmebaasi konfiguratsioon ning lisaks on teenus ära jaotada paljude layerite vahel ning et on kasutatud erinevaid mustreid.
Kokkuvõtteks võib öelda, et mõned meetodid on väga korralikult tehtud ja teised mitte – tekib arvamus, et mõni tiimi kuulnud liige ei olnud kõigest korrektselt aru saanud ja paistab, et teised ei juhtinud ka selle tähelepanud. Väga võimalik on, et selline olukord tekkis lõpuks ajapuuduse tõttu.
===Klientrakendus===
Klientrakendust kasutama hakates tekkis kohe üks väga suur probleeme. Nimelt oli soov luua konto, et näha ka kasutajale mõeldud liidest. Selleks sai täidetud registreerimise vorm. Pärast nupu vajutust avanes taaskord registreerimise vaade ilma ühegi teavitusega – polnud aru saada kas konto loomine õnnestus või mitte. Sisse logides vaates vastu veateate leht, et ei olnud võimalik Json’ist välja lugeda token’it. Uurides andmebaasi oli näha, et kasutajat ei loodudki. Pärast natukest uurimist sai selgeks, et klientrakenduse ja teenuse valideerimine ei ole vastavuses. Klientrakendus nõudis, et parool oleks vähemalt 6 tähemärki pikk aga teenus see eest lisaks veel suurt tähte ning ka numbrit. Seda teades registreerumine õnnestus.
Rakenduse UI pool jättis väga positiivse mulje – ette heidaks ainult seda, et lisamisvaated oleks võinud ka koos olla kõrvalmenüüga.
Vaadete kontrolleritest jäi kohe esimese asjana silma see, et pole kasutatud eraldi kihti teenusega suhtlemiseks, millest tulenevalt on väga palju korduvat koodi. Samuti tekkis küsimus, et milleks ei ole loodud eraldi sätete faili, kus oleks konstandina kirjas teenuse URL. Kui see peaks muutuma, siis hetkel oleks seda väga tülikas teha – ja enamgi veel kui keegi teine arendaja peaks seda tegema, siis ta ei pruugi teada kõiki kohti, kuhu see kirjutatud on (siin võib küll väita, et on olemas find and replace aga see selleks).
Kontrolleritest jäi silma veel ka andmebaasi ühenduse loomine aga seda ei kasutada kordagi – tõenäoliselt on see kogemata sinna ununenud.
Turvalisuse kohapealt on ühe asjaga samuti alt mindud – nimelt kasutaja sessioon lõpetatakse ainult klientrakenduses, teenuses aga jääb token endiselt kehtima.
Kokkuvõtteks võib öelda, et töö jätab väga läbimõtlemata mulje ja tekib küsimus, et kas neil endal ei tekkinud küsimusi koodi kopeerides, et võiks teha eraldi kihi teenusega suhtlemise jaoks.

Revision as of 11:52, 31 May 2014

XML failide retsensioon meeskonna "TÜC" poolt

Meeskonna "Kirves" xml failid on tehtud raamatukogu kohta.

Neil on tehtud 1 XML fail, 1 XMLi skeemifail ja 2 XSLT faili, nii nagu ülesandes nõutud.

Raamatukogus olevaid teavikuid on võimalik jaotada kategooriate järgi. Võimalikud kategooriad on XMLis eraldi välja toodud, milleks on lauamäng, raamat ja ajakiri. Raamatukogus olevad teavikud võivad olla erinevates keeltes. Teavikutel on erinevad grupid, mis eraldavad need teavikud keelte järgi. Selleks on pandud grupi atribuudiks vastavate teavikute keel.

Igal teavikul peavad olemas olema kohustuslikud atribuudid nimega id, saadavus, autor, pealkiri ja aasta. Lisaks on teavikul määratud asukoht raamatukogus, kus atribuudina on kood, mis on kohustuslik.

Igal teavikul on oma zanrid, neid zanreid võib olla mitu ja nende sisu on ümbritsetud CDATAga, mis on hea, sest siis ei teki zanrite sisestamisega probleeme.

XML failis on kasutatud enamat kui 4 loogilist dimensiooni. Failis on kasutatud hulgaliselt atribuute erinevatel tasemetel ja kasutatud atribuudid on keerulisemald kui lihtsalt id lisamine. XMLi skeemifailis on üle vaadatud automaatselt genereeritud muutujate tüübid ja need soblikumaks tehtud.

Meie tiimi meelest oleks võinud XML failis lisaks ka teaviku atribuudile pealkiri CDATA ümber lisada. Esimene XSLT fail kuvab teavikud kategooriate järgi.

Kõik XML failis olevad kategooriad käiakse läbi ja kirjutatakse nende väärtus HTML listi. Iga kategooria alla tehakse alam list sellesse kategoorasse kuuluvate teatmike pealkirjaga. Selles XSLT failis kasutatakse nii tsüklite kui muutujate tegemist ning tingimuste testimist ja väärtuste välja kuvamist.

Meeskonna “Kirves” teine XSLT fails on välja toodud veidi rohkem andmeid. Seal kuvatakse teavikud nende tähestikulises järjekorras ja olemasolevad andmed nende teavikute kohta. Selles failis on samuti kasutatud tsükleid, muutujaid ja väärtuste välja kuvamist. Lisaks on kasutatud ka sorteerimist.

Meie meeskonna arvates on tegemist põhjaliku tööga ning kõik nõutud kriteeriumid on samuti täidetud.

XML failide retsensioon meeskonna "MRPD" poolt

Retsentseerisime meeskonna „Kirves“ loodud XMLi kui ka XSLT-faile.

Meeskond on oma teemaks valinud raamatukogu ning selles leiduvad teavikud.

XML on loodud korrektselt ning väga hästi läbimõeldud, kasutades arvukalt loogilisi dimensioone. XML failile lisab sisu kategooriate kasutamine, mis võimaldab ka mitmekesisemat pärimist. Kasutatud on ka id-sid, zanre. Positiivsena võib veel välja tuua CDATA kasutamise, mis hoiab pära XML parsimise errorid.

XML skeemifailis olevad väärtustüübid on õigesti valitud, raiskamata liialt mäluruumi.

XSLT faile on loodud kaks ning nende päringud on erinevad, lisaks on XSLT päringud loodud korrektselt ning mitmekesiselt. Esimesel juhul päritakse teavikud kategooriate järgi ning teisel juhul moodustatakse teavikutest tähestikulises järjekorras nimekiri. Pärimisel on kasutatud on erinevaid võtteid – nii foreach kui ka sort, samuti on loodud ka vahemuutujad pärimise lihtsustamiseks.

Failid on selgesti loetavad ning korrektselt vormistatud, XML failis jäetud vahetühikud muudavad koodi lugemise hõlbsamaks.

Võib väita, et meeskond on püstitatud ülesandega edukalt ning korrektselt hakkama saanud, täidetud on kõik ettemääratud kriteeriumid.

Teenuse ja klientrakenduse retsensioon meeskonna "Vargamäe" poolt

Avades solutioni tekkis kohe arusaamatus, kuna klientrakendus ja teenus on ühes projektis koos(WebApiCocain), kuigi projektinime järgi ütleks, et tegemist on API’ga. Siinkohal tekkis kohe küsimus, et mis põhjusel on need kaks projekti koos, kui oleks saanud ka need täiesti lahus hoida, sest kui struktuuri vaadata, siis kliendi (MVC) osa selles projektis kasutab API’d nii nagu tavaks on (läbi HttpClienti) – ehk siis ei leia ühtegi mõjuvat põhjust, et need kaks projekti koos hoida.

Veebiteenus

Käivitades meeskonna poolt loodud veebiteenuse ja klientrakenduse (kuna need on ühes projektis koos, tuli seda paratamatult teha), tekkis esmalt soov näha dokumentatsiooni API kohta, et milliseid meetodeid on võimalik kasutada, et saada üldist aimu teenusest – kuid vastu vaatas klientrakendus. Sellest segamata sai leitud üles dokumentatsiooni lehekülg, kuid sealt ei olnud kokkuvõttes võimalik välja lugeda seda, et millist informatsiooni teenus vastu võtab ja tagastab (mõnes kohas oli) - see on selle pärast kehv, kuna kui keegi teine peaks hakkama teie teenuse vastu klienti ehitama siis nad ei tea, millist informatsiooni nad saatma peavad või tagasi saavad. Tagasi saadavat informatsiooni saab muidugi GET päringuga kontrollida ja kui muutujate nimed on mõistlikud, siis ei tohiks tekkida ka küsimust, mis midagi näitab.

Sellest tulenevalt, et teenuse dokumentatsioon kohati näitas vastuvõetavat ja tagasisaadetavat informatsiooni sai uurima hakatud API kontrollereid. Viga oli ilmselge, kohati tagastas teenus lihtsalt kollektsiooni objektidest (nt lihtlabane GET päring, mis tagastas kogu informatsiooni), teine kord aga HttpResponseMessage’it või IhttpActionResult’i. Kuid korrektne oleks alati kasutada tagastuseks üht formaati – üldpilt oleks selgem. Selleks, et vältida seda API dokumentatsiooni tagastusväärtuste puudumisviga oleks lihtsalt tulnud HelpPageConfig.cs’i kirjutada vastavad read koodi või kasutada meetodi peal attribuuti ResponseType’i, mille abil saab määrata tagastatava informatsiooni tüübi (näiteks string või teie teenuse puhul VehicleModelDTO).

Vaadates API kontrollereid jäi kohe silma see, et kasutatakse ära eraldi loogika kihti, et kontrolleris oleks minimaalselt koodi. See on väga positiivne kuid kohati on sellest mööda hiilitud ning on otse kontrollerisse kirjutatud koodiloogikat. Kui natuke koodi kirjutada ühte kohta ja natuke teisse kohta, siis see teeb olukorra veel segasemaks kui lihtsalt kirjutada kõik loogika kontrollerisse.

Teenuse POST ja PUT meetodite kohapealt jäi silma täielik valideerimise puudumine ning lisaks on jäetud võimalus kaasa anda ID. Veelgi enam puudub kontroll selle üle, kas POST meetodiga sissetulev ID, mis viitab teisele tabelile reaalselt eksisteerib või mitte. Kasutatud on küll DTO’sid kuid neid ainult andmete tagastamiseks. Üldiselt on ikka tavaks kasutada DTO’sid ka andmete vastuvõtmiseks, sest sageli ei ole vaja anda võimalust saata teenusesse kõiki andmeid (näiteks just see ID koht POST meetodi juures). Eriti halb on näiteks selline olukord, kus nende andmetega on võimalik muuta iseenda konto õigusi.

Teenust uurides tuli päevavalgel üks väga kohutav viga, mida üldse teha võib. Nimelt puuduvad igasugused piirangud andmete muutmiseks. Üks registreerunud kasutaja võib teha kõike – näiteks on võimalik kellegi teise auto enda nimele kirjutada.

Lisaks jäi veel silma, et igas kontrolleris on ülekirjutatud Dispose meetod, kus sees pannakse kinni Uow. Kuid kui vaadata Ninjecti konfiguratsiooni, siis seal on Uow juures kirjas InRequestScope. Kui dokumentatsiooni lugeda Ninjecti kohta, siis sealt tuleb välja, et selle abil luuakse iga päringuga uus instants ning kui sellel instantsil on olemas Dispose meetod, siis pannakse see automaatselt kinni. Kogu selle kriitika varjus, mis siiani on olnud saab rääkida ka positiivsetest külgedes. Vast kõige positiivsem külg on see, et on olemas andmebaasi konfiguratsioon ning lisaks on teenus ära jaotada paljude layerite vahel ning et on kasutatud erinevaid mustreid.

Kokkuvõtteks võib öelda, et mõned meetodid on väga korralikult tehtud ja teised mitte – tekib arvamus, et mõni tiimi kuulnud liige ei olnud kõigest korrektselt aru saanud ja paistab, et teised ei juhtinud ka selle tähelepanud. Väga võimalik on, et selline olukord tekkis lõpuks ajapuuduse tõttu.

Klientrakendus

Klientrakendust kasutama hakates tekkis kohe üks väga suur probleeme. Nimelt oli soov luua konto, et näha ka kasutajale mõeldud liidest. Selleks sai täidetud registreerimise vorm. Pärast nupu vajutust avanes taaskord registreerimise vaade ilma ühegi teavitusega – polnud aru saada kas konto loomine õnnestus või mitte. Sisse logides vaates vastu veateate leht, et ei olnud võimalik Json’ist välja lugeda token’it. Uurides andmebaasi oli näha, et kasutajat ei loodudki. Pärast natukest uurimist sai selgeks, et klientrakenduse ja teenuse valideerimine ei ole vastavuses. Klientrakendus nõudis, et parool oleks vähemalt 6 tähemärki pikk aga teenus see eest lisaks veel suurt tähte ning ka numbrit. Seda teades registreerumine õnnestus.

Rakenduse UI pool jättis väga positiivse mulje – ette heidaks ainult seda, et lisamisvaated oleks võinud ka koos olla kõrvalmenüüga.

Vaadete kontrolleritest jäi kohe esimese asjana silma see, et pole kasutatud eraldi kihti teenusega suhtlemiseks, millest tulenevalt on väga palju korduvat koodi. Samuti tekkis küsimus, et milleks ei ole loodud eraldi sätete faili, kus oleks konstandina kirjas teenuse URL. Kui see peaks muutuma, siis hetkel oleks seda väga tülikas teha – ja enamgi veel kui keegi teine arendaja peaks seda tegema, siis ta ei pruugi teada kõiki kohti, kuhu see kirjutatud on (siin võib küll väita, et on olemas find and replace aga see selleks). Kontrolleritest jäi silma veel ka andmebaasi ühenduse loomine aga seda ei kasutada kordagi – tõenäoliselt on see kogemata sinna ununenud.

Turvalisuse kohapealt on ühe asjaga samuti alt mindud – nimelt kasutaja sessioon lõpetatakse ainult klientrakenduses, teenuses aga jääb token endiselt kehtima.

Kokkuvõtteks võib öelda, et töö jätab väga läbimõtlemata mulje ja tekib küsimus, et kas neil endal ei tekkinud küsimusi koodi kopeerides, et võiks teha eraldi kihi teenusega suhtlemise jaoks.