|
|
| (4 intermediate revisions by 3 users not shown) |
| Line 1: |
Line 1: |
| ==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.<br>
| |
| <br>
| |
| Meeskond on oma teemaks valinud raamatukogu ning selles leiduvad teavikud. <br>
| |
| <br>
| |
| 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. <br>
| |
| <br>
| |
| XML skeemifailis olevad väärtustüübid on õigesti valitud, raiskamata liialt mäluruumi.<br>
| |
| <br>
| |
| 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.<br>
| |
| <br>
| |
| Failid on selgesti loetavad ning korrektselt vormistatud, XML failis jäetud vahetühikud muudavad koodi lugemise hõlbsamaks. <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.
| |
| | |
| ==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.
| |
| | |
| ==Klientrakenduse ja teenuse retsensioon meeskonnalt nimega "MeilEiOleGrupinime"==
| |
| | |
| ===Klientrakendus===
| |
| | |
| Klientrakenduse puhul vaatab meile vastu suhteliselt hea kujundusega veebileht. Tegemist on juskui „Mini ARK“-iga. Me saame vaadata mis autod kellel on, mis autod üldse on, mis kategooriasse see kuulub, kasutajad ja samuti nende juhtimisõigust Selline asi oleks hea mõnele suurfirmale näiteks, enda sõidukite haldust (mis töötaja käes on mis sõiduk).
| |
| | |
| Esimene asi on registreerimine. Kui kasutajat ei ole tuleb ASP.NET-i ilus kollane viga. Sama tuleb ka muide siis kui vaadata mittesisseloginult statistikat. Kui regada siis vahepeal viskab sisselogimise lehele tagasi ja vahel mitte. Tundus, et siis kui regamisel mõni väli oli valesti täidetud (kasutaja juba olemas, paroolid ei klapi, ebasobiv parool). Samuti tuleb ASP-i kollane kui panna vale parool või mitte registeeritud kasutajaga sisse logida.
| |
| Kui me oleme sisselogitud siis meile avaneb ilus leht. See on statistika. Meil on näha, et mitu kasutajat on registeeritud, mitu sõidukit on baasis, mitu automudelit ja mitu autotootjat. Selle all külastatavuse on graafik. Kõige all jääb silma ilus diagramm, mis näitab protsentuaalselt autode osakaalu. „Activity“ ja „Transactions“ tundub olema, kui „asi mida ei jõudnud valmis“, sest kui selle vajutada siis viiakse meid lehe ülesse. Menüüdes selliseid asju ka ei leia.
| |
| | |
| Lisada saab kõike: kasutajaid, marke, mudeleid, kategooriaid ja isikute ja autode vahelisi suhteid. Samuti saab kõiki neid kustutada. On küsitav, kas iga kasutaja ikka peaks saama lisada ja kustutada. Mõningal puhul kui püüame lisada uut siis ei puudub kontroll, selle kohta kas nõutud väljad on täidetud vaid meid visatakse vealehele (404 – Not found). Sõiduki lisamisel tehakse sõiduk, mille tootmisaastaks on 0 ja numbrimärk puudub. Kui mõnda numbrilahtrisse sisestada tähti siis kustutatakse sisestatu sõnagi lausumata. Kui vaatame, mis isik on mille omanik siis näeme, et seal on posu id-si, mis esmapilgul on väha ehmatav. Kui aga lisada uus siis saab aru, et neil on ka nimed ja sõidukid taga. Juhiload võiksid olla nähtavad ka isikute alt. Paistab, et sõidukite kasutajad on jäätud ka „järgmisesse versioon“.
| |
| | |
| Koodi poolelt näeme, et kasutatud on vaatemudeleid. Teenusega suhtluse puhul oleks võinud meetodid viia ühte klassi kust neid siis välja kutsuda. Praegune lahendus on aadressi muutumise korral väga valulik (isegi oskusliku ctrl+h kasutuse korral). „Not found“ lehe asemel oleks võinud kuvada midagi muud, näiteks enda loodud vealehte või püüda vead kinni teisiti.
| |
| | |
| ===Teenuse retsensioon===
| |
| | |
| Kuna klientrakendus on tihedalt põimitud klientrakendusega siis on isegi raske öelda, kust hakkab teenus ja kust lõppeb klientrakendus. On vaieldav, kas oleks pidanud teenuse ja klientrakenduse eraldi viima või mitte.
| |
| | |
| Näeme, et kasutatud on UOW-id ja valmis on tehtud ka repod (mis antud juhul on küll ''service'' nime all), mis on küll tühjad, aga olemas on ja edasiarenduse puhul saab neid kasutada. Kasutada mõnel pool on näha ka „tehase“ ja DTL-i kasutust kasutust, mistõttu on teenusekontrollerid enamasti puhtad ja neis on hea muudatusi teha. Võimalik, et seda oleks võinud kasutada veelgi rohkem, sest mõnel pool on näha loogika kirjutamist kontroller. Meenutuseks siis repod olid tühjad, sinna oleks võinud ka mõned read kirjutada mis nüüd kontolleris lebavad. Kasutatud on ka DTO-si, vähendada saadetavate andmete kogust. Enamasti nõuavad kõik teenuse kontrollerid ka autoriseerimist. Huvitav on näha grupitöö mõju: mõni meetod tagastab objekti või listi sellest, mõnel pool on näha IHttpActionResulti kasutamist, mõnel pool on veahaldust. Täitsa usun, et aega jäi väheks ja lõpp tuli kiirustades ning üle ei jõudnud vaadata.
| |
| | |
| Huvitav oli näha andmebaasi konfiguratsiooni. Ei oska päris täpselt öelda, kuidas see tehtu erineb ''DateAnnotations''-itest.
| |
| | |
| Kokkuvõttes võib öelda, et teenust oli tegelikult oluliselt rohkem kui klientrakendust. Klientrakendus on küll hea välimusega, aga funktsionaalsuses jättis beta mulje.
| |
| | |
| | |
| Meeskond „MeilEiOleGrupinime“
| |
