Sign in to follow this  

C++ template question

This topic is 3936 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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[i].Name == name)
            {
                // Assign the matching widget.
                pWidget = dynamic_cast<T>(m_pGUI->GetWidget(m_Registry[i].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[i].Name == name)
            {
                // Assign the matching widget.
                pWidget = dynamic_cast<T>(m_pGUI->GetWidget(m_Registry[i].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.

Share this post


Link to post
Share on other sites
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)

// becomes
void 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

Share this post


Link to post
Share on other sites
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, T &pWidget)
{
// Initialize the widget to NULL.
pWidget = NULL;
// ...
}

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites

This topic is 3936 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this