1

I have the following lines of code and compilation errors. It should be my wrong understanding of the template function, or c++ generics, or something else. Thanks in advance for pointing it out.

#include <iostream>
#include <vector>

using namespace std;

template <typename T>
T* find(vector<T> &vec, T value)
{
  vector<T>::iterator first = vec.begin();
  vector<T>::iterator last = vec.end();
  for(; first != last; first ++)
      if(*first == value)
          return first;
  return 0;
}

The compile errors in console

debug.cpp: In function ‘T* find(std::vector<T, std::allocator<_CharT> >&, T)’:
debug.cpp:9: error: expected `;' before ‘first’
debug.cpp:10: error: expected `;' before ‘last’
debug.cpp:11: error: ‘first’ was not declared in this scope
debug.cpp:11: error: ‘last’ was not declared in this scope
Zhile Zou
  • 1,949
  • 2
  • 14
  • 12

3 Answers3

5

you need to use typename vector<T>::iterator first and similar for last. Otherwise the compiler finds the declaration ambiguous, as it does not realize that vector<T>::iterator is a type. It can be a member function or something else. Technically they are called dependent typenames (as they depend on the template type T. Every time you have a dependent type use typename to avoid headaches like this. See e.g. http://pages.cs.wisc.edu/~driscoll/typename.html for more details.

vsoftco
  • 55,410
  • 12
  • 139
  • 252
  • Thanks vsoftco! the link you pasted cleared my question! "typename is mandatory before a qualified, dependent name which refers to a type (unless that name is naming a base class, or in an initialization list)" – Zhile Zou Apr 26 '14 at 04:29
2

Along with not including the typename keyword where it is needed due to type name dependency, you're also reinventing someone else's algorithm, namely std::find. Regarding what typename dependency is and why it needs resolution, this answer does a far better job of explaining it than I could.

And your also treating your container iterator type as a item-type-pointer, a practice that will work with vector, but severely limits your code if you ever want to use different containers where iterators are wrappers and not immediate pointers.

Regarding shortening the code as well as addressing the typename issue:

template <typename T>
T* find(std::vector<T> &vec, const T& value)
{
    typename std::vector<T>::iterator it = std::find(vec.begin(), vec.end(), value);
    return (it != vec.end()) ? &(*it) : nullptr;
}

Note: with C++11, this could be avoided with auto

template <typename T>
T* find(std::vector<T> &vec, const T& value)
{
    auto it = std::find(vec.begin(), vec.end(), value);
    return (it != vec.end()) ? &(*it) : nullptr;
}
Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
0

Below is a working version, but still not clear why the code in the original post is not right.

#include <iostream>
#include <vector>

using namespace std;

template <typename IteratorType, typename T>
IteratorType find(IteratorType first, IteratorType last, T &value)
{
  for(; first != last; first ++)
      if (*first == value)
          return first;
  return last;
}
Zhile Zou
  • 1,949
  • 2
  • 14
  • 12
  • no need to include the `using namespace` statement when you don't use anything from the namespace either :D which you should never do anyway but is a plus – RamblingMad Apr 26 '14 at 02:25
  • This version doesn't match your initial requirement of passing the vector rather than an iterator range (although this is a better idea) – M.M Apr 26 '14 at 02:26
  • 2
    Absolutely a better idea. One that is so good in-fact, it is included in the standard library as [**`std::find`**](http://en.cppreference.com/w/cpp/algorithm/find). – WhozCraig Apr 26 '14 at 02:37