On Tuesday 16 July 2002 12:48 pm, Martin Vermeer wrote:
> On Mon, Jul 15, 2002 at 05:57:01PM +0100, Angus Leeming wrote:
>
> ...
>
> > Do you really need to make char Counters::hebrewCounter(int n) a class
> > method? It doesn't use any class variables.
> >
> > I'd suggest, in the .C file only:
> >
> > namespace {
> >
> > char Counters::hebrewCounter(int n)
> > {
> >     ...
> > }
> >
> > } // namespace anon
> >
> > Ditto for other, similar helper functions. Keep the header file minimal.
> >
> > Angus
>
> Did this. Diff attached.
>
> Martin

One more suggestion: naked pointers are evil. Naked pointers in an STL 
container are doubly evil. Wrap that pointer in a boost::shared_ptr. Memory 
is automatically delete-d as the list goes out of scope.

There are /lots/ of examples in the code base. Something like this:

class Counter {
        ///
        typedef std::map<string, boost::shared_ptr<Counter> > CounterList;
        ///
        CounterList counterList;
}

@@ -73,36 +108,35 @@ void Counters::newCounter(string const &
 {
        // First check if newc already exist
        CounterList::iterator cit = counterList.find(newc);
-       // if alrady exist give warning and return
+       // if already exist give warning and return
        if (cit != counterList.end()) {
-               lyxerr << "The new counter already exist." << endl;
+               lyxerr << "The new counter already exists." << endl;
                return;
        }
        counterList[newc] = boost::shared_ptr(new Counter());
+       cit->second->setMaster("");


Why don't you move on and try and use it ;-)
Angus

Reply via email to