[QGHG-it-dev-list] Zadatak 8 - ucitavanje kompleksa iz fajla
Marko Vojinovic
vmarko at ipb.ac.rs
Sun Nov 20 10:57:24 CET 2022
Pozdrav Dusane,
Evo pogledao sam I/O kod --- nisam ga jos testirao, ali ovako iz citanja i pregledanja mogu da ti dam par komentara i saveta.
Prvo, kod je super! :-) To sto je dugacak nema veze, i to sto treba pocistiti detalje oko error-handling i memory leaks nije veliki problem. Odlicno si uradio posao, svaka cast! ;-)
Drugo, buduci da je kod relativno dugacak, bilo bi pametno da ubacis komentare --- osnovnu ideju, bazicnu strukturu .xml fajla koji se snima/ucitava, kao i cemu koji deo koda sluzi i sta radi, u glavnim crtama. To je potrebno za slucaj da se kasnije vratimo na taj kod (bilo ti, bilo neko drugi), da bi bilo jasno sta se dogadja i kojim redom. Komentare bi bilo dobro da ubacis dok ti je cela stvar sveza u glavi, jer posle nece ni tebi biti jasno sta si tu sve radio... ;-) Inace, i ja planiram ovih dana da ubacim gooomilu komentara u ceo dosad napisani kod projekta, da sve bude bolje i jasnije objasnjeno i dokumentovano.
Trece, ima par sitnih detalja koje mozes malo da pocistis u kodu, u f-ji read_complex_from_xml_file( rapidxml::xml_document<>& doc ):
(1) Negde na sredini f-je stoji "bool first = true;" pri cemu se varijabla first nigde kasnije ne koristi. Pretpostavljam da je to visak?
(2) Pri kraju f-je u petlji pozivas dve f-je,
read_level_node(current_node, sc, stoi(current_node->first_attribute()->value()));
read_ksimplex_node(current_node, sc);
koje imaju redom tri odnosno dva argumenta, a nize si te iste dve f-je definisao kao
void read_level_node(rapidxml::xml_node<>* node, SimpComp* sc, int level, string delimiter)
void read_ksimplex_node(rapidxml::xml_node<>* node, SimpComp* sc, string delimiter)
tako da imaju cetiri odnosno tri argumenta. Taj dodatni nepostojeci argument (string delimiter) se onda eksplicitno prosledjuje f-ji parse_level(), koja ga koristi (na netrivijalan nacin). Nije mi jasno kako ovo uopste hoce da se kompajlira (broj argumenata u f-jama se ne slaze), i nije mi jasno gde je "delimiter" zapravo definisan? Ako je on neka globalna varijabla unutar namespace rapidxml ili nesto tako, zasto ga onda prosledjujes eksplicitno kao argument u parse_level(), a to isto ne radis u read_level_node() i read_ksimplex_node()? Kako to sve zapravo radi?
(3) Na samom kraju f-je, u poslednjoj petlji, brises privremene UniqueID boje. Nisam siguran da li ti treba provera "if (!ks->colors.empty())", jer bi svaki simpleks trebalo da ima taj privremeni ID. Naime, kad si snimao fajl, f-jom save_complex_to_xml_file(), odmah na pocetku si uradio
UniqueIDColor::colorize_entire_complex(simpComp);
sto je obojilo bukvalno sve simplekse. Kada posle radis read_complex_from_xml_file(), ti zapravo citas bas te brojeve (privremene id-jeve), i struktura snimljenog fajla bi trebalo da ti garantuje da svaki simpleks u novokreiranom kompleksu sigurno ima makar tu boju, ako nista drugo. Pa zato ne vidim da li postoji situacija kada uslov "if (!ks->colors.empty())" nije zadovoljen?
Cetvrta tema --- pitanje sta uciniti sa f-jom colorize_node(). Moj predlog je sledeci. Prvo, po nekoj ideji dizajna, f-je za I/O treba da budu agnostic o tipovima i vrednostima boja, i slicnim detaljima --- krajnji korisnik ce mozda zeleti da definise svoje custom boje, i da ih snima i cita iz fajlova, ali mi sigurno ne zelimo da krajnji korisnik ceprka po kodu I/O f-ja da bi mu to radilo. Za snimanje u fajl imamo lepo resenje --- svaka boja u klasi Color ima definisanu f-ju get_color_value_as_str(), i ti to koristis kada snimas boju u fajl, tako da te uopste ne zanima kako je definisana vrednost boje (to je prosto neki string koji ces da snimis).
Istu filozofiju treba primeniti i u slucaju citanja iz fajla, na sledeci nacin --- prepravi f-ju colorize_node() tako da bude prosto wrapper f-ja koja ce da poziva novu f-ju,
void Color::colorize_node_from_string( KSimplex* ks, int color_type, string color_value );
i samo toj f-ji prosledi pointer ks i parametre
int color_type = stoi(color_node->first_attribute()->value());
string color_value = color_node->value();
pa ce posao te f-je da bude da oboji simpleks kako treba, a ti samo zavrsis colorize_node() f-ju. Pritom, celu switch/case strukturu preseli u tu novu f-ju --- ona ce da zivi u klasi Color (prebaci je u color.cpp i color.hpp fajlove), i nju ce krajnji korisnik moci da azurira kada/ako bude dodavao svoje custom boje.
Ovakvim wrapper-zahvatom postizemo dve stvari: (a) dodavanje novih boja vise nema direktne veze sa I/O f-jama, nego se vrsi samo modifikovanjem f-ja u klasi Color, i (b) kasnije cemo mozda switch/case strukturu da zamenimo necim inteligentnijim, mozda za svaku boju da zahtevamo da ima definisanu f-ju set_color_value_from_str(), poput ove koju si uveo za ScreenCoordinateColor, itdisl. Generalno, to sve dalje vise nije tvoj problem (za I/O f-je), a klasu Color cemo verovatno jos dosta da (re)dizajniramo i doterujemo kasnije (bilo ti, bilo neko drugi, bilo krajnji korisnik, o tom potom)...
Mislim da bi ovakav dizajn bio generalno mudriji, pa vidi da tako reorganizujes taj kod.
I konacno, peta tema --- cini mi se da sam pronasao i jedan mali bag. Naime, u switch/case strukturi unutar colorize_node() imas izmedju ostalog i sledeci kod:
case TYPE_UNIQUE_ID:
color = new UniqueIDColor(stoi(color_node->value()));
ks->colors.push_back(static_cast<UniqueIDColor*>(color));
break;
koji se izvrsava ako je snimljeni kompleks imao neke simplekse obojene UniqueID bojama. Kada u nekoj kasnijoj sesiji (neki drugi main(), tj. drugi program) ucitavas taj fajl i konstruises novi kompleks, moze da se desi da su te snimljene UniqueID boje (iz fajla) vec iskoriscene za neki drugi kompleks (seed-ovan ranije u ovoj novoj sesiji), sto vodi do dupliranja UniqueID boja, sto ne sme da nam se dogodi. :-)
Ovo dakle mora da se popravi, tako sto se dati simpleks ne oboji UniqueID bojom koja je procitana iz fajla, nego mu se dodeli prvi slobodan UniqueID iz tekuce sesije, ovako:
case TYPE_UNIQUE_ID:
ks->colors.push_back(new UniqueIDColor());
break;
Jasno, tada vrednosti id-jeva za kompleks vise nece da se poklapaju sa id-jevima koji pisu u fajlu. Ali to sto se ne poklapaju niti moze da se garantuje, niti je vazno --- vazno je samo da su jedinstveni, tj. da nema preklapanja ni sa jednim drugim simpleksom koji je trenutno u memoriji. Tako da obavezno popravi i taj detalj. ;-)
:-)
Marko
Dr. Marko Vojinovic
Group for Gravitation, Particles and Fields
Institute of Physics
University of Belgrade
======================
home page: www.markovojinovic.com
e-mail: vmarko at ipb.ac.rs
On Sat, 19 Nov 2022, Dušan Cvijetić wrote:
>
> Pozdrav,
>
> Dovršio sam kod za ispis u fajl. Ostalo mi je da ga iskomentarišem i ozbiljnije istestiram. Na prvi pogled se čini da radi, i da je i redosled svih stavki očuvan.
>
> Planiram, takođe, još malo da ulepšam I/O funkcije, trenutno su mahom ogromne i ružne. Ostalo mi je par nedoumica oko nekih rešenja, koja sam označio u komentarima sa // NOTE: ili //
> TODO:. Uglavnom se tiču izuzetaka, ili rada u okviru postojeće arhitekture (konkretno, čitanje različitih boja sam rešio putem case strukture, što možda nije najsrećnije, ali mi ništa
> drugo nije palo na pamet).
>
> Pre nego što nastavim sa testiranjem, imam još jedan predlog: nedavno sam počeo da učim cmake, pa bih voleo da pokušam da napravim cmake za naš projekat, ako se slažete. Tako bismo
> mogli bolje da organizujemo testove i kompajliranje.
>
> Pozdravi,
> Dušan
>
> On 10/11/2022 11:55, Nenad Korolija wrote:
> Zdravo Marko,
>
> Ni ja se nisam dalje bavio triangulatorom, ali sam bar završio i poslednje repove sa Amerikancima i valjda skoro sve oko papirologije za reizbor (tu je bilo komplikacija, ali će
> valjda sve proći u redu:).
> Ako ćeš imati vremena da baciš pogled na moje đubre, možda možemo i da budemo na vezi, pa da mi odmah kažeš šta da menjam (umesto da kucaš email), pa da nastaviš gledanje kad
> promenim...
>
> Vidimo se,
> Nenad
>
> On Thu, Nov 10, 2022 at 11:52 AM Nenad Korolija <nenadko at gmail.com> wrote:
> Zdravo Dušane,
>
> Koliko vidim, snimanje simplicijalnog kompleksa radiš redom.
> for (unsigned int lvl = 0; lvl < simpComp->elements.size(); lvl++) {
> for (auto ks : simpComp->elements[lvl]) {
> Dakle, verovatno je redosled snimanja i učitavanja isti, tj. nema potrebe da sortiraš po ID-u ili šta već.
> Dakle, trebalo bi da Markov predlog dobro testira snimanje i učitavanje - poređenje snimljenog sa onim što si nakon toga učitao pa snimio.
>
> Vidimo se,
> Nenad
>
> On Wed, Nov 9, 2022 at 10:52 PM Marko Vojinovic <vmarko at ipb.ac.rs> wrote:
>
> Pozdrav Dusane i Nenade,
>
> Evo konacno sam se vratio u Srbiju i uspeo malo da se organizujem. Dosad se nisam odazivao jer sam bio bas pretrpan obavezama, nisam stizao da pogledam sta ste
> radili... :-( Ali nadalje cu biti malo bolji sa odgovaranjem na mail-ove. ;-)
>
> Dusane, vidim da si poceo da radis na citanju kompleksa iz fajla, i otprilike sam bacio pogled na ono sto si stavio u svoj branch dosad. Nisam testirao, ali
> pretpostavljam da radi, tj. da ispisuje na ekran ono sto procita? :-) Sto se tice posebne klase za I/O, svakako je napravi ako mislis da je potrebna, i Nenad i
> ja se slazemo. Javi dokle si stigao i kako ide.
>
> Btw, fundamentalni test za I/O moze da ide otprilike ovako:
>
> (1) seed-ujes neki kompleks SimpComp *G1 (npr. 5-sferu ili stagod),
>
> (2) snimis G1 u fajl-prvi.xml f-jom za snimanje (koju si napravio),
>
> (3) ucitas fajl-prvi.xml u novi kompleks SimpComp *G2, f-jom za citanje (koju sad pravis),
>
> (4) snimis G2 u fajl-drugi.xml f-jom za snimanje,
>
> (5) uporedis fajl-prvi.xml i fajl-drugi.xml (npr. diff-om ili cime god).
>
> Ako su dva fajla uglavnom identicna (do na vrednosti UniqueID brojeva i eventualne izmene u redosledu zapisivanja isl), test je prosao, i zadatak je resen. ;-)
>
> Osim toga, ako bi uspeo jos da namestis obe f-je (za citanje i snimanje) tako da cak i redosled kojim su stavke zapisane u dva fajla bude isti, tj. da fajlovi
> budu potpuno identicni (do na UniqueID-ove), to bi onda bila bas prava stvar. :-) Ali to nije obavezno --- redosled stavki u fajlovima u principu nije bitan,
> dokle god mozemo pouzdano da se ubedimo da fajlovi zaista sadrze iste podatke.
>
> :-)
> Marko
>
> P.S. Drago mi je da si se lepo proveo u CERN-u, i da postoji mogucnost da ides opet, svakako iskoristi priliku ako ti se ukaze...
>
>
> Dr. Marko Vojinovic
> Group for Gravitation, Particles and Fields
> Institute of Physics
> University of Belgrade
> ======================
> home page: www.markovojinovic.com
> e-mail: vmarko at ipb.ac.rs
>
>
>
> On Mon, 7 Nov 2022, Nenad Korolija wrote:
>
> > Zdravo Dušane,
> >
> > Ja se slažem. Ima smisla izdvojiti celu I/O funkcionalnost u posebnu klasu, tj. fajl.
> >
> > Pozdrav,
> > Nenad
> >
> > On Sun, Nov 6, 2022 at 5:16 PM Dušan Cvijetić <dusancvijetic2000 at gmail.com> wrote:
> > Pozdrav,
> >
> > Predlažem da se za I/O funkcionalnosti napravi nova klasa, pošto
> > trenutno radim na čitanju kompleksa iz fajla i imam mnogo pomoćnih
> > funkcija koje sam napravio, a koje trenutno plutaju u etru, pa bi bilo
> > lepše grupisati ih.
> >
> > Pozdravi,
> > Dušan
> >
> > On 30/10/2022 19:58, Dušan Cvijetić wrote:
> > > Pozdrav,
> > >
> > > Radim na čitanju iz fajla, pošto sam konačno prešao na Ubuntu.
> > >
> > > Da li treba sada da razmišljam i o hvatanju izuzetaka, ili je to tema
> > > kojoj ćemo kasnije da se posvetimo?
> > >
> > > Pozdravi,
> > > Dušan
> > >
> > > On 07/03/2022 06:00, Marko Vojinovic wrote:
> > >>
> > >> Vazi. :-)
> > >>
> > >> Sto se citanja iz fajla tice, obrati samo paznju kako ces da
> > >> instanciras UniqueID boje --- moraces "rucno" da im menjas vrednosti
> > >> na osnovu onoga sto procitas iz .xml fajla, jer konstruktor
> > >> UniqueIDColor::UniqueIDColor() zadaje te id-jeve automatski.
> > >>
> > >> Takodje, kad sve zavrsis, treba da setujes vrednost za
> > >> UniqueIDColor::next_free_uid_number na ono sto bi trebalo da bude
> > >> prvi slobodan id broj. To je valjda id vrednost prvog simpleksa u
> > >> fajlu, jer su oni u fajlu poredjani rastucim redom, a ti id-jevi iz
> > >> fajla ne treba da postoje u konacnoj strukturi u memoriji, pa bi prvi
> > >> od njih trebalo da bude "slobodan" na samom kraju...
> > >>
> > >> Jedino nisam siguran kako ce next_free_uid_number da funkcionise u
> > >> kontekstu veceg broja istovremeno instanciranih kompleksa, ali to je
> > >> pitanje dizajna o kome moram jos da razmislim.
> > >>
> > >> Ostatak algoritma osmisli sam, kako mislis da je najzgodnije.
> > >>
> > >> :-)
> > >> Marko
> > >>
> > >>
> > >> Dr. Marko Vojinovic
> > >> Group for Gravitation, Particles and Fields
> > >> Institute of Physics
> > >> University of Belgrade
> > >> ======================
> > >> home page: www.markovojinovic.com
> > >> e-mail: vmarko at ipb.ac.rs
> > >>
> > >>
> > >>
> > >> On Mon, 7 Mar 2022, Dusan Cvijetic wrote:
> > >>
> > >>> Pozdrav,
> > >>>
> > >>> S obzirom da sam radio ispis u .xml fajl, preuzeću i učitavanje
> > >>> kompleksa iz fajla.
> > >>>
> > >>> Pozdravi,
> > >>> Dušan
> > >>>
> > >>> пет, 11. феб 2022. у 23:17 Marko Vojinovic <vmarko at ipb.ac.rs> је
> > >>> написао/ла:
> > >>>
> > >>> Implementirati metod u klasi SimpComp koji ce da procita dati
> > >>> fajl i instancira u memoriji simplicijalni kompleks na osnovu
> > >>> podataka iz fajla:
> > >>>
> > >>> SimpComp* SimpComp::read_complex_from_file( file* f );
> > >>>
> > >>> Input: Pointer na fajl iz koga se citaju podaci.
> > >>> Output: pointer na instancirani simplicijalni kompleks
> > >>> kreiran na osnovu podataka iz fajla.
> > >>>
> > >>> Ovaj zadatak treba da instancira nov prazan kompleks, zatim da
> > >>> cita dati fajl, interpretira sintaksu iz zadatka 7, i da na osnovu
> > >>> tih podataka popuni kompleks odgovarajucim
> > >>> simpleksima datih nivoa, a zatim i boje i susede svakog
> > >>> simpleksa ponaosob. Ovo je u sustini inverzni algoritam u odnosu na
> > >>> zadatak 7, i zavisi od tacne implementacije tog
> > >>> zadatka (tj. od tacne sintakse snimljenog fajla), pa ovaj
> > >>> zadatak treba raditi tek kada budemo sigurni da smo skroz zadovoljni
> > >>> sa radom f-ja za snimanje u fajl.
> > >>>
> > >>> I u ovom slucaju cemo mozda uraditi vise verzija f-je za
> > >>> citanje, po jednu za svaki format fajla.
> > >>>
> > >>>
> > >>> :-)
> > >>> Marko
> > >>>
> > >>>
> > >>> Dr. Marko Vojinovic
> > >>> Group for Gravitation, Particles and Fields
> > >>> Institute of Physics
> > >>> University of Belgrade
> > >>> ======================
> > >>> home page: www.markovojinovic.com
> > >>> e-mail: vmarko at ipb.ac.rs
> > >>>
> > >>>
> > >>> --
> > >>> QGHG-it-dev-list mailing list
> > >>> QGHG-it-dev-list at ipb.ac.rs
> > >>> http://mail.ipb.ac.rs/mailman/listinfo/qghg-it-dev-list
> > >>>
> > >>>
> > >>>
> > >>
> > --
> > QGHG-it-dev-list mailing list
> > QGHG-it-dev-list at ipb.ac.rs
> > http://mail.ipb.ac.rs/mailman/listinfo/qghg-it-dev-list
> >
> >
> >--
> QGHG-it-dev-list mailing list
> QGHG-it-dev-list at ipb.ac.rs
> http://mail.ipb.ac.rs/mailman/listinfo/qghg-it-dev-list
>
>
>
>
More information about the QGHG-it-dev-list
mailing list