Kirjautuminen

Haku

Tehtävät

Keskustelu: Koodit: C++: Kätevä tapahtumahallitsija

Weggo [29.04.2012 01:58:59]

#

Mikä?
Tällanen hyödyllinen tapahtumakäsittelijä tuli koodattua jokunen aika sitten auttamaan isojen projektien toteuttamisessa. Olen itse käyttänyt tätä samaa luokkaa monissa pienemmissä projekteissa ja todennut sen hyödylliseksi ja joustavaksi suunnittelumalliksi. Bugeja ei toteutuksessa ole, sillä sen verran on sitä tullut testattua.

Mikä tapahtumakäsittelijä??
Tapahtumakäsittelijää voidaan käyttää projektin 'selkärankana', eli se määrää ohjelman toiminnan abstraktilla tasolla. Tapahtumakäsittelijä onkin todella kätevä suunnittelumalli monissa projekteissa.

Koodin rakenne
Kyseinen koostuu kahdesta luokasta, ensimmäinen

template<class eventC> class ut_eventsContainer

luokka on Container eli tässä kontekstissa se pitää listaa tapahtumista ja kutsuu jokaista listassa olevaa tapahtumaa käyttäjän niin halutessa. Toinen luokka

template<typename eventT> class ut_event

on itse tapahtuma, eli se kutsutaan Container luokasta käsin.
Ylimääräinen template listaus tekee jokaisesta Container:sta ja siihen linkitetyistä tapahtumista uniikin, joten Container luokalla voidaan käsitellä useampia tapahtumakokonaisuuksia.
Tämä on todella kätevää esim. peliarkkitehtuurissa, sillä tällä tavalla voidaan erottaa loogisen puolen tapahtumat renderöinnistä.

Koodi
Koodi toimii suoraan ilman sen kummempia säätöjä header(*.h) tiedostoissa, joten kannattaa ihmeessä testata :)
Koodi sisältää boost headerin käyttöä, mutta sen voi poistaa jos kyseistä header-tiedostoa ei löydy.

#ifndef _HEADER_EVENTHANDLER
#define _HEADER_EVENTHANDLER
#include <boost\assert.hpp>
#include <vector>

// these containers are static, so they are allocated on need and deallocated on software end
template<class eventC>
class ut_eventsContainer
{
public:
    typedef eventC eventC;
private:
    ut_eventsContainer() {}
    // copying not allowed because copies may expose references to be NULL
    ut_eventsContainer(const ut_eventsContainer &) {}
    ut_eventsContainer &operator=(const ut_eventsContainer &) {return *this;}
public:
    std::vector<eventC *> m_clients;

    // runs every event without args
    void triggerEvents() const
    {
        for(size_t i = 0; i < this->m_clients.size(); i++)
            if(this->m_clients[i]->isActive())
                this->m_clients[i]->onEvent(NULL);
    }
    // runs every event with given args
    void triggerEvents(typename eventC::eventT &args) const
    {
        for(size_t i = 0; i < this->m_clients.size(); i++)
            if(this->m_clients[i]->isActive())
                this->m_clients[i]->onEvent(&args);
    }
    static ut_eventsContainer &getInstance() {static ut_eventsContainer i; return i;}
};

template<typename eventT>
class ut_event
{
    friend class ut_eventsContainer<ut_event>;
public:
    typedef eventT eventT;
protected:
    // may be NULL
    virtual void onEvent(eventT *) = 0;
    void registerEvent()
    {
#ifdef _DEBUG
        for(std::vector<ut_event *>::const_iterator i = this->m_server.m_clients.begin(); i != this->m_server.m_clients.end(); i++)
            BOOST_ASSERT_MSG(*i != this, "event already registered!");
#endif
        this->m_server.m_clients.push_back(this);
        this->m_is_active = true;
    }

    ut_eventsContainer<ut_event> &m_server;
    bool m_is_active;
public:
    ut_event() : m_is_active(false), m_server(ut_eventsContainer<ut_event>::getInstance()) {}
    template<typename eventT>
    ut_event(const ut_event<eventT> &copy) : m_is_active(false), m_server(ut_eventsContainer<ut_event>::getInstance())
    {
        if(copy.m_is_active)
            this->registerEvent();
    }
    virtual ~ut_event()
    {
        for(std::vector<ut_event *>::const_iterator i = this->m_server.m_clients.begin(); i != this->m_server.m_clients.end(); i++)
            if(*i == this)
            {
                this->m_server.m_clients.erase(i);
                break;
            };
    }

    bool isActive() const {return this->m_is_active;}
};

#endif /* _HEADER_EVENTHANDLER */

Koodin käyttö
Jos et aivan päässyt jyvälle koodista vielä tässä vaiheessa, näytän vielä miten kyseistä tapahtumakäsittelijää käytetään.

class my_event : public ut_event<int>
{
private:
    void onEvent(eventT *e)
    {
        // varmistetaan vielä että argumentti on pätevä
        if(e != NULL)
        {
            if(*e == 10)
                this->m_is_active = false; // asettaa tämän tapahtuman 'sleep' tilaan
        }
    }
public:
    my_event() {this->registerEvent();} // rekisteröi tapahtuman Container:iin
};

int main()
{
    // luo Singleton tyyppisen Container:in, joka pystyy kutsumaan ut_event<int> tyyppisiä tapahtumia
    ut_eventsContainer<ut_event<int>> &container = ut_eventsContainer<ut_event<int>>::getInstance();
    my_event e[5]; // luo tapahtumia

    for(int i = 0; i < 100; i++)
        container.triggerEvents(i); // huomaa: my_event tapahtumia ei kutsuta 10:nen iteraation jälkeen
}

Nyt olet oppinut tapahtumakäsittelijän käytön. Onneksi olkoon!
Toivottavasti tästä koodivinkistä on jotain hyötyä projekteissasi.

Metabolix [29.04.2012 17:26:36]

#

Vikoja on paljon, ja tämäkin lista on epäilemättä vielä puutteellinen:

Weggo [29.04.2012 18:18:30]

#

Se, joka tätä koodia ei taida ymmärtää, on lukija.
Koodihan on sitä varten, että sillä voidaan esittää ohjelman toimintaa. Jos kyseinen lukija ei ymmärrä koodia, ei sen sisäisen toiminnan kommentointi paranna merkittävästi käytettävyyttä.

Metabolix kirjoitti:

Koodissa on monta virhettä, jotka käyvät ilmi, kun sen kääntää Linuxissa vaikkapa GCC- tai Clang-kääntäjällä.

Kyllä, näin voi olla. Kirjoitin koodin Visual C++:lla, joten en ole varma miten hyvin sen kääntyy muilla compilereilla. Koodin voi kuitenkin kirjoittaa muotoon, jota tukee kaikki compilerit.

Metabolix kirjoitti:

Koodi on asettelultaan ja sisällöltään epäselvää ja heikosti dokumentoitua.

Sen ulkoinen käytettävyys on selkeää ja johdonmukaista.

Metabolix kirjoitti:

Käytät boostia täysin tarpeettomasti, kun voisit vaikka heittää std::logic_error-poikkeuksen.

Mielestäni throw lauseen liiallinen käyttö ei ole suotavaa. Kun on olemassa debuggaukseen tarvittavat virheenhallinta välineet, on kirjaston vääränlaisen käytön ilmoittaminen throw lauseella yliammuntaa. Poikkeukset ovat tarkoitettu pääasiassa ennalta arvaamattoman virheen hallintaan(esim. tiedostoa ei voi avata, yms.), eikä kirjaston vääränlaisen käytön ilmaisemiseen.

Metabolix kirjoitti:

Käytät vektorin kanssa epäjärjestelmällisesti välillä indeksejä ja välillä iteraattoreita.

Olen jostain lukenut, että indekseillä saataisiin pieni nopeusero verrattuna iteraattoreihin. Siksi käytän sitä niissä funktioissa, joissa nopeus on tärkeää.

Metabolix kirjoitti:

Et hyödynnä standardikirjaston mahdollisuuksia. Tiedätkö esimerkiksi funktion std::find olemassaolosta?

Kyllä, mutten hyödyntänyt sitä. Jos totta puhutaan, voisin kyllä hyödyntää sitä funktiota.

Metabolix kirjoitti:

Nimet ovat pielessä, koska eiväthän ut_event ja eventC kuvaa tapahtumaa vaan käsittelijää (handler). Entä mistä tämä ut_-etuliite edes tulee?

eventC = event_class, eventT = event_type. Mielestäni tämä kuvastaa tarpeeksi hyvin parametrejä. ut-etuliite taas tulee sanasta utils, joka oli vanha projektini. Etuliitteellä ei kuitenkaan pitäisi olla merkitystä luokan deskriptiivisen nimen kannalta.

Metabolix kirjoitti:

Koodin rakenne on omituinen, esimerkiksi m_clients on aivan perusteettomasti julkinen eikä onEvent-funktiolle ole mitään syytä välittää osoitinta.

Perusteettomasti? On turhaa kirjoittaa get & set funktioita luokan jäsenelle erikseen, kun voidaan tehdä jäsenestä julkinen. Tarkoituksena on antaa Container:in omistajalle valta kutsua tapahtumia yksitellen, sekä muuttaa niitä tarvittaessa. Parempi tapa olisi kuitenkin osoittimen määrittäminen vakioksi, jottei clienttejä pystyttäisi määrittämään NULL:ksi. Se ei kuitenkaan onnistu.
Koodin rakenteesta: en koe rakennetta omituiseksi, mahdollisesti vain lukijan tietämättömyys saa luokan tuntumaan omituiselta.

Metabolix kirjoitti:

Kommentti "may be NULL" herättää epäilyn, ettet oikein ymmärrä seuraavan rivin merkitystä.

Koodia kannattaisi tutkia tarkemmin, jotta ymmärtäisi seuraavan rivin merkityksen. Kuten tapahtumaluokan funktion sisällä lukee "varmistetaan vielä että argumentti on pätevä", voi olettaa parametrin saavan NULL arvoja. Kommentti "may be NULL" muistuttaa käyttäjää siitä, että parametri "may be NULL".
Ja miksikö käytän osoitinta? Jotta koodi olisi nopeaa. Viittauksen käyttäminen isoilla objekteilla tulisi nopeasti erittäin hitaaksi, jos Container ei haluaisi lähettää argumentteja tapahtumille.

Metabolix kirjoitti:

Kun käytät esimerkissäkin tyyppiä ut_event<int>, koko vinkistä ei käy mitenkään ilmi, miten tällä voitaisiin erottaa pelin käyttöliittymän ja logiikan tapahtumat, kuten hienosti mainostat.

"Ylimääräinen template listaus tekee jokaisesta Container:sta ja siihen linkitetyistä tapahtumista uniikin, joten Container luokalla voidaan käsitellä useampia tapahtumakokonaisuuksia." Mitä tämä tarkoittaa? Sitä, että voidaan määrittää useampia Container luokkia ainoastaan vaihtamalla template parametrin arvo. Kun template parametri on erilainen, voi luokan kuvitella olevan täysin eroava muista luokista. Luokka käyttää kuitenkin samaa rajapintaa, ainoastaan sen template parametri on erilainen.
Jollei selitykseni ollut vieläkään tarpeeksi selvä, annan esimerkin ajaa asian:

struct event_drawarg {};
struct event_logicarg {};

typedef ut_event<event_drawarg> event_draw;
typedef ut_event<event_logicarg> event_logic;

class my_rendering_event : public event_draw {};
class my_logic_event : public event_logic {};

Tässä koodissa on kaksi eri tapahtumakokonaisuutta, rendering parametreilla toteutetut tapahtumat, ja logic parametreilla toteutetut tapahtumat. Tämän jälkeen Container luokalla voidaan kutsua näitä kahta tapahtumakokonaisuutta erikseen määrittämällä template parametri joko event_drawarg tai event_logicarg.

Voin korjata osan koodiin liittyvistä virheitä jota listasit, mutta koodin toimintalogiikassa ja toteutuksessa muuten ei ole mitään sanottavaa.

Jos vielä tuli ajatuksia liittyen tähän viestiin, lähetä ne vastauksena tähän samaiseen ketjuun.

Metabolix [29.04.2012 20:17:32]

#

Weggo kirjoitti:

Kirjoitin koodin Visual C++:lla, joten en ole varma miten hyvin sen kääntyy muilla compilereilla.

Et siis osaa kunnolla C++:aa. C++ on standardoitu kieli, joten ei pitäisi olla mitään vaikeutta kirjoittaa globaalisti toimivaa koodia.

Weggo kirjoitti:

Jos kyseinen lukija ei ymmärrä koodia, ei sen sisäisen toiminnan kommentointi paranna merkittävästi käytettävyyttä. – – Sen ulkoinen käytettävyys on selkeää ja johdonmukaista.

Ei riitä, että koodi toimii ja on kopioitavissa. Vinkkien on tarkoitus olla selvää ja laadukasta koodia, jota lukemalla oppii ohjelmoimaan hyvin ja tyylikkäästi. Kattavat selitykset ja riittävä kommentointi ovat vinkeissä aivan yhtä olennaisia kuin itse koodi.

Weggo kirjoitti:

Poikkeukset ovat tarkoitettu pääasiassa ennalta arvaamattoman virheen hallintaan(esim. tiedostoa ei voi avata, yms.), eikä kirjaston vääränlaisen käytön ilmaisemiseen.

C++:n standardissa lukee: "The class logic_error defines the type of objects thrown as exceptions to report – – violations of logical preconditions or class invariants." Siis kyseinen poikkeustyyppi on juuri tähän tilanteeseen tarkoitettu. Myös tavallinen assert ajaisi saman asian, joten Boost on joka tapauksessa turha.

Weggo kirjoitti:

Olen jostain lukenut, että indekseillä saataisiin pieni nopeusero verrattuna iteraattoreihin.

Perustatko kaikki päätöksesi noin huteralle pohjalle? Yleensä nimittäin iteraattorit toimivat nopeammin. Muutenkaan tuo minimaalinen ero ei varmasti ole olennainen kenenkään pelissä, joten optimointi on hölmöä, jos mitään todettua ongelmaa ei ole.

Weggo kirjoitti:

On turhaa kirjoittaa get & set funktioita luokan jäsenelle erikseen, kun voidaan tehdä jäsenestä julkinen.

En ole tätä ehdottanutkaan. Kyseisen jäsenen sorkkiminen sotii minusta olio-ohjelmoinnin perusperiaatteita vastaan. Jos ilmenee tarve muokata listaa muuten kuin ut_event-olioita luomalla ja tuhoamalla, siihen on tilanteen mukaan parempiakin tapoja. Vinkissäsi tällaista tarvetta ei edes ole, joten vinkin tulisi olla tältä osin tyylikkäämpi.

Weggo kirjoitti:

Kommentti "may be NULL" muistuttaa käyttäjää siitä, että parametri "may be NULL".

Jos kerran puhe on parametrista, kommentissa kannattaisi lukea "parameter may be NULL". Tämä ei käynyt mistään ilmi, joten luulin, että yrität väittää, että puhtaasti virtuaalinen funktio onEvent voi olla NULL, mikä on tietenkin järjetön väite. Parametrin mahdollinen NULLius kävisi helpommin ilmi vaikka oletusargumenttia käyttämällä.

Sinänsä en ymmärrä, miksi tätä funktiota edes pitäisi voida kutsua NULL-argumentilla – ja jos pitää, miksi triggerEvents-funktiosta on kuitenkin erilliset versiot eikä vain samanlaista osoitinversiota. Koodisi on amatöörimäisen epäjohdonmukaista.

Weggo kirjoitti:

Ja miksikö käytän osoitinta? Jotta koodi olisi nopeaa.

Viittaus on sisäisesti sama asia kuin osoitin. Nopeus on siis sama.

Weggo kirjoitti:

Container luokalla voidaan kutsua näitä kahta tapahtumakokonaisuutta erikseen määrittämällä template parametri joko event_drawarg tai event_logicarg.

Tiedän kyllä sen, mutta koska et selitä sitä vinkissäsi, vinkki on vaikeatajuinen niille, jotka eivät asiaa ennestään osaa. Heille ut_event<int> on äärimmäisen epähavainnollinen käyttöesimerkki.

Weggo [30.04.2012 01:10:31]

#

Kuten olen jo edellä maininnut, koodin voi kirjoittaa muotoon, joka on standardia C++:aa. Myös kommenttien määrää voin lisätä.

Metabolix kirjoitti:

Siis kyseinen poikkeustyyppi on juuri tähän tilanteeseen tarkoitettu.

Tästä voitaisiin käydä loputtomia keskusteluja, että onko soveliaampaa käyttää assert funktioita vai poikkeusta. Tästä kuitenkin sen verran, että assert funktion käyttö olisi suotavampaa, sillä voi olla tilanne, jossa koodin pätkä sivuuttaa tuntemattomat poikkeuksen ilman sen ihmeempiä ilmoituksia. assert funktio sen sijaan ilmoittaa selkeästi koodaajalle mikä on pielessä.

Metabolix kirjoitti:

Myös tavallinen assert ajaisi saman asian, joten Boost on joka tapauksessa turha.

Kuten alussa ilmoitin, koodinpätkän voi poistaa jos boost:in assert headeria ei löydy. Kuitenkin sanallinen assert on aina selvempi kuin kryptinen "*i != this" ilmoitus.

Metabolix kirjoitti:

Yleensä nimittäin iteraattorit toimivat nopeammin. Muutenkaan tuo minimaalinen ero ei varmasti ole olennainen kenenkään pelissä, joten optimointi on hölmöä, jos mitään todettua ongelmaa ei ole.

Voin olla väärässä tässä suhteessa, joten voin korjata koodin käyttämään iteraattoreita.

Metabolix kirjoitti:

Kyseisen jäsenen sorkkiminen sotii minusta olio-ohjelmoinnin perusperiaatteita vastaan.

Väitätkö, että olisi parempaa kirjoittaa ns. 'wrapper' funktiot vektorin muokkaamiselle, taikka tapahtuman saamiseksi vektorista? Perusperiaatteena on kuitenkin code-reuse, joka tulee tässä esille. Jäsenen määrittäminen yksityiseksi vähentäisi käyttäjän valtaa päättää tapahtumista, joka vähentäisi luokan käytettävyyttä. Ainoa hyödyllinen esto vektorille olisi osoittimien määrittäminen vakioiksi.

Metabolix kirjoitti:

Sinänsä en ymmärrä, miksi tätä funktiota edes pitäisi voida kutsua NULL-argumentilla – ja jos pitää, miksi triggerEvents-funktiosta on kuitenkin erilliset versiot eikä vain samanlaista osoitinversiota.

C++:ssa osoittimella voi olla muukin merkitys, kuin dynaaminen taulukko tai muistin käsittely matalalla tasolla. Tässä osoittimella on erityinen tehtävä: se voidaan määrittää NULL:ksi, jolloin tapahtumaa kutsutaan ilman ennalta määritettyä parametria. Tapahtumalle syötetty parametri voi tällöin syrjäyttää tapahtuman oletusarvoisen käyttäytymisen. Miksi en kuitenkaan määritä triggerEvents funktioita kelpuuttamaan osoitin tyyppistä argumenttia? Koska kyseessä on luokan sisäisen toiminnan enkapsulointi; on selvempää käyttää näitä kahta funktioita kuin säätää osoittimilla sellaisenaan.

Metabolix kirjoitti:

Weggo kirjoitti:

Container luokalla voidaan kutsua näitä kahta tapahtumakokonaisuutta erikseen määrittämällä template parametri joko event_drawarg tai event_logicarg.

Tiedän kyllä sen, mutta koska et selitä sitä vinkissäsi, vinkki on vaikeatajuinen niille, jotka eivät asiaa ennestään osaa.

Totta, voisin sisällyttää paremman käyttöesimerkin joka selventäisi luokan käyttömahdollisuuksia.

Metabolix [30.04.2012 02:09:37]

#

Sellainen vielä tuli mieleen, että m_server on täysin turha jäsen ja aiheuttaa vain ongelmia esimerkiksi =-operaattorin kanssa.

Weggo kirjoitti:

Kuitenkin sanallinen assert on aina selvempi kuin kryptinen "*i != this" ilmoitus.

Saa niitä tekstejä tarvittaessa tavalliseenkin asserttiin, esim. assert(("viesti", ehto)) tai assert(ehto && viesti).

Weggo kirjoitti:

Väitätkö, että olisi parempaa kirjoittaa ns. 'wrapper' funktiot vektorin muokkaamiselle, taikka tapahtuman saamiseksi vektorista?

Ainakin lisäykselle ja poistolle olisi kannattanut alusta asti tehdä siistit ja uudelleenkäytettävät funktiot, kun kerran toiminnot joka tapauksessa ovat koodissasi.

Weggo kirjoitti:

Jäsenen määrittäminen yksityiseksi vähentäisi käyttäjän valtaa päättää tapahtumista, joka vähentäisi luokan käytettävyyttä.

Jos olet tuota mieltä, miksi sitten käytät ollenkaan suojamääreitä? Entä miksi taas m_is_active ei ole julkinen? Sen vapauttamisesta ei olisi edes mitään vaaraa.

Mahdollisesti vaarallisten "ominaisuuksien" jättäminen luokkaan ei ole hyvä tapa eikä kuulu varsinkaan kirjastoihin tai opetusmateriaaliin, jos ei ole selvää ja konkreettista perustelua.

No, olen sanottavani sanonut. Et saanut mielipidettäni muuttumaan yhdessäkään kohdassa. Perusteluja on erityisen vaikea ottaa vakavasti, kun et edes itse sovella niitä järjestelmällisesti.

Vastaus

Aihe on jo aika vanha, joten et voi enää vastata siihen.

Tietoa sivustosta