Talk:Kirves: Difference between revisions

From ICO wiki
Jump to navigationJump to search
Utiitson (talk | contribs)
No edit summary
Aluuri (talk | contribs)
Replaced content with "-"
 
(3 intermediate revisions by 2 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“
 
==Klientrakenduse ja teenuse retsensioon meeskonnalt nimega "TÜC"==
 
===Klientrakenduse retsensioon===
 
See retsensioon on tehtud meeskond Kirves veebiteenusele, mis oli teemal autode registrikeskus.
 
Tegemist on väga korralikult üles ehitatud veebiteenusega. Aines Võrgurakendused II oli vaja teha veebiteenus kas Windows Communication Foundation või ASP.NET MVC 4 Web API tehnoloogiat kasutades. Tiim kirves oli valinud enda teenuse jaoks Web API. Lisaks oli vaja kasutada erinevaid kihte näiteks: repositooriumid,  DTO kiht ja Transport Logic kiht.
 
Nõutud kihid olid teenuses kõik kasutatud. Lisaks nõutud kihtidele oli kasutatud ka Unit of worki ja ninjectit. Tiim oli ühendanud oma ASP.NET aine ja Võrgurakendused II projekti.
 
Oli täidetud andmebaasi tabelite mahu nõue (pidi olema vähemalt 6 olemit, neil oli olemeid tehtud isegi rohkem). Lisaks ise tehtud mudelitele oli veel kasutatud ka AspNetUser, AspNetRole, AspNetUserClaim ja AspNetUserLogin mudeleid, mis olid seotud tiimi enda tehtud mudelitega.
 
Kirjutatud veebiteenus tagastab klientrakendustele lisaks andmetele ka infot päringu õnnestumise kohta. Kontrolleri meetodid kasutavad ära  eraldi loogika kihti, kus on tehtud keerulisemad päringud. Andmete kliendile saatmisel on kasutatud ka DTOsid, et ei peaks saatama päringuga kaasa ebavajalikke andmeid.
Lisaks tavalistele CRUD meetoditele on kontrolleritesse lisatud ka keerulisemaid päringuid.
 
Üldiselt on koodi lugeda on kerge, sest ülesehitus on suhteliselt hea ning kood on ilusti trepitud ja osadesse jaotatud.
 
Küll leiduvad mõned asjad, mida võiks puuduste all välja tuua. Näiteks on tiim väga vähe keskendunud dokumentatsiooni kirjutamisele. Dokumentatsioon on üks väga oluline osa teenuse kirjutamisel kuna see on asi, mille järgi kliendi kirjutaja oskab teha järeldusi milliseid andmeid ja mis päringute peale teenus neid andmeid tagastab.
Kõik ülejäänu tundub olevat hästi ülesehitatud, hästi töötav ja korralik töö.
 
 
===Teenuse retsensioon===
 
Meeskond “Kirves” on teinud oma klientrakenduse enda sama aine raames kirjutatud rakendusele.
 
Esimesel pilgul jääb kohe silma, et meeskond on näinud vaeva oma rakenduse kujundamisel. Tore on näha, et mõni tiim on ka välimuse kallal vaeva näinud, eriti kuna meie tiim sellega väga tegeleda ei jõudnud. Meeskond on kasutanud oma rakenduse välimuse jaoks väga korralikku template ja on sinna lisanud silmapaistvaid graafikuid. On küll kahju, et neist ainult üks töötab õigete andmetega ja teised on hetkel staatilised.
 
Antud klientrakenduses saab hetkel registreerida endale kasutaja. Loomulikult saab selle uue kasutajaga sisse logida ja välja logida. Kasutajat registreerides aga ei ole mingit märki sellele, kas kasutaja registreerimine õnnestus või ei, kuna kasutaja jäetakse täpselt samale registreerimis lehele.
Lisaks on reliseeritud mõningate andmete teenusesse lisamise, vaatamise, muutmise ja kustutamise funktsionaalsus. Näiteks sai teenusesse lisada autosid, automarke, inimesi ja nii edasi.
 
Kontrollereid vaadates jäi silma aga päris palju korduvat koodi, mille oleks võinud kindlasti kuskil kokku võtta. Lisaks on näha, et kontrolleritesse on käsitsi kirjutatud sisse teenuse URL. Kui teenuse URL peaks muutuma on tiimil tükk tegu, et igas kontrolleris see käsitsi ära muuta.
Kuna peaaegu kõigile aine tegijatele oli raskuseks kasutajate sisselogimise ja regristreerimise realiseerimine (kaasa arvatud meie enda tiimil), siis kuna see osa on hästi ära tehtud, on lisa punktid meie meeskonna poolt.
 
Lisaks meeldib meile väga, et kuupäeva vormides on ära kasutatud datepickerit, mis lihtsustab ja teeb kuupäeva lisamise tunduvalt mugavamaks. Lisaks ei pea kuupäeva lisamisel kasutaja vaevama oma pead, mis formaadis talt kuupäeva oodatakse.
 
Kindlasti üks vigadest mille meie tiim tahaks välja tuua on see, et kui kasutaja ei ole sisse logitud, siis näidatakse kasutajaole ka linke, mida ta ei tohiks näha või ei saa kasutada. Meie leiame, et sellised lingid peaksid olema välja logitud kasutaja eest olema ära peidetud.

Latest revision as of 13:46, 27 June 2014

-