tags:

views:

529

answers:

12

I am creating my own implementation of XUL in C++ using the Windows API. The fact that the elements are constructed by the XML parser requires that they have identical interfaces, so that we don't need to write custom code for each element constructor. The result is that most of my elements look like this:

class Button : public Element
{
public:
    static const char * Type() { return "button"; }

private:
    friend class Element;
    Button(Element * inParent, const AttributesMapping & inAttributesMapping);
};


class Label : public Element
{
public:
    static const char * Type() { return "label"; }

private:
    friend class Element;
    Label(Element * inParent, const AttributesMapping & inAttributesMapping);
};


class Description : public Element
{
public:
    static const char * Type() { return "description"; }

    virtual bool init();

private:
    friend class Element;
    Description(Element * inParent, const AttributesMapping & inAttributesMapping);
};

So there is a lot of code duplication here. I wonder if it would be a good idea to replace them with macro calls like this:

#define DECLARE_ELEMENT(ElementType, XULName)           \
class ElementType : public Element                      \
{                                                       \
public:                                                 \
    static const char * Type() { return XULName; }      \
                                                        \
private:                                                \
    friend class Element;                               \
    ElementType(                                        \
        Element * inParent,                             \
        const AttributesMapping & inAttributesMapping); \
};                                                      \


DECLARE_ELEMENT(Window, "window")
DECLARE_ELEMENT(Button, "button")
DECLARE_ELEMENT(Label, "label")

I haven't completely worked out the concept yet, so a few things are missing here, like the class definitions, and (maybe) the ability to add methods per element.

But I'd like to know your opinion of using macros in this situation. Feel free to share your thoughts.

EDIT

I am now using a small ruby script that generates the source and header files from a set of templates. I enhanced the scripts so that the files are also automatically marked for addition on SVN, and the Visual Studio project file is modified to include the files. This saves me a lot of manual labor. I'm quite happy with this solution. FYI this is what the templates look like now:

#ifndef {{ELEMENT_NAME_UPPER}}_H_INCLUDED
#define {{ELEMENT_NAME_UPPER}}_H_INCLUDED


#include "XULWin/Element.h"


namespace XULWin
{

    class {{ELEMENT_NAME}} : public Element
    {
    public:
        static ElementPtr Create(Element * inParent, const AttributesMapping & inAttr)
        { return Element::Create<{{ELEMENT_NAME}}>(inParent, inAttr); }

        static const char * Type() { return "{{ELEMENT_TYPE}}"; }

        virtual bool init();

    private:
        friend class Element;
        {{ELEMENT_NAME}}(Element * inParent, const AttributesMapping & inAttributesMapping);
    };

} // namespace XULWin


#endif // {{ELEMENT_NAME_UPPER}}_H_INCLUDED

CPP document:

#include "XULWin/{{ELEMENT_NAME}}.h"
#include "XULWin/{{ELEMENT_NAME}}Impl.h"
#include "XULWin/AttributeController.h"
#include "XULWin/Decorator.h"


namespace XULWin
{

    {{ELEMENT_NAME}}::{{ELEMENT_NAME}}(Element * inParent, const AttributesMapping & inAttributesMapping) :
        Element({{ELEMENT_NAME}}::Type(),
                inParent,
                new {{ELEMENT_NAME}}Impl(inParent->impl(), inAttributesMapping))
    {
    }


    bool {{ELEMENT_NAME}}::init()
    {
        return Element::init();
    }

} // namespace XULWin
+4  A: 

I think macros can be okay to reduce repetition (and thus, the risk of introducing errors) at a low level like this.

The use of macros will remain very localized, and should make the code as a whole easier to understand. Of course it might require some documentation effort too.

unwind
I agree completely. Avoid Macros doesn't mean ban them alltogether. In this case, you code is made much more clear with the macros and much less error prone through DRY.
Dolphin
A: 

I would vote for macros in this case. They aren't that bad after all, you shouldn't try to write inline functions with them but other than that they are good.

vava
+3  A: 

Use whatever makes the code simpler.

DRY and Avoid Macro both have the same goal: making your code simpler.

  • DRY: avoid repetition
  • Avoid Macro: because they can introduce hard to diagnose compiler errors or hard to diagnose bugs (as they bypass namespace boundaries and are not C++ aware / typesafe).

As usual with the guidelines, I would thus suggest to follow the spirit rather than the letter. In your case it appears evident that the macro will actually simplify your code, so you should probably use it.

However, taking into account the problems that a macro may introduce, make sure to name it 'safely'. Include the project name / file name at the beginning for example to reduce the potential 'clash' with an existing macro.

(you can take a look at BOOST header guards to have an idea of naming convention)

Matthieu M.
A: 

I think the use of macros is okay in this case, but only if you

  • can develop a solution that is not too complex but covers (preferably) all necessary Element class structures
  • document the macro well
  • are aware that some IDEs have problems with macro-generated class structures and can live with the consequences
hjhill
+3  A: 

Be wary of using macros that replace class definitions if you plan on using automatic code documentation tools like doxygen. You'll have to run the code through the preprocessor before generating any documentation. Not, perhaps, the most important consideration, but something to consider nonetheless.

Bill Carey
Thanks for the hint! I hadn't thought so far yet.
StackedCrooked
I'm working on some legacy code that used macros to replace all the class and function delcarations and definitions. Makes getting a handle on the structure of the program difficult as it confounds both the IDE and the documentation generators.
Bill Carey
+2  A: 

IMHO this macro is justified. Although I think it would be better to add #undef DECLARE_ELEMENT to prevent dangling macros. (Unless you plan to use this macro in other files as well.)

Note however that this will work only if those classes will never differ much (or best at all).


There is yet another solution using templates. Consider following code

namespace impl
{
 struct ButtonTag;
 struct LabelTag;


 template< typename TypeTag >
 struct NameGenerator;

 template<>
 struct NameGenerator< ButtonTag >
 {
  static const char * getName() { return "button"; }
 };

 template<>
 struct NameGenerator< LabelTag >
 {
  static const char * getName() { return "label"; }
 };


 template< typename TypeTag >
 class SimpleElement : public Element
 {
 public:
  static const char * Type()
  { return NameGenerator< TagType >::getName(); }

 private:
  friend class Element;

  SimpleElement(
   Element * inParent,
   const AttributesMapping & inAttributesMapping);

 };
}

typedef impl::SimpleElement< impl::ButtonTag > Button;
typedef impl::SimpleElement< impl::LabelTag > Label;

It is somewhat more verbose however avoids macros.

Adam Badura
+1  A: 

code samples

enum Types { BUTTON, LABEL,...}

struct TypeList {
    static const char * Type(const int nID)
    {
         switch(nID) {
         case BUTTON: return "button";
         ...
    }
};

template<ID>
class IElem : public Element
{
private:
    static TypeList m_oTypeList;

public:
    static const char * Type() { return m_oTypeList.Type(ID); }
private:
friend class Element;
    IElem(Element * inParent, const AttributesMapping & inAttributesMapping)
    {...}
};

for non-common functions and specialized

class Button : public IElem<BUTTON>
{
...
}
EffoStaff Effo
+4  A: 

I would not use a macro here. The clue is in your class "Description", which has an extra member function init, which the others don't. So you wouldn't be able to use the macro to define it, but you'd instead expand the macro manually and add the extra line.

To me, this is a bigger violation of DRY than just writing out all the class definitions. Almost not repeating yourself, but doing it just for one case, often ends up harder to maintain that repeating yourself consistently. DRY is about finding good abstractions, not just cutting down on boilerplate.

I might replace those constructors, though, with a SetAttributes function in class Element. That might cut the amount of boilerplate actually required in each derived class, since constructors are the one thing that can't be inherited from the base. But it depends how similar the implementations are of the constructor of each class.

Steve Jessop
+32  A: 

If you use a template solution, you can avoid macros and avoid repeating yourself:

template <const char *XULName>
class ElementType : public Element
{
public:
    static const char * Type() { return XULName; }

private:
    friend class Element;
    ElementType(
        Element * inParent,
        const AttributesMapping & inAttributesMapping);
};

char windowStr[]="window";
char buttonStr[]="button";
char labelStr[]="label";

typedef ElementType<windowStr> Window;
typedef ElementType<buttonStr> Button;
typedef ElementType<labelStr> Label;

Rule of thumb: Templates can be used for just about everything that macros were necessary for in C.

Implementation note: String literals can't be used directly as template arguments because they have internal linkage -- that's why you need the windowStr etc. In practice, you would want to put the declarations of windowStr, buttonStr and labelStr in the H file and the definitions of those strings in a CPP file.

Martin B
+1, I knew one could not use a literal string as a template argument but that would be the first time I see this work around. Not sure I'd like to use it, but it's always good to have more tools for the job!
Matthieu M.
@Matthieu: Agreed... I find this workaround a bit iffy, too. A more agreeable solution would be to use a traits class... chances are, in this kind of situation, that would be required sooner or later anyway.
Martin B
+5  A: 

As an alternative, you might consider generating the code an a separate build step, instead of using the preprocessor. I like cog, But you could use whatever you like -- This way you get full programmatic control over what is generated. (Macros are powerful, but limited in what you can do.)

Sean McMillan
I was surprised that noone else saw the third option. ++ for code generation. You can add custom parts where you need them. You can generate proper documentation. You don't break line numbering / debugging the way macro would do it. If you cannot use a c++ template for this solution, use code generation.
viraptor
+1  A: 

I could even go a bit further and use both the single hash and double hash feature when using macros. Single hash create string constants and the double concatenate identifiers to build new combined.

#define DECLARE_ELEMENT(ElementType)                     \
class C ## ElementType : public Element                  \
{                                                        \
public:                                                  \
    static const char * Type() { return # ElementType; } \
                                                         \
private:                                                 \
    friend class Element;                                \
    C ## ElementType(                                    \
        Element * inParent,                              \
        const AttributesMapping & inAttributesMapping);  \
}

DECLARE_ELEMENT(window); // defines Cwindow
DECLARE_ELEMENT(button); // defines Cbutton
DECLARE_ELEMENT(label);  // defines Clabel

For instance the below code is something I sometimes write to test the sizeof for some common types.

#include <stdio.h>

#define OUT( _type ) printf("sizeof(%s) = %d\n", #_type, sizeof(_type))

int main() {
  OUT( char );
  OUT( int );
  OUT( short );
  OUT( long );
  OUT( long long );
  OUT( void* );
  return 0;
}
epatel
A: 

This code looks an awful lot like the program that Tom Cargill dissects and reassembles in Chapter 1 of his book "C++ Programming Style", dating back to 1992. Admittedly, that code did not use macros to replicate the almost identical classes, but the net result looks awfully similar from out here.

Jonathan Leffler
Are you tricking me into checking out this book? :)
StackedCrooked
@StackedCrooked: from the library - yes; buying - not necessarily. There are more recent books that cover modern C++ better, but this still covers much of the basics and is not, in my estimation, completely out of date. You can still buy it new from Amazon - which pleasantly surprised me (they had one left when I looked).
Jonathan Leffler
The gist of the chapter is that you don't build classes that only differ in the values returned. Your macro scheme doesn't properly (or, perhaps more charitably or accurately, sufficiently) allow for variation in the derived classes. Roughly speaking, if you can deal with it all in macros, there isn't enough difference between the classes to warrant using different classes - they should be handled by parameterization (by value) of a more generic class. That's a very quick gloss on perhaps 20 pages - though the material on each page is not all that dense (it is an open, easy to read book).
Jonathan Leffler
You're right that the macro scheme doesn't allow for variation in the derived classes. That's why I ditched the idea.
StackedCrooked