Da sam ja dobio da ti radim review, na osnovu ovoga vec sto si napisao (ne ulazim u to kakav je tacan zadatak), ja bi ti dao par primedbi:
- Sredi identanciju, generalno meni ovo nije neki problem kada vidim i obicno ne zameram, ali neki minimum da bude uredno je potrebno.
- Svaka klasa u poseban fajl (naravno, postoje izuzeci, ali s obzirom da je ovo test, ipak bi i to gledao)
- ne koristi
nikad stringove za control flow, ovo je prvo i najociglednije pravilo da neko ne razume type system, pa samim tim svejedno mu je da li radi C# ili js
- Ne razumem svrhu za lock (BTW, ako budes radio ikad neke komplikovanije multithreading stvari, lock izbegavaj po svaku cenu, immutable i re-entrant metode su tvoj spas), lock koristis u re-entrant metodi, sta tacno lock treba da brani? Nema ni svrhu mnogo lock stavljati nad FS, cak i da je zadatak da pravis ACID DB, to bi onda moralo da fercera na nivou OS-a, ne samo na nivou processa, pa samim tim bi morao i sa mutexima da se igras, no cela poenta je da je lock sasvim bespotreban
- Za public API, nikad ne koristi List<T>, cak i IEnumerable<T> je nekad nesiguran, opet, za multithreading jedino array ([]) sa immutable tipovima jedino stvarno radni posao
- Ne hvataj toliko visoko exception, nema potrebe, od exceptiona se brani samo tamo gde mislis da ima smisla, nikad neces naciniti program stablinijim ako dodajes sto vislje i svuda exception (type system is your friend)
Ovo su neke stvari koji mi padaju prvo na oko, neke druge stvari poput organizacije same klase bi moglo vise da se prica, i zahteva vise informacija sta je sam smisao zadatka, ali cak i sa ovim moze bolje. Drugo, ono sto meni generalno smeta je kada neko u strongly type sistemima trazi neke unit testove, na tvom primeru, a i na primeru drugih, osim hype, ne vidim nikakav razlog za ovu nebulozu (osim na integration testotove, jedini realni vredni testovi). Recimo tvoj primer:
Code:
[Test]
public void dGetUsersWhereParameterIsSortinParammeter()
{
var phonebook = new PhoneBook();
Assert.IsInstanceOf(typeof(List<User>), phonebook.GetUsers(""));
}
Nema smisla, type system ti garantuje da je List<User> uvek vracen (naravno, govorimo o tipu, ne i o instanci, na zalost C# nema non-null referentne tipove).
Nadam se da ovo neces shvatiti kao kritiku, nego na savet sta da ne/radis za ubuduce, pa samim tim i sto bolje naucis C# :)
