tags:

views:

311

answers:

10

I need to compare one given value with a retrieved values. I do this several times in the code. I am not satisfied with how it looks and I am seeking for a some sort of an util function. Anyone wrote one?

Number of values I am comparing with is known at the compile time.

Update: I'd like to get rid of containers as I know exact amount of values ( often not more then 3 ) I want to compare with. And it is not so convenient to put items to the container every time.
I don't love if neither because it is not obvious as "find".

#include <algorithm>
#include <string>
#include <vector>

std::string getValue1()
{
    return "test";
}

std::string getValue2()
{
    return "the";
}

std::string getValue3()
{
    return "world";
}

int main()
{
    const std::string value = "the";

    // simple if
    if ( value == getValue1() ||
         value == getValue2() ||
         value == getValue3() )
        return 1;

    // using collections like vector, set
    std::vector<std::string> values;
    values.push_back( getValue1() );
    values.push_back( getValue2() );
    values.push_back( getValue3() );
    if ( values.end() != std::find( values.begin(), values.end(), value ) )
        return 1;

    // third option I'd use instead
    //

    return 0;
}
A: 

std::find_first_of.

Of course, if you only care about finding a single value, you can create your own wrapper around std::find.

MSN
I don't want to fill a vector since I now exact amount of values I am going to compare with.
Mykola Golubyev
You don't need to use a vector for std::find. You can store the values in an array and pass that down as well.
MSN
And why not use a vector? It's quick and easy to use, and will work with larger numbers.
David Thornley
A: 

I would recommend your method 2, using a std::vector or other container and looking for membership. Since the order of elements that you're checking against is probably not relevant, you may want to use a std::set or std::map. If you have very many items in your set of values, then using a set or a map will be a faster, while if you have only a few a vector may be faster.

The advantage of any of these approaches is that you can then store the set/map somewhere common, and avoid having to build the set of matching responses every time.

JSBangs
A: 

i like the collections approach, maybe use a hash_set instead of a vector. store the values in a property file and have a method to populat the hash_set from the file, and another one to return a boolean if the value is in the hash_set. then you can get down to one 2 lines in the main code.

John Ellinwood
+5  A: 

If the values you're looking for are Comparable with operator< (like ints, float and std::strings), then it's faster to use an std::set to put the values there and then check set.find(value) == set.end(). This is because the set will store the values with a certain order that allows for faster lookups. Using an hash table will be even faster. However, for less than 50 values or so you might not notice any difference :) So my rule of thumb would be:

  • Less then 5 items: if with multiple ||

  • 5 or more: put in a set or hash table

João da Silva
I'd like third option as I don't like first two.
Mykola Golubyev
Maybe you prefer the python way: value = 123; value in (getValue1(), getValue2(), getValue3()). This is not possible in C++ though.
João da Silva
+3  A: 

Hi,

you can write a set of template functions which will help you through with this, for example:

template <typename T>
bool InSet(const T & item, const T & i1, const T & i2) {
  return item==i1 || item==i2;
}

template <typename T>
bool InSet(const T & item, const T & i1, const T & i2, const T & i3) {
  return item==i1 || item==i2 || item==i3;
}

Note that you can make InSet to work like it took a variable number of arguments by creating multiple templates with different number of arguments.

And then:

int i;
if (InSet(i, 3, 4, 5)) { ... }
string s;
if (InSet(s, "foobar", "zap", "garblex")) { ... }

etc.

antti.huima
Thought about this one. What about something like InSet(i)(3,4,5)?
Mykola Golubyev
Yes, you could also let InSet(i) produce a temporary object which has an operator(...) but I guess that wouldn't change too much...
antti.huima
See my answer below for how you can implement this.
Eclipse
+1  A: 

You don't need a std::set or a std::vector. Just use std::set_intersection()...

Code is best...

#include <set>
#include <iostream>
#include <iterator>
using namespace std;

#define COUNT(TYPE,ARRAY)  ( sizeof(ARRAY) / sizeof(TYPE) )

inline bool CaseInsensitiveCompare (const char * a, const char * b)
  {  return strcasecmp( a, b ) < 0;  }

int  main()
{
  const char * setA[] = { "the", "world", "is", "flat" };
  const char * setB[] = { "the", "empty", "set", "is", "boring" };

  stable_sort( setA,  setA + COUNT( const char *, setA ),
               CaseInsensitiveCompare );

  stable_sort( setB,  setB + COUNT( const char *, setB ),
               CaseInsensitiveCompare );

  cout << "Intersection of sets:  ";
  set_intersection( setA, setA + COUNT( const char *, setA ),
                    setB, setB + COUNT( const char *, setB ),
                    ostream_iterator<const char *>(cout, " "),
                    CaseInsensitiveCompare );
  cout << endl << endl;
}


Or perhaps, given your 1-N lookup problem:
(Note: Use *binary_search()* AFTER sorting!)

if ( binary_search( setA, setA + COUNT( const char *, setA ),
         "is", CaseInsensitiveCompare ) )
  ...

if ( binary_search( setA, setA + COUNT( const char *, setA ),
         "set", CaseInsensitiveCompare ) )
  ...
Mr.Ree
I didn't get why should I do all this tricks to simply check is one value equals to others?
Mykola Golubyev
Nothing says you can't say if a==b || a ==c || ... || a ==z. You wanted an alternative. Perhaps a more efficient one that's O(N*logN) up front + O(logN) each time rather than O(N^2) each time. But hey, it's your call... I *like* setting things up so I can easily change the hardcoded values.
Mr.Ree
I don't need efficient since there is no more then 5 elements to compare with.
Mykola Golubyev
A: 

It depends on the source of the retrieved values, if you're reading in from a file or stream then you'd do something different but if your source is a series of functions then the following is a different way to do it, not perfect but may suit your needs:

const int count = 3;
std::string value = "world";
boost::function<std::string(void)> funcArray[count];
funcArray[0] = &getValue1;
funcArray[1] = &getValue2;
funcArray[2] = &getValue3;

for( int i = 0; i < count; ++i )
{
 if( funcArray[i]() == value )
  return 1;
}

If you know which functions are the source (as well as the count of objects) I expect you could assemble the function pointer array using the preprocessor.

Patrick
I find only for simpler ( shorter and cleaner ) solution that if/for/collections.
Mykola Golubyev
This is an overkill...
The guys asking for different ways of doing it. This is different...
Patrick
its usefulness depends on the exact problem domain
Patrick
+1  A: 

For your request to do

if (InSet(value)(GetValue1(), GetValue2(), GetValue3()))
{
   // Do something here...
}

Try this:

template <typename T>
class InSetHelper
{
     const T &Value;
     void operator=(const InSetHelper &);
public:
     InSetHelper(const T &value) : Value(value) {}

     template<class Other, class Another>
     bool operator()(const Other &value1, const Another &value2) const
     {
         return Value == value1 || Value == value2;
     }
     template<class Other, class Another, class AThird>
     bool operator()(const Other &value1, const Another &value2, const AThird &value3) const
     {
         return Value == value1 || Value == value2 || Value == value3;
     }
};

template <typename T> 
InSetHelper<T> InSet(const T &value) { return InSetHelper<T>(value); }

This syntax might be more clear though:

if (MakeSet(GetValue1(), GetValue2(), GetValue3()).Contains(value))
{
   // Do something here...
}

template <typename T, typename U, typename V>
class Set3
{
    const T& V1;
    const U& V2;
    const V& V3;
    void operator=(const Set3 &);
public:
    Set3(const T &v1, const U &v2, const V &v3) : V1(v1), V2(v2), V3(v3) {}

    template <typename W>
    bool Contains(const W &v) const
    {
        return V1 == v || V2 == v || V3 == v;
    }
};

template <typename T, typename U>
class Set2 
{ 
     // as above 
};

template <typename T, typename U, typename V>
Set3<T, U, V> MakeSet(const T &v1, const U &v2, const V &v3)
{
    return Set3<T, U, V>(v1, v2, v3);
}

template <typename T, typename U>
Set3<T, U> MakeSet(const T &v1, const U &v23)
{
    return Set3<T, U, V>(v1, v2);
}

If those values are really part of a tree or a linked list, then you have your set/container already, and your best bet is to just use some recursion:

parent.ThisOrDescendantHasValue(value);

You'd just add this to whatever class parent and child belong to:

class Node
{
public: 
    Value GetValue();
    Node *GetChild();
    bool ThisOrDescendantHasValue(const Value &value)
    {
        return GetValue() == value
           || (GetChild() && GetChild->ThisOrDescendantHasValue(value));
    }
};
Eclipse
Josh, would you use this? I simply asked the Question itself because I doubt about writing such thing. I want use this and similar. Is it normal practice?
Mykola Golubyev
I've never really come into the situation where I had that much comparing to do that it'd be worth it. It's not really common practice, but it's not unheard of either. I might lean more towards something like this: if(MakeSet(v1, v2, v3).Contains(v)) See coming edit.
Eclipse
Things like this are trade-offs. You make things a little more complicated behind the scenes, but if your usage is clear enough, and you use it often enough then it may be worth it.
Eclipse
+1  A: 

Which changes more, the 'value' or the values returned by 'getValueX()'? You can insert everything into a hash_map/map and then do a search that way, as you've suggested with the containers.

wchung
A: 

How about using a boost::array (or std::tr1::array) and creating a simple function like this:

template <typename ValueType, size_t arraySize>
bool contains(const boost::array<ValueType, arraySize>& arr, const ValueType& val)
{
    return std::find(arr.begin(), arr.end(), val)!=arr.end();
}

You could then reuse that pretty easily:

#include <string>
#include <iostream>

#include <boost\array.hpp>

template <typename ValueType, size_t arraySize>
bool contains(const boost::array<ValueType, arraySize>& arr, const ValueType& val)
{
    return std::find(arr.begin(), arr.end(), val)!=arr.end();
}

int _tmain(int argc, _TCHAR* argv[])
{
    boost::array<std::string, 3> arr = {"HI", "there", "world"};

    std::cout << std::boolalpha 
        << "arr contains HI: " << contains(arr, std::string("HI")) << std::endl
        << "arr contains blag: " << contains(arr, std::string("blag") ) << std::endl
        << "arr contains there: " << contains(arr, std::string("there") ) << std::endl;

    return 0;
}

Edit: So boost is out. It's pretty easy to adapt this to a regular array:

template <typename ValueType, size_t arraySize>
bool contains(ValueType (&arr)[arraySize], const ValueType& val)
{
    return std::find(&arr[0], &arr[arraySize], val)!=&arr[arraySize];
}

int _tmain(int argc, _TCHAR* argv[])
{
    std::string arr[3] = {"HI", "there", "world"};

    std::cout << std::boolalpha << "arr contains HI: " << contains(arr, std::string("HI")) << std::endl
        << "arr contains blag: " << contains(arr, std::string("blag") ) << std::endl
        << "arr contains there: " << contains(arr, std::string("there") ) << std::endl;

    return 0;
}
zdan
neither boost nor tr1 at the project.
Mykola Golubyev
I'm not a big fan of boost but the answer here is perfectly respectable and doesn't deserve a down-vote.
Harold Bamford
It was pretty easy to generate a non-boost version, which I've added.
zdan