C++ listener

C/C++

Sziasztok,

Egy beagyazott linuxos board-ra kell fejlesztenem, ezert csak az STL-t szeretnem hasznalni. Egy listenert kell megvalositanom (Sajnos ennel jobban nem tudtam kodkent megjeleniteni).


class A;
typedef std::function<void(const A *, char)> Callback;
class A {
public:
A(){}
~A(){}
void addListener(int, Callback callback);
void removeListener(int, Callback callback);
private:
std::map<int, std::vector<Callback>> mCallbacks;
}


void A::notify(char ch) {
auto it = mCallbacks.find(mState);
if(it==mCallbacks.end())
return;
vector<Callback> scb = it->second;
if (scb.empty())
return;
for(unsigned int i=0;i<scb.size();i++)
scb[i](this,ch);
}


void A::addListener(int state, Callback callback)
{
auto it = mCallbacks.find(state);
if (it == mCallbacks.end()) {
vector<Callback> cb=it->second;
cb.push_back(callback);
mCallbacks.insert(map<int,vector<Callback>>::value_type(state, cb));
return;
}
vector<Callback> scb = it->second;
scb.push_back(callback);
}


void A::removeListener(int state, Callback callback)
{
auto it = mCallbacks.find(state);
if (it == mCallbacks.end())
return;
vector<Callback> scb = it->second;
if (scb.empty())
return;
// scb.erase(callback); <--------- Nem talal illeszkedo fuggvenyt
}

Az scb.erase-re uzengeti, hogy
no matching function for call to ‘std::vector<std::function<void(const A*, char)>>::erase(Callback&)’

Kellene irnom egy template fggv.-t, vagy belekeveredtem?

koszi,
Zamek

  • 9272 megtekintés

( Zsugabubus | 2013. 11. 23., szo – 23:59 )

A vector.erase() egy iteratort vár paraméterként, nem elemet.

( Joejszaka v | 2013. 11. 24., v – 10:57 )

Milyen platform?

Vigyazz, mert ha nincs virtualis memoria a cuccon, akkor a memoriad toredezni fog, es le fog lassulni a program egy ido utan.

Beagyazott rendszereken csak ovatosan hasznalunk dinamikus memoriat.

( Replaced | 2013. 11. 24., v – 11:17 )

std::remove ?

szerk:

pontositva: scb.remove(cb)

--
NetBSD - Simplicity is prerequisite for reliability

Mindig az a baja, hogy nem talal == operatort.
A scb egy vector, amelyben callback fuggvenyek cimei vannak.

no matching function for call to ‘std::vector >::erase(Callback&)’

Ha jol ertelmezem, az a baja, hogy a vectorban std::function-ok vannak, es a Callback-re referenciat akarok kerestetni vele.

Udv,

en egy ilyen kategoriaju vason (ha valoban a wandboardrol van szo) nem felnek Qt-t hasznalni (persze lehetnek mas elvarasok ami ezt itt megneheziti). A libQtCore.so.4 ARM-re forditva, strippelve 2.7M, a QextSerialPort statikus libje 61K, ebben vegulis benne van minden ami ide kell (signal-slot mechanizmus, filekezeles, adatszerkezetek, sorosport kezeles) ennyit szerintem ki lehet birni, es meguszod az STL-es mokolast.

Disclaimer:
Magaban a problemaban nem merultem el, csak a partszelrol okoskodok.

( Joejszaka v | 2013. 11. 24., v – 13:45 )

En a fuggvenypointereket messzirol kerulnem.

Felejtsd el, ha van ra lehetoseg.

Inkabb csinalj egy ososztalyt, ami egy abstract class, es van egy pure virtual callback() methodusa. Az abstract class-ok id-it, vagy pointereit rakd a map-ba.

A fuggvenypointer helyett talaltak ki a virtualis fuggvenyt.

( Joejszaka v | 2013. 11. 24., v – 13:47 )

vector scb = it->second;
if (scb.empty())
return;

Na, ez a kod igen verzik. Tudod mit csinal, ha pl. scb 10000 elemu? :)

const vector &scb = it->second;
if (scb.empty())
return;

Ez igy mar jo.

( Joejszaka v | 2013. 11. 24., v – 13:50 )

Ugyanigy itt:

vector cb=it->second;
cb.push_back(callback);

Hazi feladat: megszamolni hanyszor masolodik a vector.

Ez nem Java, hanem C++, itt tudni kell, hogy mikor csinal az ember deep copy-t es mikor shallow copy-t.

( bocs | 2013. 11. 24., v – 14:31 )

1. Az addlistener-ben it->second -ot írsz,pedig elotte kiderült, hogy
it == mCallbacks.end()
ez crash, azonnal javítani kell.

2. mCallbacks.insert(map<int,vector<Callback>>::value_type(state, cb)); helyett
mCallbacks[state] = cb;
Hát nem sokkal egyszerubb?

3. Nagyon eroforráspazarló, hogy vector<Callback> másolódik ide-oda.
Használj referenciát! Pl az addlistener-ben
vector<Callback> scb = it->second; itt másolatot készítesz az egész tömbrol, ehelyett
vector<Callback>& scb = it->second;
ez csak 1 pointert vesz ki, teljesítményben ég és föld különbség

Emiatt az addlistener ráadásul teljesen hibás is, feltétlen javítandó:
a lemásolt vector-ba beleteszel egy elemet, aztán az egész vektort eldobod, nem kerül vissza a map-be!
Ez a válasz a kérdésedre is. Nincs illeszkedo függvény, mert soha nem is regisztráltál be egyet sem.

4. removelistener-ben is lemásolod az egész tömböt. Ehelyett:
vector<Callback>& scb = it->second;

5. A függvényeidban érték szerint adod át a Callback-et.
Ez mindig egy objektum létrehozást, lemásolást jelent, akár költséges is lehet.
Ehelyett referenciaként add át, az garantáltan olcsó. Tehát ehelyett:
void addListener(int, Callback callback);
használd ezt:
void addListener(int, Callback& callback);

6. removelistener-ben:
scb.erase(callback);
Ez így vektorra soha nem muködhet, az csak iterátort átvéve képes törölni a mutatott elemet.
(Ahogy mások is mondták, az azonban vektornál költséges muvelet.)
Itt vektort használva nincs más lehetoséged, mint végigmenni a vektoron, megkeresni a callback-et és ha megvan, az erase-zel kitörölni:

	for(vector<Callback>::iterator it=scb.begin(); it!=scb.end(); ++it)
	{
		const Callback& cb = *it;
		if( cb == callback )
		{
			scb.erase(it);
			break;
		}
	}

7. Használj multimap-et a map helyett! Pont a te esetedre van kitalálva. Vektoros szenvedés megspórolva.

( Andrei | 2013. 11. 24., v – 18:20 )

A tobbiek ramutattak 1-2 hibara (pl. sulyos baki hozzaferes soran kimasolni a callback tombot), de szerintem pont a lenyeg maradt ki:

Sajnos itt egy kicsit zsakutcaban vagy, az std::function<> template-nek nincs ==/!= operatora. Illetve van neki, de csak nullptr_t-hez lehet hasonlitani (ami azt jelenti, hogy meg tudod nezni, hogy az illeto peldany meghivhato-e vagy nem). Ha jobban belegondolsz, akkor ez igy is van rendjen, mert eleg nehez elkepzelni olyan muveletet, ami ket std::function objektumot hasonlitana ossze (az std::function-ban tarolt callable object lehet lambda, functor, fgv pointer, member fgv pointer ....).

Azokon a bakikon tulmenoen, amit a tobbiek feszegettek, szerintem ez a huzosabb koncepcionalis hiba. Te azzal a feltetelezessel elsz, hogy a fuggveny objektum azonosithato (azaz megallapithato, hogy azonos-e egy masik objektummal), igy jo lehet handle-nek, sajnos nem ez a helyzet a fenti okok miatt.

Csak hogy ne csak a pofamat jarassam, hanem valami javaslattal is eljek:

1: a Callback-eket tarolhatod igy:


private:
typedef std::multimap<int, Callback> callbacks_type;
public:
typedef callbacks_type::iterator callback_handle;

2: modositani kellene az addListener es removeListener fgv-t:


callback_handle A::addListener(int state, Callback callback);
void A::removeListener(callback_handle handle);

Magyarazat:
- az iterator amit a Callback reprezentalasara hasznalunk az mar osszehasonlithato/keresheto/eltavolithato. Mindezek tetejebe meg olcso is masolni, mig a Callback objektum masolasa pokolian koltseges is lehet (fuggoen attol, hogy tartalmaz-e mar kotott argumentumokat). Marpedig egy handle-t altalaban ide-oda passzolgat az ember, szoval jo ezt a koltseget alacsonyan tartani.
- szukseges, hogy az iterator amit visszaadsz, ne valjon invalidda Callback-ek beszurasa es a kivetele soran. Ezt a multimap teljesiti.
+1: Mivel a Callback-et ertek szerint passzolod be (ami itt a helyes megkozelites!!!), ezert erdemes move-ot hasznalni a megfelelo helyen...

tehat felteve, hogy a fenti typedef-et hasznalod, igy nezne ki az addListener fgv:


callback_handle A::addListener(int state, Callback callback) {
return mCallbacks.insert(std::make_pair(state, std::move(callback));
}

akkor mar raadaskent a ket masik fuggveny is:


void A::removeListener(callback_handle handle) {
mCallbacks.erase(handle);
}


void A::notify(char ch) {
auto r = mCallbacks.equal_range(mState);
for(auto it = r.first; it != r.second; ++it) {
it->second(this, ch);
}
}

Remelem nem vetettem hibat, probald ki! Ja es persze vegig feltettem, hogy c++11-et hasznalsz (ez mondjuk valszeg igaz az std::function miatt). Jo hir, hogy a fuggvenyek eleg egyszeruek, es a removeListener-nek mar nem kell state parameter.

Erre vannak a virtualis fuggvenyek a Java-ban. Konkretan c++-ban az std::function egy igen jo megoldas ezekben az esetekben.

- Mi van ha van egy legacy osztalyod, amit nem modosithatsz, de van olyan fuggvenye tudna fogadni a megfelelo callback-et.

pl.


struct foo {
void feed(A*, char);
};

Ne lehessen listener, csak mert nem ugy hivjak a fuggvenyt? Az std::function<>-ra epulo megoldassal ennyit kell tenni:


A a;
foo bar;
a.addListener(some_state, std::mem_fn(&foo::feed, bar));

Persze mondhatod, hogy ezen lehetne segiteni:

struct foo_adapter : public foo, public ilistener {
void fire(A* a, char ch) {
feed(a, ch);
}
/// es meg itt lesz kod boven
};

ott kezdodik a dolog, hogy ezt az adaptert jol megirni elegge hosszadalmas is lehet. A c++11-ben vannak feature-ok, amik segitenek (pl. konstruktor delegalas, de mi van, ha gcc-4.7 elotti forditod van?), de azert meg lehet porgetni a szopogepet, ha nekiallsz... Meg oszinten, egy egesz osztaly csak azert mert mashogy hivnak egy fuggvenyt.

de mi van, ha:
- foo-nak privat konstruktorai vannak (mert o egy singleton)?, akkor ez nem is mukodik
- foo destruktora nemvirtualis: ugy kell hasznalni az adaptert, mint a himes tojast, mert eleg csak egyszer foo-ra mutato pointeren keresztul torolni es maris csunya dolgok jonnek.

Masik pelda:


A a;
a->addListener(some_state, [](A* a, ch) { std::clog << a << " incoming char: " << ch << std::endl; });

es maris generaltunk egy aranyos egysoros tracert a-hoz, ami minden inputot kuld a logba...

Egyebkent kellene egy uj osztalyt szarmaztatnod az ilistener-bol, azt peldanyositani (a konstruktorban regisztralni, a destruktorban torolni a regisztraciot), a peldanyt valahol tarolni....

2. Tegyuk fel, hogy:


struct foo : public ilistener {
};

struct bar : public ilistener {
};

struct baz : public foo, public bar {
};

Na maris osszehoztunk egy jo kis "dreaded-diamond"-ot. Lehet ezt mukodokepesse tenni, de nem konnyu jol csinalni, vannak spec esetei... Mondjuk ugy, hogy en sok mindennel megprobalkoznek, csak hogy NE kelljen igy csinalni... ;)

3. virtualis fgv eseten legalabb protected-nek kell lennie callback fuggvenyeknek, az std::function<>-alapu megoldasnal siman lehet privat is fuggveny, ami fogadja a callback-et.

4. Ebben a konkret esetben hogy csinalod meg virtualis fuggvenyekkel, hogy egy objektumbol tobb regisztraciot is vegzel mas es mas allapothoz)? Leginkabb sehogy...

Es hidd el vannak meg tovabbi aprosagok is, amik az std::function<>-fele megoldast eleg vonzova teszik az ilyen esetekre.... A virtualis fuggveny koncepciojaval az a gond, hogy nem csak a szignaturat koti meg, hanem a nevet, a lathatosagi osztalyt es az ososztalyt is (amikre pedig semmi szukseg). Komoly lyukakat losz a az informaciorejtesbe es a kod ujrafelhasznalhatosagba, azaz a c++ legfontosabb alapelvei kozul kettobe is.

Ha csak annyi az elvaras, hogy ez-meg-az az erkezo parameterlista, akkor miert akarjuk megmondani, hogy mi legyen a fuggveny neve, hogy taggfuggveny legyen egy osztalybol(es hogy szarmazzon egy masik osztalybol) es legalabb protected legyen? (mert ezt mindet kikotod a virtualis fuggvennyel!!!).

Javaslat az implementaciohoz:

Erdemes lehet torolni a callback szignaturabol az A* argumentumot, mert ugy sokkal flexibilisebb lesz a design. Ha a listenernekszuksege van ra, akkor bind-dal hozzapakolja a Callback objektumhoz a hivas helyen.


class A {
// ez az egyszerubb szignatura
typedef std::function Callback;
}

struct listeners {
listeners(bool trace = false) :
h1(a.registerCallback(some_state1, std::bind(&listeners::handler1, this, std::placeholders::_1)), // ez a handler nem kapja meg az A* ptr
h2(a.registerCallback(some_state2, std::bind(&listeners::handler2, this, &a, std::placeholders::_1)), // ez a handler megkapja az A*-ot
h3(trace ? a.registerCallback(some_state2, std::bind(&std::ostream::put, std::clog, std::placeholders::_1)) : A::callback_handle()) // ez meg egy alap opcionalis logger ;)
{
}
private:
void handler1(char ch) {}
void handler2(A*, char ch) {}

A a;
A::callback_handle h1;
A::callback_handle h2;
A::callback_handle h3;
}

No ez volt vagy 15 sor, ezt valaki megcsinalja virtualis fuggvenyekkel, az eleg ugyes...

Elég gáz, hogy memleak-es példa van egy oldalon, amit kezdők olvasnak elsősorban...

A valgrind meg nem ad minden kérdésre választ, és ha ad sem biztos, hogy jót.

Az ember előbb gondolkodik, és csak aztán használ valgrindet bugkeresésre. Nem fordítva.

"...handing C++ to the average programmer seems roughly comparable to handing a loaded .45 to a chimpanzee." -- Ted Ts'o

Szerintem nem az. A mem_fun hasznalatara mutat peldat, es nem azt taglalja, hogyan irj tokeletes programot. Hibakezeles sincs benne, feltunt? ;) Egy jo pelda tomor es lenyegretoro.

Mondtam, hogy a valasz trivialis. De ha megsem az, akkor eloszor jobb ha az ember "megkerdez" egy tool-t, mielott egy forumon erdeklodik. Az elsobol ugyanis tobbet lehet tanulni. Es ez lenne a cel, gondolom.

Az oldal írói nem értenek egyet. Írtam nekik tegnap, hogy hiányzik a felszabadítás, és mára ott van.

Abban igazad van, hogy nincs minden hibalehetőség lekezelve (mondjuk a potenciális bad_alloc-on kívül nem látok mást), és valahol meg kell húzni a határt, de ebben az esetben 2 plusz sorról beszélünk, ami nem rontja az olvashatóságot, sőt.

Egyébként én azt a határt a mindig rossz, és a hiba esetén rossz között húznám meg ha már nem lehet tökéletes a példa, nem pedig a mindig rossz fölött...

"...handing C++ to the average programmer seems roughly comparable to handing a loaded .45 to a chimpanzee." -- Ted Ts'o

En ertekelem a szorgalmadat, tenyleg. De irhatnal nekik megint, hogy mivel az oldalt alapvetoen kezdok latogatjak, talan jobb lenne, ha nem mutatnak be egy 15 soros peldan belul, hogyan kell ketfelekeppen egy vector-on vegignyargalni, mikozben semmi mast nem akarnak, mint a mem_fun hasznalatat szemleltetni ;)

De, fel kéne. Vagy a vektorba std::unique_ptr<std::string> std::string* helyett...

(Mondjuk ha van unique_ptr akkor az c++11, tehát mem_fn kell (a mem_fun deprecated).)

"...handing C++ to the average programmer seems roughly comparable to handing a loaded .45 to a chimpanzee." -- Ted Ts'o

Ez jó kérdés. Azt gondolnám, mivel amúgy is befejeződik a program futása, az erőforrások automatikusan felszabadulnak.
Hogy ez kinek bántja a szemét, az más kérdés. Az enyémet nem annyira, mert nem okoz valós problémát, legfeljebb akkor, ha valaki ész nélkül nekiáll bemásolni valahova ezt a kódot. De pl Singleton-t sem szokás felszabadítani.

szerintem igenis azkell, hogy minden lefoglalt byte fel van szabadítva:
- holnap átírják a Singletont. Ha nincs ott explicite kiírva egy kód, akkor azt nem refaktorálják: memóriaszivárgás lesz.
- az automatikus memória felszabadítás ellenőrzők működése így lehetetlenné válik, ráadásul egy singleton miatt esetleg egy csomó más member objektum sem lesz megfelelően elpusztítva.

Ha a GoF fele singleton-t hasznalod, akkor valoban felszabaditatlan lesz kilepesnel a memoria. Ezen javithatsz annyit, hogy a letrehozott objektumot auto_ptr-ben vagy unique_ptr-ben tarolod, attol fuggoen, hogy mi erheto el. Mondjuk szalbiztos ettol meg nem lesz.

Amennyiben a Meyers-fele singletont hasznalod es a letrehozott objektum egy fuggvenyre lokalis statikus valtozoban tarolodik, akkor nem kell smart pointer. Ha a regi szabvany szerint megy a forditod, akkor ugyan szalbiztos nem leszel, de az uj szabvany mar kotelezoen eloirja, hogy a statikusok inicializalas szalbiztosan kell, hogy tortenjen.

Szerintem erdemes arra menni, hogy leak-mentes legyen a programod. Egyfelol, mert egy objektum nem csak memoriat (azaz process kilepessel visszakapott) eroforrast kezelhet. Masfelol meg, ha a valgrind-ban van 346 leak reportod, akkor a 347. valoban problemas bug mar nem fog szemet szurni....

Tehat jelen esetben, ha csak annyi a teendo, hogy a Meyers fele singleton-t hasznalod (ami szerintem meg elegansabb is), es megsporolsz egy leak-et magadnak, akkor egyertelmuen azt kell valasztani.