Talk:X-Ladu

From ICO wiki

X-Ladu klientrakenduse retsensioon

Meeskonnalt AFFA II

X-Ladu klientrakendus on koodi poole pealt ülesse ehitatud loetavalt. Eraldi on grupeeritud: mudelid, staatilised meetodit, vaated, vaatemudelid. Mudelite ja vaatemudelite klassides oleks soovitatav grupeerida privaatsed väljad,property-d ning meetodid. Hiljem meetodeid (juurde) lisades on pärast raske orienteeruda koodis. Kood on klientrakenduse poole pealt dokumenteerimata, sellega teeksite projekti realiseerimisel omale palju tööd juurde. X-Ladu klientrakendusel on 2 erinevat gruppi. Administraatorid ja tavakasutajad. Kuna administraatorid saavad teha ka tavakasutaja toiminguid, siis puudused (mõni üksik positiivne tähelepanek ka ja kõik kindlasti mitte pahas mõttes) toon ära administraatorite poole peal.


Tavakasutajad saavad

1)Lisada tooteid

2)Otsida ja muuta tooteid

3)Vaadata ja vajadusel muuta oma profiili ja parooli.


Administraatorid saavad

1)Lisada, kustutada tootegruppe (kirjaviga antud lehel: valtud peaks olema valitud). Kindlasti tuleks juurde lisada muutmine. Toodegrupi puhul näiteks inimlikust aspektist tulevnev kirjaviga mõne päeva pärast tähendab seda, et tootegrupp tuleks eemaldada ning seejärel teha uus kirjaveata tootegrupp. Paraku tähendab see seda, et kustutamisel kaovad ka kõik grupis olevad tooted. Sellise probleemi ette ei tahaks ükski administraator sattuda.

2)Vaadata ja vajadusel muuta oma profiili ja parooli. Parooli muutes leidsin sellise bugi, et uue parooli sisestamisel ei ole mingisugustki kontrolli. Võin sinna lisada vähem kui 8 tähemärki, aga ka jätta täiesti tühjaks. Kindlasti tasuks see suur potensiaalne turvaauk korda teha.

3) Lisada toodet. Toodete lisamisel on välja toodud andmed, mida peab täitma, seega ei juhtu nii, et suhtlus andmebaasiga toimiks probleemidega. Paraku ei ole limiteeritud näiteks toote nime pikkus. Kui lisada toote nimi, mis on kui kui 50 tähemärki, siis klientrakendus „annab otsad“. Seega edasiste komplikatsioonide vältimiseks tuleks ära limiteerida sisestatud välja pikkus. Kui toote nimi on 50 tähemärki ja teised kastid on täidetud, siis lisamisel kuvab uude aknasse ainult teksti „Toode“ koos nupuga. Otseselt ei ole aru saadav, kas nüüd lisati toode või mitte.

4)Otsida toodet. Toodete otsimise lehele minnes tulevad ette hetkel olemasolevad tooted. Tooted, mille nimetuse pikkus on pikem kui 50, kuvatakse ainult 24 tähemärki. Tootegrupi puhul näidatakse 20 tähemärki. Valides antud toote, kuvatakse toote nimena 24 tähemärki. Seega 16 tähemärki on „kaduma läinud“. Sama asi on tootegrupi kohta, kus 25 tähemärgi asemel kuvatakse 21. Lisaks saab tooteid muuta, selleks tuleb valida toode ning vajuta muuda toodet. Antud toote muutmise võimalus võiks olla toodud esile logimise järgsel lehel. Näiteks „muuda/otsi toodet“. Antud hetkel tuleb toote muutmise võimalus üllatusena. Kui toode on valitud, siis on paremal all nupp „Lisa“, mis ei tööta. Kas see peaks viima leheni „lisa toode“ ? 5) Vaadata logi. Administraatoril on võimalus detailselt näha , mida keegi teinud on. Näiteks kes vaatas toote profiili, kes lisas toote, kes mida otsis, kes logis sisse ja välja jne ning seda kõike ajaliselt. Lisaks saab vajadusel tühjendada otsingu filtrit.

6)Hallata kasutajaid. Võimalik on kasutajat kustutada, blokeerida teatud ajaks, kaotada ja lisada administraatori õiguseid ning vaadata kasutaja profiili.Lisaks on kasutajatele ära toodud, mis on nende ID, kasutajanimi, kas on administraator ning millal on lisatud. Kõik väljad on konkreetselt täies pikkuses nähtaval ning ei ole ülekattuvusi. Valides kasutaja profiili ning vajutades tagasi, satun ma „Halda kasutajaid“ lehe asemel pealehele.


Soovitused, plussid ja miinused

1)1 klass ning üks .xaml on kirjutatud täpitähega. Samuti osad meetodid. See ei ole keelatud, aga tasuks kindlasti vältida, sest võib tekitada ikaldusi.

2) Kasutajate lisamisel on piiratud sisestamise pikkus. See on väga positiivne.

3) Kui kasutaja saab registreeritud (aken tuleb ette, et on registreeritud), siis ok vajutades võiks suunata pealehele, mitte tagasi kasutaja lisamise lehele.

4)Tavakasutaja ei saa lisada tootegruppi, seega ei saa samuti lisada tooteid antud tootegruppi.

5)Miks ainult toote lisamisel,muutmisel, kasutajate haldamisel on nupp „Sulge rakendus“.

6)Klientrakenduse suurus ei ole muudetav.

7)Admini kasutaja oleks võinud lisada andmebaasi. SQL Management Studio-s on MD5 krüpteeringuga parooli on päris keeruline peast kirjutada (nüüd pidi veidi koodi kopeerima). Tagantjärgi oleks lihtsam olnud teha kasutaja ning siis andmebaasis muuta Admin true-ks.

8)Toote lisamisel võiks olla täpselt ära toodud, kus kohas tekib viga.


Kokkuvõttena on näha, et antud meeskond on palju vaeva näinud antud projekti tehes. Eeltoodud vigade (enamasti pisivigade) parandamine võrreldes antud projekti mahukusega on pea olematu. Enamasti on küll välja toodud vead ja puudused, aga need on toodud välja selleks, et projekt saaks realiseeritud võimalikult väheste vigadega.



X-Ladu veebiteenuse retsensioon

Retsenseeris meeskond AFFA II

Teenuse käivitamine juhendi järgi õnnestus. Konkreetses süsteemis, kus teenuse testimine toimus, tuli küll andmebaasi loomise skripti mõnevõrra muuta (eemaldada paar rida, mis järgnesid andmebaasi loomise reale).

Teenuse kood on üldjuhul korralikult dokumenteeritud. Teenuse meetodid on jõutud praktiliselt täiel määral ära kirjeldada. Samas on natuke üle pingutatud koodi kommenteerimisega meetodeid teostavates klassides. Kuna teenuse meetodid viitavad teistesse klassidesse ja kaasa antakse samad parameetrid, mis teenuse meetodile kaasa anti, siis poleks vaja olnud tühjade kommentaaride lisamist „MudeliteMeetodid“ kausta klassidesse. See teeb koodi lugemist ebamugavamaks, failid pikemaks ja ei anna lisandväärtust.

Andmebaasi ülesehitus tundub mõistlik. Teenuse vahekihis andmebaasi olemitele vastavad mudelid on ka kasulikud. Selle läbi saab kontrollida, milliseid andmeid andmebaasis olevate olemite kohta saata ja vajadusel nende formaati muuta.

Teenuse meetodite viimine eraldi klassidesse vastavalt sellele, kes antud toiminguid teostada saavad, aitab hoida teenuse kirjelduse konkreetse ja kompaktsena ning annab programmeerijale parema ülevaate. Kui on vaja uusi meetodeid lisada, siis peaks olema lihtne näha, et näiteks administraatorite meetodid vajavad mingeid kindlaid kontrolle ja sellega seoses peaks vähenema vigade tekkimise oht.

Teenuse turvalisuse kohapealt on küsitavusi. Kasutajatega seotud meetodis „annaKasutajad“ tehakse päring kasutajate kohta ja teenuse kasutajale antakse kõik andmed, kaasa arvatud kasutaja parool. Paroole ei ole mõistlik teenusel kliendile üle võrgu sellisel moel saata, isegi kui need on krüpteeritud (antud juhul on kliendis MD5-ga hash-itud, ning seda ei peeta enam turvaliseks viisiks). Teenuse meetodite kasutamine ei nõua teenuse poolel mingit kasutaja või kasutajarolli autentimist, mis teeb võimalikuks teenuse kasutamise autentimiseta ja mitte registreeritud kasutajatel andmete muutmise.

Natuke arusaamatu on, miks kasutajate statistika kirjutamine on üks teenuse poolt pakutavatest võimalustest. Statistika peaks tekkima automaatselt teenuse sees, ilma et klient selle kohta andmeid saadaks. Ühelt poolt tekitab see riski, et kliendis on võimalik mingi tegevuse logi tekitamist vältida ja teiselt poolt suunatakse sellega funktsionaalsus, mis peaks toimuma teenuse poole peal, osaliselt ka kliendi poolele. Suureneb keerukus, kuna kliendis peab hakkama tegelema otseselt sellega mitteseotud tegevustega. Kui tulevikus peaks soovima mingi muu kliendi teha, tuleb kõik logimisega seotud uuesti kirjutada ja kontrollida, et see kattuks ka teiste klientidega, kui on soov, et ühe tegevuse kohta ei tekiks erinevate kirjeldustega ridu tabelisse. Risk on ka see, et „lisaLogisse“ meetod sisaldab kasutaja identifikaatorit, mis jällegi võimaldab kliendil päringuid võltsida ja teistele kasutajatele logi kirjeid teha.

Teatud meetoditel võiks olla tagastusväärtus. Näiteks kõik muutmise ja kustutamisega seotud tegevused peaks kliendile tagastama, kas antud operatsioon õnnestus või mitte. Vastasel korral peab teenust kasutav klient peale muutmise või kustutamisega seotud operatsiooni kõik objektid uuesti pärima, et tulemust teada saada. See suurendaks teenuse koormust ja raiskaks võrguressurssi.

Kasutajate loomisel ei toimu teenuse poolel kontrolli, kas mingi kasutajanimega kasutaja on juba olemas, ka see kontroll peaks tegelikult teenuse poolel toimuma ning vastava sõnumi saaks kliendile saata näiteks mingi erindiga.

Mõned teenuse poolt olemite muutmiseks ja lisamiseks pakutavad teenused kasutavad parameetrina teenuse kihi olemeid(„muudaKasutajat“, „muudaToode“, „lisaToode“, „lisaKommentaar“), teised jälle konkreetseid väärtustega parameetreid („muudaTootegrupp“, „muudaKommentaar“, „lisaTootegrupp“). Mõistlik oleks, kui kõik muutmise teenused toimiks teenuse poolt pakutavate olemite saatmise kaudu. Sellisel kujul, kus muutmiseks tuleb parameetritena saata kõik väärtused eraldi, tekib teenuse kasutamisel hiljem probleeme, juhul kui teenus muutuma peaks. Siis tuleks kliendil ka oma kasutatavaid meetodeid muuta. Saates terve muudetava olemi tagasi saab seda probleemi vältida.

Koodi kirjutamine eesti keeles on maitse asi, kuid see võib probleeme tekitada (ei saa täpitähti kasutada, mis võib teatud juhtudel segadust tekitada) ning piirab teenuse kasutamist eesti keelt mitteoskaval kasutajaskonnal.

Kokkuvõttes on teenuse ülesehituses nii häid kui ka halbu külgi. Koodi struktureerimine ja kommenteerimine on hästi teostatud. Täiendada saab veel teenuse turvalisust, ühtset stiili hoidmist (lisamise ja muutmise meetodite puhul), teatud toimingud vajaksid tagastusväärtusi, et klient saaks tulemuse kohta tagasisidet, logi kirjutamine ei peaks olema kliendi poolt saadetava infoga, vaid see peaks toimuma teenuse sees automaatselt.

X-Ladu veebiteenuse retsensioon

Retsenseeris meeskond SaanEndagaHästiLäbi

Projekt on kenasti jaotatud ära kolmeks mooduliks, klassid hästi ära jaotatud kaustadesse, midagi pole ripakil ja kõik on loogiliselt ja intuitiivselt kätte saadav. Teenusel on korralikult interface ja implmentatsioon nagu olema peab. Implementatsioon kenasti kommenteeritud, piisaval kujul täpselt. Ühes kohas panin tähele, et returns tag on tühi, kuigi returnis listi, aga see on väga minor näpukas ja kindlasti pole viga ega väga mainimist väärt(aga kuidagi pean ju mina ka oma sõnade arvu täis saama :) ). Pärast poole tundub, et on laisemaks jäädud, näiteks TavakasutajaMeetodid klassis on vist automaatselt kommentaarid doc genereeritud, aga summaryt ja parameetrite taha pole eriti midagi kirjutatud. Mina kommenteeriks pigem interface'i kui implementatsiooni. Aga see pole viga, see on lihtsalt minu arvamus ja minu meetod. Natuke lugemist võib-olla sellel teemal siin : http://stackoverflow.com/questions/759703/comment-the-interface-implementation-or-both. Meetodid mida kutsutakse välja kus pole vaja objekti teha on staatilised, see on hea. See on koodi lugemise jaoks hea ja ka performance wise on parem kui objekti loomine ja sealt siis meetodi välja kutsumine (mälu halduse poolelt). Muidu väga ilus kood, aga jäi natuke arusaamatuks miks eriti objektidel konstruktoreid ei kasutatud. Näiteks oleks võinud muudaTootegrupp meetodis uue Tootegrupp objekti teha konstruktoriga, mitte 4 rida settereid kirjutada. See jällegi pole viga, samas oleks olnud ilus ja mõnusamini loetav kood. Meeldib ka regioonide kasutus igal pool ühtlaselt. See teeb koodi palju loetavamaks. Hästi on hoitud ühtset stiili regioonide puhul (privaatväljad, propertyd, konstruktor ja meetodid). Andmebaas teenuse juures on suht väike, aga täiesti piisav. Toimetab ära kõik mis vaja. Kasutatud linq-to-sql-classes meetodit, mis on väga mugav. Väga loetavalt on kirjutatud linq päringud data access kihis. Ilusti on ka andmebaasi SQL lisatud juurde, mis on positiivne. Referencide all nägin ninjecti, kuigi koodis kuskil kasutus ei näinud. Ei väida, et polnud, lihtsalt ma võib-olla ei osanud seda välja lugeda. Kokkuvõtteks oli hästi tehtud teenus, kood oli väga ilus ja loetav, stiili oli hästi hoitud.