Hi, yes db.erase(name) is better. Duplicates in a map is not allowed so an item with the same key replaces the old item, so we should add a check for this. Adding const to insert and erase can be done but it will require const changes in every functions calling it. /Thanks HansN
-----Original Message----- From: Zoran Milinkovic Sent: den 4 april 2014 15:48 To: praveen malviya; Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject: RE: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] + typename AmfDbMap::iterator it = db.find(name); db.erase(it); The code above can be replaced with: db.erase(name); So, no extra checks for iterators. Regards, Zoran -----Original Message----- From: praveen malviya [mailto:praveen.malv...@oracle.com] Sent: den 4 april 2014 15:24 To: Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Please find comments inline with [Praveen]. Thanks, Praveen On 26-Mar-14 4:57 PM, Hans Nordeback wrote: > osaf/services/saf/amf/amfd/include/db_template.h | 63 > ++++++++++++++++++++++++ > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, > +but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are > +licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for > +full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include <map> > +#include <string> > + > +template <typename T> > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map<std::string, T*> AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template <typename T> > +void AmfDb<T>::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + [Praveen] Patricia tree returns failure for duplicate entry or if addition itself fails. Don't we require such return code here? We check return code after addition. Also can we make argument as constant. > +template <typename T> > +void AmfDb<T>::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} [Praveen] If db.find returns NULL then this function should return error otherwise it crashes. something like this: if (it == db.end()) return ERROR; db.erase(it); Also can we make argument as constant. > + > +template <typename T> > +T *AmfDb<T>::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > + return NULL; > + else > + return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel