generic classes for safe %macro substitution

Harri Porten porten at kde.org
Wed Jan 1 19:28:08 GMT 2003


Hi !

On Wed, 1 Jan 2003, Oswald Buddenhagen wrote:

> as macro expansions (aka "expandos") are needed all over the place, i've
> written some classes for this task.

I don't have a picture of the situation so I'll refrain myself to give
some suggestions regarding the API.

> the class names are somewhat scary. if you can think of equally
> descriptive shorter names, then yell.

What I find slightly "scary" is the _number_ of classes. Can't they maybe
be merged into one class with different function names or flags denoting
the different behaviour ?

> the derived classes got messier than expected, to be honest.
> do you think it would make sense to remove the non-static versions of
> the functions together with the set*() methods?

I was actually about to make a comment completely opposite to Simon: my
initial feeling was that a set of static functions would do the job
equally well (especially considering the low numbers of data members and 
the lack of any state other than the map passed to the ctor).

> in case you're wondering about the "goto nohit" ... i looked at the asm
> gcc generates for the complex expression i originally used and it was
> not fun. so i helped it a bit ...

gotos are perfectly fine I think.

Now to the header (for the sake of consitency in KDE's API):

class KMacroExpander {
public:
    KMacroExpander( QChar c = '%' );
    virtual ~KMacroExpander();
    void expandMacros( QString &str );

A QString parameter doesn't really warrant an in-place modification.
Returning the processed value often eases the usage and doesn't cost much
more. Even your kmxtest.cpp program contains extra copy statements because
of this :)

    bool expandMacrosShellQuote( QString &str );
    bool expandMacrosShellQuote( QString &str, uint &pos );

Same issue as above + suggestion to to combine those two like:

   QString expandMacrosShellQuote( const QString &strm, bool *b = 0,
				   uint *pos = 0 );

    enum Quoting { noquote, singlequote, doublequote, dollarquote, 
		parent, subst, group, math };


99.9% percent of our enums use "camel style" capitalization, i.e. your
code will fit in better with NoQuote etc.

    } Save;
    KMacroExpanderPrivate *d;
};

As the Save and State structs are only used within a private function you
might as well move their definitions into the .cpp file (not that elegant
I know, but save the compiler from parsing those each time the .h is
included somewhere).

    void setMacroMap( const QMap<QString,QString> &map );
    QMap<QString,QString> &macroMap() const;

You can return a value here without worrying about the costs (shallow
copy). Returning a reference gives the user direct access to private data
and thus breaks encapsulation.

private:
    KMacroMapExpanderBasePrivate *d;
};

    void expandMacros( QString &str ) { KMacroExpander::expandMacros( str
); }
    bool expandMacrosShellQuote( QString &str ) { return
KMacroExpander::expandMacrosShellQuote( str ); }
    bool expandMacrosShellQuote( QString &str, uint &pos ) { return
KMacroExpander::expandMacrosShellQuote( str, pos ); }

These functions can all be "const", can't they ?

private:
    bool isIdentifier(uint c) { return c == '_' || (c >= 'A' && c <= 'Z')
|| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); }
};

"const" :)

Harri.





More information about the kde-core-devel mailing list