🔒 Locked

C++ template question

Started by Mantear Mar 7, 2007 at 12:42 PM 4 replies 1.1k views

Original Post

Mar 7, 2007 at 12:42 PM

Greetings! I'm trying to add a templated member function to a class of mine, but it's not working the way I expected. Here's the code:

class GUI_Register
{
public:
    // Constructors and Destructors
    GUI_Register(GUI* pGUI){ m_pGUI = pGUI; }
    ~GUI_Register(){}

    template <typename T>
    void GetWidget(const std::string& name, T pWidget)
    {
        // Initialize the widget to NULL.
        pWidget = NULL;
        
        // Search the registry for a widget with a matching name.
        for (unsigned int i = 0; i < m_Registry.size(); i++)
        {
            if (m_Registry.Name == name)
            {
                // Assign the matching widget.
                pWidget = dynamic_cast<T>(m_pGUI->GetWidget(m_Registry.WidgetLocation));
                break;
            }
        }
    }

private:
    GUI* m_pGUI;
};

I'm trying to use the above code in the following manner:

GUI_ListBox* pListBox = NULL;
m_pRegister->GetWidget("List Box 1", pListBox);
// Check for NULL to make sure the widget found is of the correct type, etc.

When I step through the template code, the pWidget gets assigned properly. However, as soon as the function returns, pListBox is still NULL. Now, if I modified the code to:

    template <typename T>
    T GetWidget(const std::string& name, T pWidget)
    {
        // Initialize the widget to NULL.
        pWidget = NULL;
        
        // Search the registry for a widget with a matching name.
        for (unsigned int i = 0; i < m_Registry.size(); i++)
        {
            if (m_Registry.Name == name)
            {
                // Assign the matching widget.
                pWidget = dynamic_cast<T>(m_pGUI->GetWidget(m_Registry.WidgetLocation));
                break;
            }
        }

        return pWidget;
    }

With the usage of:

pListBox = m_pRegister->GetWidget("List Box 1", pListBox);

That works. However, it's not clean. Can someone point out what's wrong here? Using MSVC 2005 Express.

jouley

Mar 7, 2007 at 12:51 PM

You're passing by value, so a temporary widget is created on the stack, assigned, and promptly thrown away when it goes out of scope (once the function returns). Passing by reference should do the trick:

void GetWidget(const std::string& name, T pWidget)// becomesvoid GetWidget(const std::string& name, T& pWidget)

Unless I've missed something, this phenomenon isn't restricted to templates - it applies to all data types.

-jouley

rip-off

Mod

445

rip-off

Mar 7, 2007 at 12:51 PM

What do you expect this code to do:

void foo( int *ptr ){    ptr = new int(42);}void bar(){     int *pointer = NULL;     foo(pointer);     // pointer has what value?}

The change in the value of ptr isn't seen in the calling code because the pointer is passed as a copy. To get the correct usage from your code, use a T reference:

template <typename T>    void GetWidget(const std::string& name, <font color = "red">T &</font>pWidget)    {        // Initialize the widget to NULL.        pWidget = NULL;        // ...    }

osmanb

Mar 7, 2007 at 12:52 PM

This is a simple issue of pass-by-value, rather than pass-by-reference. Stripping away all the template and pointer issues (that just complicate what's happening), what you're really written is this:

void foo(int x){  x = 42;}int main(){  int y = 13;  foo( y );  cout << y << endl;}

If you've studied how parameter passing works, then it's obvious that this is going to print '13', not '42'. Your example is a complex version of the same problem. You can fix it by making the second argument a 'T&' instead of a 'T', but I think returning the pointer is a better solution anyway. And if all the possible types derive from a common base class (which must be true, given that you're using dynamic_cast), then I'd just make the function return the base class. This gets rid of the need for the template, and the second function argument. It does move some of the checking logic to the caller, though, if you expect an object a certain name to be a certain type.

johdex

Mar 7, 2007 at 12:56 PM

The problem has nothing to do with templates. Does the problem become obvious if you look at your instantiated code?

    void GetWidget(const std::string& name, GUI_ListBox* pWidget)    {        // Initialize the widget to NULL.        pWidget = NULL;

The pointer is passed by value. That allows to modify the object to which the pointer points, but not the pointer itself. If you want to change the pointer itself, you will need a pointer to a pointer, or a reference to a pointer.

Zahlman

257

Zahlman

Mar 7, 2007 at 6:38 PM

The modified example does work, and in fact is what I would recommend, with a bit of clean up. Reference parameters (in general, whether you get the effect via pointers, references or some other way) are a bit smelly; if you have an available return value, you should use it.

So all we need to do is get rid of that second parameter, since it's no longer needed. (This also cleans up the calling code, and eliminates the defensive-coding question of who's responsible for initializing the pointer.) All it does right now is allow the template to be inferred; but we could instead just specify the template type explicitly, and it would be less work than setting up the "out" variable.

Meanwhile, since we are always returning a pointer, we might as well modify the function signature to reflect that - let the template type be the pointed-to type, rather than the returned type. This again simplifies things for the caller.

Oh, and there are much nicer ways to use standard library containers. :)

// By using standard library algorithms, we can avoid explicitly looping:struct matching_name : public binary_function<RegistryEntry, std::string, bool> {	bool operator()(const RegistryEntry& x, const std::string& y) {		return x.Name == y; 	}};template <typename T>T* GetWidget(const std::string& name) {        // Search the registry for a widget with a matching name.	list<int>::iterator entry = find_if(m_Registry.begin(),	                                    m_Registry.end(),	                                    bind2nd(matching_name(), name));	return (entry == m_Registry.end()) 	       ? 0	       : dynamic_cast<T*>(m_pGUI->GetWidget(entry->WidgetLocation));}// We use it like:GUI_ListBox* pListBox = m_pRegister->GetWidget<GUI_ListBox>("List Box 1");

By the way, though, all these pointers in your design make me nervous. Are these levels of indirection really necessary? Can you show why?

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.