@AMD guy
Ne moras mi davati tu verziju ako ste problem resili.. Slucajno sam krenula da citam vasu temu, ucinilo mi se da ste otisli u pogresnom pravcu, pa sam htela da pomognem :)
@Cal Lightman
Po stilu pisanja koda se vidi da nisi dugo u ovoj prici. Evo ja cu napisati par stvari koje su mi upale u oko, cisto da znas za ubuduce :)
1. Ovo je AMD guy vec rekao, a evo i ja cu opet, jer malo bode oci. Pogledaj ovo parce koda:
Code:
Person person = new Person();
try { person = FindPerson(listView1.SelectedItems[0].Text); }
Ovim "Person person = new Person();" si napravio novi objekat i smestio ga u promenljivu person, a odmah nakon toga si u promenljvu person stavio rezultat izvrsavanja metoda FindPerson. Sto znaci da je jedan objekat uzaludno napravljen i odmah po nastanku je dat garbage collectoru na ciscenje. Posto s druge strane vidim da promenljivu person ne koristis van tela "try" naredbe, ja bih ovo parce koda napisala ovako:
Code:
try {Person person = FindPerson(listView1.SelectedItems[0].Text); }
2. Nikako mi se ne svidja usmeravanje toka izvrsavanja programa uz pomoc try catch naredbe.
Code:
try { person = FindPerson(listView1.SelectedItems[0].Text); }
catch { return; }
try/catch treba da se koristi iskljucivo za handle-ovanje situacija koje su stvarno izuzeci, a ne i za kontrolu toka izvrsavanja programa. To je uvek losa praksa. Ja bih FindPerson napravila tako da vraca null u slucaju da osoba nije pronadjena, tako da bi kod posle moje izmene izgledao ovako:
Code:
Person person = FindPerson(listView1.SelectedItems[0].Text);
if (person == null) { return; }
3. Uslov iz donjeg ifa nikad nece biti false. Nema potrebe za njim :)
Code:
if (listView1.SelectedItems.Count > 0)
{
try
{
if (listView1.SelectedItems.Count == 0) return;
. . .
4. Ovaj deo koda mozda ima pricu koju ne shvatam, ali trenutno mi deluje bespotrebno raditi to u svakoj iteraciji petlje:
Code:
listView1.Items[listView1.Items.Count - 1].Selected = true;
... dovoljno bi bilo uraditi jednom na kraju
5. Ovde je suvisno proveravati x.Name != null (pored toga postoji ista greska kao i u tacki 1).
Code:
Person person = new Person();
person = people[people.FindIndex(0, x => x.Name == listView1.SelectedItems[0].Text && x.Name != null)];
Takodje, malo je nebezbedno tek tako proslediti vrednost koju si dobio pozivanjem "FindIndex" metoda. Moze se desiti da FindIndex vrati -1, zbog toga sto iz nekog razloga nije nasao coveka sa tim imenom. Postoji elegantnije resenje, koriscenjem "Find"-a (kod sam na slepo kucala, i nisam proverila, ali trebalo bi da je tako):
PS za stringove se savetuje koriscenje Equals metode umesto ==
Code:
Person person = people.Find(x => x.Name.Equals(listView1.SelectedItems[0].Text));
6. Cini mi se da podatke u svoj xml cuvas samo pri zatvaranju forme.. Na taj nacin ne bi mogla da funkcionise nijedna ozbiljnija aplikacija. Praksa je ili da postoji neki "Save" button (gde prepustas odgovornost korisniku) ili da automatski pri svakoj izmeni cuvas podatke. Na taj nacin se osiguravas da nepredvidjenim prekidom rada aplikacije nece sav dotadasnji rad u njoj pasti u vodu..
Eto to su mojih par saveta :) Samo nemoj da pomislis da je to sto si uradio lose, daleko od toga.. Ovo su samo stvari koje sam ja naucila programirajuci i citajuci kojekakve tekstove.. Ja se nadam da ce ti ovo biti od koristi u daljem radu :)
"Time is a drug. Too much of it kills you." Terry Pratchett