Talk:MOT

Veebiteenuse retsensensioon by meeskond BeerPressure:

Üldiselt:

• Puudub README vms. Dokumentatsioon projekti ülespanemiseks ja seadistamiseks.
• Rollide loomine vigane, seda võiks teha eraldi failis, mis on DAL.App.EF projektis (kasutasime algul sama lähenemist nagu antud projekt). Takistab esialgset andmebaasi uuendust, kui rollid veel puuduvad. Rakenduse installimine tarnitud kujul ei õnnestunud. Selleks et Nuget Package Manageri käsk Update-Database töötaks pidi ajutiselt välja kommenteerima Startup.cs faili rea 153 (// CreateRoles(services).Wait();).

Andmebaasist:

• Üldiselt paneksin igale väljale juurde AddTime ja UpdateTime see on hea tava millest korduvalt ka andmebaasi alustes räägiti ja rõhutati. Kuid antud projekti juures on see pigem nice-to-have.

Olemitest:

• Kui juba domainis nimetate kõik olemid singulaarselt siis võiks ju sama teha ka andmebaasis - lisakeerukuse loomine kohas kus see ei ole vajalik.
• Entity-te arv oli nõutust väiksem - Kokku 8 olemit, millest 1 on many-to-many liittabel ja 1 otsene koopia Identity framework Users tabelist.
• Olem People mis on põhimõtteliselt Usersite laiend ja sisaldab ka viidet Identity tabelile ApplicationUsers (ApplicationUserId). Juhul kui see on mõeldud rakendusega mitte registreerunud kasutajate võistlusele kirja panemiseks, siis oleks olemil rohkem infot vaja - vähemalt mingigi kontakti väli (email |/& tel.nr. |/& aadress).
• Competition.Comment on varchar(100) - võiks ja peaks olema varchar(1000)
• Competition.CompetitionPlaceId - kui võistluse toimumiskoht mingil põhjusel peaks keelduma korraldusest või kui võistlust alles pannakse kokku ja ei ole veel teada kus see toimuma saab siis peaks saama sinna sisestada NULL väärtust.
• Competition.Time -> Mis aega see näitab? Property nimest seda välja ei loe. Kui StartTime, siis kus on EndTime või Duration?
• CompetitionPlace.County -> int type enum default väärtus on 0, kuna see pole aga enumi klassis implementeeritud põhjustab see tn. kunagi kuskil vea. (Millisele maakonnale vastab 0?) See on teada tuntud probleem millele google annab hõlpsasti erinevaid lahendusi: näiteks siin ja siin.
• Väljad mis viitavad Identity framework olemitele läbi nende ID-de ei peaks olema nvarchar(max) vaid nvarchar(450) (kuigi isegi see on reaalsuses liiga pikk).
• PersonInTeam.PersonInTeamId -> ei peaks olema sellist välja üldse, selle tabeli PK peaks olema kombinatsioon nende tabeli ID-st mida ta ühendab. Praegu ei ole unique indeksit üle nende, mis tähendab et sama inimene saab mitu korda sama tiimiga liituda.
• Participations - Miks selline nimi, kui registreerimine juba oli? Saan aru et selles olemis tahetakse säilitada võistluse tulemusi. Siis võiks ju juba olla CompetitionResults. Kus on aga saavutatud koha numbri talletamine? Kui võistluse punktid ja ajad on kõik kirja saadud siis tahaks ju tulemusi ka kuidagi näha ja võrrelda ning selle võrdluse tulemi kuhugi salvestada.
• Participations.Time - Mis aega näitab? Eeldan, et võistluse läbimise aega? Kui nii, siis miks ei ole NULL lubatud - mõnedel võistlustel aega ju ei mõõdeta?
• Participation.Points - Kas igal võistlusel antakse ainult täisarv punkte (int)? Võiks igaks juhuks siiski implementeerida double väärtuse tüübi. Lisaks ei ole võimalik osalust luua ilma punkte teadmata - peaks siis määrama default väärtus. Mõnede võistluste puhul jälle punkte ei anta vaid mõõdetakse aega - võibolla peaks ka NULL-i lubama.
• Participation.Disqualified - Nullid ei ole lubatud -> järelikult oleks default väärtust vaja.
• Registrations - Projekti Must have alla on märgitud “Osaleja saab registreerida võistlusele meeskonda” kuid olemis puudub TeamsId ning samuti ei ole seda võimalust kuidagi läbi Servicei implementeeritud.
• Registrations - Millal registreering tehti on suhteliselt tähtis teada. Paljude võistluste puhul sõltub sellest näiteks registreerimistasu.

DTO-d:

• Hetkel tundub nagu rakendus otseselt seda abstraktsiooni kihti ära ei kasuta. Kõik DTO-d konverteeritakse ümber 1:1-le Domain objektideks ja vastupidi.
• Üldiselt oleks vaja teha tööd veel DTOdega, et need rohkem peegeldaksid oma kasutust. Näiteks:

küsides kõiki võistlusi, kas meil on tarvis avalikustada kliendile, mis on kasutaja ID, kes iga võistluse lõi? DTOsid võiks olla rohkem erinevate kasutusjuhtumite jaoks. CompetitionDTO võiks sisaldada ka nimekirja võistlejatest, kus koondatakse info ManyToMany seoste põhjal kasutades vahetabelit, nagu näiteks:

List<PersonDTO> people = c.Participation
.FindAll(p => p.CompetitionId == c.CompetitionId)
.Select(PersonDTO.CreateFromParticipation)
.ToList();

API Kontrolleritest:

• Puudub Swagger startup konfiguratsioon, Swagger annotatsioonid kaotavad mõtte ning pole võimalik kontrollida, kas need on korrektselt loodud.
• ProducesResponse annotatsiooni pole kusagile lisatud, staatuskoodid on natuke poolikult läbi mõeldud, vähemalt need, mida Swaggeri raames mainitud on
• Annotatsioon Authorize, kus on ära määratud AuthorizationScheme peaks minema kogu klassi peale, meetodis saab ära määrata vaid selle, mis rollid on lubatud.
• Kontrollerid enamuses pöörduvad vaid spetsiifiliste domeenide poole ja viivad ellu väga tavalist CRUD funktsionaalsust, mille tõttu tundub, et need on natuke veel toored. Näiteks on välja toodud “must have” nimekirjas projekti analüüsis, et meeskonna liige peaks saama muuta meeskonna andmeid. Antud kontrollerites vaid autentitakse, et kasutaja oleks rollis “participant”, kuid mitte seda, kas ta ka muudetava meeskonna liige on. Samuti pole võimalik kasutajatel näha, palju on mingile võistlusele osalejaid registreerunud,
• CompetitionsController ei tegele sellega, et tagastada võistlusele registreerunud kasutajaid, selle asemel on olemas ParticipationsController, mis tagastab vahetabeli infot selle kohta, mis võistlusega on seotud mis inimene. See võib olla tohutult suur info, mida hakata filtreerima kliendi poolelt. Rohkem tuleks teha andmete agregeerimist vastavalt teenuse kontekstile.

• AccountController
  • Response code uue kasutaja loomisel võiks olla 201
  • Puuduvad koodid nagu 429, 500
  • Registreerumisel võiks kontrollida, kas kasutaja on juba olemas, mitte vaid rolli olemasolu
  • Kas kasutajal ei või antud kontekstis olla mitu rolli? Sisselogimisel võetakse rollide seast vaid esimene ja lisatakse juurde Claimile. Võimalus kirjutada ka meetod, mis lisab kasutajale kõik rollid.

• CompetitionPlacesController/CompetitionsController/CompetitionTypesController/ParticipationsController/RegistrationsController/TeamsController
  • Antud kontrollerid on kõik väga sarnase loogikaga
  • Get() tagastab vaid koodi 200, tagastuskoodi võimalusi on veel
  • Post() meetod küsib sisse DTOd, mille üheks parameetriks on id, id genereeritakse automaatselt, seda ei peaks panema kaasa postitatavasse mudelisse, mis alles loob objekti
  • Put() puhul sama kriitika, siin on id küll vajalik, aga see juba pannakse kaasa parameetrina eraldi
  • See tundub natuke veidrana üritada kontrollida uuendatud mudeli põhjal, kas tegemist oli BadRequestiga. Eelkõige peaks id puhul üle otsima, kas antud objekt on üldse olemas ja seejärel üritada uuendada, muidu tagastada BadRequest. Kui see ei õnnestu, siis peaks juba errorkood 500 peegeldama seda, et midagi läks teenuse poole peal valesti.
  • Sama kommentaar deletemise kohta, enne tuleks kontrollida, kas objekt on olemas. Siis alles objekt kustutada.
• PeopleController
  • Kas tõepoolest igaüks saab pärida infot inimeste kohta ja neid postitada?
• PersonInTeamsController
  • API disaini poole pealt on natuke veider teha vahetabelite kohta kontroller. Tundub loogilisem panna antud kontrolleri meetodid teistesse kontroleritesse, mis on seotud siis kas meeskondade või inimestega. Näiteks küsida PeopleControlleris kõik inimesed, kes on meeskondades või küsida neid meeskonna järgi. TeamsControlleris seevastu anda võimalus lisada uusi inimesi meeskonda, neid muuta või eemaldada.
  • Id järgi ei hakka ükski klient küsima vahetabeli kohta infot, see tundub otstarbetu tegevus, pigem küsida kas meeskonna või inimese id järgi sissekandeid

Projekti kihid/struktuur:

• Olemas Business Logic, Data Access Layer, DTO’d, Service’id, Unit of Work, domeeniklassid ning API spetsiifilised kontrollerid, mis asuvad oma kaustas põhiprojekti all.
• DAL omab erinevaid kihte interface’ide ja neid implementeerivate klasside jaoks.
• Repositooriumitega suhtlemine on abstraktne, kõik käib läbi Data Access Layer’i, mille pööle pöörduvad teenuseklassid ning DAL erinevad kihid hoolitsevad selle eest, et loodaks õiget tüüpi repositooriumeid, andmeid agregeeritakse, uuendatakse ja salvestatakse organiseeritud kujul ning neid tagastatakse teenustele soovitud kujul (DTOd).