remove multiple elements from vector in a loop

I have a vector,

Code:

---

std::vector<int> myVector;

myVector[0] = 3;
myVector[1] = 4;
myVector[2] = 5;

---

and I need to remove certain elements, in this case [1] and [2]. This is happening in a loop where some things are evaluated, and under certain circumstances, and element is removed.

Code:

---

for(i=0; i<myVector.size(); i++) {
  if(myVector[i] == 4 || myVector[i] == 5) {
      myVector.erase( myVector.begin()+i );
  }
}

---

the logic is simplified here. The output after this loop is,
myVector.size() = 2
myVector[0] = 3;
myVector[1] = 5;

As I read this, when myVector[i] == 3, control falls through and nothing happens. When myInt[i] == 4, myVector.begin()+i = 0+1 and myVector[1] is erased. When myVector[i] == 5, myVector.begin()+i = 0+2 and myVector[2] is erased. Element [2] doesn't exist anymore, so the value 5 remains on the list. Obviously I'm not using the iterator properly.

The iterator position needs to be dynamic to take into account the changing size of the vector, but I can't see how to do that. This seems like a simple issue, so I presume I am missing something here. I can't just loop through and look for 4 or 5, the logic is more complex than that. I need to know how to recursively remove elements from a vector, taking into account the fact that the vector size changes.

Maybe I need to loop differently. It's almost like I need to repeat the value of i in a case where a given i was removed. This has to work in larger vectors where the elements to be removed can occur in any element. I guess I could just set i=0 after each element is removed and repeat from the top, but that doesn't seem like the best solution either.

LMHmedchem

Didn't read the whole post, so sorry if this is the wrong answer, but when removing items in a loop from an index based container, it's easiest to start at the end and work backwards.

See remove_if

see the following post on using remove_if and removing elements
while looping.

http://www.codeguru.com/forum/showthread.php?t=475058

So it would appear I could do something like,

Code:

---

int remove;
for(i=0; i<myVec.size(); i++) {
  if(myVec[i] == 4 || myVector[i] == 5) {
      remove_this = myVector[i];
      myVec.erase( remove(myVec.begin(),myVec.end(),remove_this) , myVec.end() );
  }
}

---

This seems to have the same problem as the first code I posted.
myVector.size() = 2
myVector[0] = 3;
myVector[1] = 5;

LMHmedchem

Code:

---

myVec.erase(
    std::remove_if(myVec.begin(),myVec.end(), [](int i) { return i == 4 || i == 5; }),
    myVec.end());

---

Or you can create an appropriate function or functor if your compiler does not support the lambda function I used above.

Quote:

---

Originally Posted by Lindley

Code:

---

myVec.erase(
    std::remove_if(myVec.begin(),myVec.end(), [](int i) { return i == 4 || i == 5; }),
    myVec.end());

---

Or you can create an appropriate function or functor if your compiler does not support the lambda function I used above.

---

My complier doesn't like this, I get
expected primary-expression before '[' token
expected primary-expression before ']' token
expected primary-expression before "int"

It seems as if I need a function that returns the values for what I need to remove?

LMHmedchem

geez. you mean like this? Lindley did say that the lamda expression might not work for you. In that case write a global function, static member function of a class, or a functor.

Code:

---

// write a global function
void RemoveTheseValues
{
  return(i == 4 || i == 5);
}

// or write a functor
struct RemoveTheseValues
{
  bool operator() (int i)
  {
      return(i == 4 || i == 5);
  }
};

---

See here for even more examples.
http://www.cplusplus.com/reference/algorithm/remove_if/

Well I'm going to have to go with this, for lack of being able to get anything else working,

Code:

---

for(i=0; i<myVector.size(); i++) {
  if(myVector[i] == 4 || myVector[i] == 5) {
      int remove = myVector[i];
      myVector.erase( remove(myVector.begin(), myVector.end(), remove), myVector.end() );
      i = 0;
  }
}

---

It will cost an extra iteration through the loop or so, but these vecs will never have more than two or three elements. Perhaps I should have just looked at removing elements starting from the back of the vector as GCDEF suggested.

Quote:

---

Originally Posted by kempofighter

geez. you mean like this? Lindley did say that the lamda expression might not work for you. In that case write a global function, static member function of a class, or a functor.

Code:

---

// write a global function
void RemoveTheseValues
{
  return(i == 4 || i == 5);
}

// or write a functor
struct RemoveTheseValues
{
  bool operator() (int i)
  {
      return(i == 4 || i == 5);
  }
};

---

See here for even more examples.
http://www.cplusplus.com/reference/algorithm/remove_if/

---

I tried quite a few versions of things like this, but I couldn't get any of them to compile. I just can't spend all afternoon trying to improve on something this simple. It would be nice if there was a version of the stl documentation that wasn't written in ancient Greek. Maybe I'm just a moron, but I don't find things like, "Unary predicate taking an element in the range as argument, and returning a value indicating the falsehood (with false, or a zero value) or truth (true, or non-zero) of some condition applied to it. This can either be a pointer to a function or an object whose class overloads operator()" to be very helpful if you weren't a CS major, which I wasn't. English is a nice language too.

LMHmedchem

Quote:

---

Originally Posted by LMHmedchem

So it would appear I could do something like,

Code:

---

int remove;
for(i=0; i<myVec.size(); i++) {
  if(myVec[i] == 4 || myVector[i] == 5) {
      remove_this = myVector[i];
      myVec.erase( remove(myVec.begin(),myVec.end(),remove_this) , myVec.end() );
  }
}

---

This seems to have the same problem as the first code I posted.
myVector.size() = 2
myVector[0] = 3;
myVector[1] = 5;

LMHmedchem

---

The problem is obviously that you are still decreasing the size of the vector within the loop. Don't do that. The loop will end up terminating before all of the values are tested. When the loop tests the size again, it'll be shorter.

Quote:

---

Originally Posted by LMHmedchem

Well I'm going to have to go with this, for lack of being able to get anything else working,

Code:

---

for(i=0; i<myVector.size(); i++) {
  if(myVector[i] == 4 || myVector[i] == 5) {
      int remove = myVector[i];
      myVector.erase( remove(myVector.begin(), myVector.end(), remove), myVector.end() );
      i = 0;
  }
}

---

It will cost an extra iteration through the loop or so, but I can't spend all afternoon trying to improve on something this simple. Perhaps I should have just looked at removing elements starting from the back of the vector as GCDEF suggested.

LMHmedchem

---

It really is not that hard to tweak what lindley posted. All you have to do is write the global function and substitute its name where the lamda expression went (that is the 3rd argument to remove_if). It's that simple. If you can't do that in 10 minutes then you deserve an F on your assignment.

Quote:

---

Originally Posted by kempofighter

The problem is obviously that you are still decreasing the size of the vector within the loop. Don't do that. The loop will end up terminating before all of the values are tested. When the loop tests the size again, it'll be shorter.

---

I tried storing the values to remove in another vector, but of course I ended up looping though that as well. I need one of these remove_if methods that I can pass a range to and a value to remove, but no joy so far on that.

LMHmedchem

Quote:

---

Originally Posted by LMHmedchem

Well I'm going to have to go with this, for lack of being able to get anything else working,

Code:

---

for(i=0; i<myVector.size(); i++) {
  if(myVector[i] == 4 || myVector[i] == 5) {
      int remove = myVector[i];
      myVector.erase( remove(myVector.begin(), myVector.end(), remove), myVector.end() );
      i = 0;
  }
}

---

It will cost an extra iteration through the loop or so, but these vecs will never have more than two or three elements. Perhaps I should have just looked at removing elements starting from the back of the vector as GCDEF suggested.



I tried quite a few versions of things like this, but I couldn't get any of them to compile. I just can't spend all afternoon trying to improve on something this simple. It would be nice if there was a version of the stl documentation that wasn't written in ancient Greek. Maybe I'm just a moron, but I don't find things like, "Unary predicate taking an element in the range as argument, and returning a value indicating the falsehood (with false, or a zero value) or truth (true, or non-zero) of some condition applied to it. This can either be a pointer to a function or an object whose class overloads operator()" to be very helpful if you weren't a CS major, which I wasn't. English is a nice language too.

LMHmedchem

---

Did you look at the example? You could have copy and pasted the example from that link and compiled it. Simply substitute your function names.

Quote:

---

Originally Posted by LMHmedchem

I have a vector,

Code:

---

std::vector<int> myVector;

myVector[0] = 3;
myVector[1] = 4;
myVector[2] = 5;

---

and I need to remove certain elements, in this case [1] and [2]. This is happening in a loop where some things are evaluated, and under certain circumstances, and element is removed.

---

Code:

---

#include <algorithm>
#include <vector>
#include <iostream>

bool RemoveMe(int val)
{
  return ( val == 3 || val == 4);
}

int main()
{
    std::vector<int> vect(3);
    vect[0] = 3;
    vect[1] = 4;
    vect[2] = 5;
    vect.erase(std::remove_if(vect.begin(), vect.end(), RemoveMe), vect.end());
    for (size_t i = 0; i < vect.size(); ++i )
        std::cout << vect[i] << "\n";
}

---

Is this what you're trying to code?

But in general, if you're writing convoluted loops to remove elements from a container in C++, then you need to take a step back and rethink what you're doing, as more than likely, an alogrithm or set of algorithm functions will do the job, safely and correctly.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by LMHmedchem

It would be nice if there was a version of the stl documentation that wasn't written in ancient Greek. Maybe I'm just a moron, but I don't find things like, "Unary predicate taking an element in the range as argument, and returning a value indicating the falsehood (with false, or a zero value) or truth (true, or non-zero) of some condition applied to it. This can either be a pointer to a function or an object whose class overloads operator()" to be very helpful if you weren't a CS major, which I wasn't. English is a nice language too.

---

You just have to get used to the terminology. All me to translate:

"Unary predicate (function taking one argument) taking an element in the range as argument (the function's argument type must match the type in the container), and returning a value indicating the falsehood (with false, or a zero value) or truth (true, or non-zero) of some condition applied to it (the function must return a bool indicating whether or not to remove the element in question). This can either be a pointer to a function or an object whose class overloads operator() (You can specify the function several different ways.)"

Such formal language can have a learning curve, but its preciseness is useful when you want to know exactly what a container or algorithm is going to do.

Thanks for all the replies. Sorry for the delay, I got invited to a friends house for dinner.

Quote:

---

Originally Posted by kempofighter

Did you look at the example? You could have copy and pasted the example from that link and compiled it. Simply substitute your function names.

---

The example had the data in an array instead of a vector. I couldn't tell if the function wanted pointers or iterators, or pointers to iterators, etc, and the things I tried (myVector.begin(), etc) wouldn't compile.

Quote:

---

Originally Posted by Paul McKenzie

Code:

---

#include <algorithm>
#include <vector>
#include <iostream>

bool RemoveMe(int val)
{
  return ( val == 3 || val == 4);
}

int main()
{
    std::vector<int> vect(3);
    vect[0] = 3;
    vect[1] = 4;
    vect[2] = 5;
    vect.erase(std::remove_if(vect.begin(), vect.end(), RemoveMe), vect.end());
    for (size_t i = 0; i < vect.size(); ++i )
        std::cout << vect[i] << "\n";
}

---

Is this what you're trying to code?

But in general, if you're writing convoluted loops to remove elements from a container in C++, then you need to take a step back and rethink what you're doing, as more than likely, an alogrithm or set of algorithm functions will do the job, safely and correctly.

---

This is more or less it, but I already know that I want to remove "val" before I pass it to the remove function. I tried several things like this, but I think I was misunderstanding what the function was returning. I was trying things like,

Code:

---

bool RemoveMe(int val) {
  return val;
}

---

because I thought the function was returning the value of what was to be removed, not weather or not to remove the value that was passed. I also, of course, was confused about why/how the function could be returning a bool in that context. The RemoveMe(int val) function,

Code:

---

bool RemoveMe(int val)
{
  return ( val == 3 || val == 4);
}

---

returns true if the passed value == 3 or 4? I guess I was not seeing the parentheses with the return. It may make more sense to evaluate remove/keep in a function like this instead of how I have it now. I will look at that more carefully now that I understand how this works. As usual, I revert to loops because that is what I know how to do. The loop I got working is pretty simple, especially for small structures like this, but I don't learn anything if I just stick to what I already know.

Quote:

---

Originally Posted by Lindley

You just have to get used to the terminology. All me to translate:

"Unary predicate (function taking one argument) taking an element in the range as argument (the function's argument type must match the type in the container), and returning a value indicating the falsehood (with false, or a zero value) or truth (true, or non-zero) of some condition applied to it (the function must return a bool indicating whether or not to remove the element in question). This can either be a pointer to a function or an object whose class overloads operator() (You can specify the function several different ways.)"

Such formal language can have a learning curve, but its preciseness is useful when you want to know exactly what a container or algorithm is going to do.

---

This is what I wish all the documentation looked like, so thanks for the clarification. With just the added phrases above, I can completely understand the previous posts, which I spent half the afternoon trying to implement with no luck (mostly because I didn't under stand what the function was expecting and returning). I do get frustrated at times with the C++ doc. I have three degrees, two in hard sciences, and this isn't my first day programming, but it still takes forever for me to get anywhere with some of this stuff. My non-science degree is in tech writing, so I get that things are written to a specific audience, but it seems like a little bit of plain language , like in Lindley's post, wouldn't exactly stop the world from turning on its axis. Sometimes I feel like I don't understand because I don't know the secret handshake and that you have to look at it upside down in a mirror. Mostly I get frustrated with myself about how long it takes to do things when I know what I want to do, but not exactly how to implement it. I don't expect to be an expert at something I'm, well, not an expert at. I presume that everyone here worked pretty hard to acquire the skills and knowledge they possess. I don't mind putting in the time, but some days it would be nice it something would actually just work, without having to wrestle with it all afternoon.

Chemistry, Biology, and AI certainly have plenty of specific terminology, so I won't complain too much, plus that's a bore to listen to, even where it's justified. I do think there are probably a few more people who are interested in deciphering c++ code than calculating ab inito molecular orbitals from the Schrödinger Equation, so broadening the audience just a bit couldn't hurt.

Thanks again, I will try again later toady after a bit of shut eye.

LMHmedchem

Quote:

---

This is more or less it, but I already know that I want to remove "val" before I pass it to the remove function. I tried several things like this, but I think I was misunderstanding what the function was returning. I was trying things like,

Code:

---

bool RemoveMe(int val) {
  return val;
}

---

---

c'mon, think about that function for a sec! it does NOTHING helpful :)

Quote:

---

Originally Posted by LMHmedchem

Thanks for all the replies. Sorry for the delay, I got invited to a friends house for dinner.

The example had the data in an array instead of a vector. I couldn't tell if the function wanted pointers or iterators, or pointers to iterators, etc, and the things I tried (myVector.begin(), etc) wouldn't compile.

---

First, an algorithm requires iterators, but pointers are iterators. So the algorithm functions work with arrays as well as any other containers that have the iterator concept.

The remove_if() algorithm works perfectly fine with regular arrays, as remove_if() doesn't remove anything -- it only arranges the sequence so that the "bad" items are placed at the end of the sequence.

Code:

---

int array[] = (3, 4, 5};
//...
int* removed_values_start_here = remove_if(array, array + 3, RemoveMe);

---

You will see that remove_if rearranges the items so that the items to remove are at the end, and returns an iterator to the first removed item.

Quote:

---

This is more or less it, but I already know that I want to remove "val" before I pass it to the remove function. I tried several things like this, but I think I was misunderstanding what the function was returning.

---

I think you're misunderstanding what an "iterator" is.

An iterator is a concept -- that concept allows you to use operator ++ to go to the next item in the sequence, operator -- to go to the previous item in the sequence, operator += to jump to any item in the seqence, similarly with operator -=, etc. Since pointers have all of these properties, pointers qualify as iterators.

There are some iterators that do not support all of the operations I listed, but I won't get into that. But the general idea is that an iterator allow you to use pointer syntax on a sequence. Since C++ has the ability to overload operators, any class that has overloaded ++, --, +=, etc. and whose purpose is to go to a position in a sequence is called an iterator.

So for a regular array, what is returned from remove_if() is an iterator that is compatible with the iterators you provided as the parameter -- that iterator is a pointer to int (in my example).

Quote:

---

The loop I got working is pretty simple, especially for small structures like this

---

Your loop was buggy and didn't work, even though the code compiled. You even started a thread on that very subject -- your removal code didn't work. So it wasn't "simple", as it took you at least 2 or 3 renditions to get it right without using algorithms, with code that initially compiled OK with no issues, but had bugs (this is an important aspect).

Every time I get code that removes item from a container and it doesn't use an algorithm, I always need to look at it 3 or 4 times to make sure it works, and then that isn't enough. I am sure that some here, if not everyone else looking at your code, had to step through it mentally before figuring out what was wrong -- maybe some had to run it to see what was wrong. When removal code is written using an algorithm such as remove_if() with a function object or function pointer, I know right away it works without stepping through the code.

The beauty with algorithms is that they are never wrong, given the correct arguments -- they do exactly as written. The code when using algorithms will refuse to compile if you give it the wrong argument types (so there is no way you can get the algorithm "wrong", as with the case with coding your own for() loops). Once you give the algorithm the right arguments and you're using the correct algorithm, the algorithms work every time. No need to debug, no need to worry that you typed in something wrong or your loop is inefficient or wrong, etc.

Regards,

Paul McKenzie

Just a note on your loop (I don't know if this is your final version or not):

Code:

---

for(i=0; i<myVector.size(); i++)
{
  if(myVector[i] == 4 || myVector[i] == 5) {
      int remove = myVector[i];
      myVector.erase( remove(myVector.begin(), myVector.end(), remove), myVector.end() );
      i = 0;
  }
}

---

1. The number of times thru the loop is basically the number of
elements minus the number of fours and fives. So for a vector
with 10000 elements, you would be doing un-needed loops.

2. If you are using remove like this, why loop at all ?

Code:

---

myVector.erase( remove(myVector.begin(), myVector.end(), 4), myVector.end() );
myVector.erase( remove(myVector.begin(), myVector.end(), 5), myVector.end() );

---

Bit note: remove_if will end up being more efficient.

Quote:

---

Originally Posted by Amleto

c'mon, think about that function for a sec! it does NOTHING helpful :)

---

Yes, that is a singularly un-useful bit of code. I thought the function was returning the position for what you wanted to remove, which I already knew, so I tried to make it a dummy that would just return what I passed. Of course, that didn't work since it was a bool function and remove_if didn't want an int returned anyway.

Quote:

---

Originally Posted by Philip Nicoletti

Just a note on your loop (I don't know if this is your final version or not):

---

This is what I got working at one point,

Code:

---

for(i=0; i<myVector.size(); i++) {
  if(// passes test) {
      myVector.erase( myVector.begin()+i );
      i = 0;
  }
}

---

This compiles and runs with correct output for the examples I tested on. Paul McKenzie's point is well taken that it can be difficult to judge when a loop construct may get you in trouble, since you have to walk your way through all the permutations of where it may go.

After looking at this, my original post didn't correctly outline what I was trying to do. As you can see above, the loop is also used to evaluate which elements need to be removed. Removal is also done in the loop, since the position of the element to be remove is in scope as the loop iterator. In the current setup, I basically need to pass the value in the loop iterator (which isn't an iterator) to the function that will implement the removal. The problem trying to implement the suggestions here is that I don't know the element values or position of elements to be removed until I do some testing. I didn't make that clear in my first post, so that is my fault. I need to pass the position for the element to be removed as a variable. That makes this kind of thing problematic, since the value of elements to be removed could be anything, not just 3 or 4.

Code:

---

bool RemoveMe(int val)
{
  return ( val == 3 || val == 4);
}

---

If I understand Lindley's explanation of unary predicate, I can't pass the bool function any additional information about what to remove in a given case, so it seems like these functions need to be hard coded case by case.

Let me back up and more clearly define the overall process. There are two data structures involved here. One contains node numbers (myVector), and the second is an array holding a type number associated with each node. The node numbers can be anything 1-300 and occur in any order. There can be any number of them (in theory), but 0-3 would be the most likely cases.

Code:

---

std::vector<int> myVector;

myVector[0] = 3;
myVector[1] = 4;
myVector[2] = 5;

int mylookup[5];

mylookup[0] = 99999;
mylookup[1] = 24;
mylookup[2] = 16;
mylookup[3] = 30;
mylookup[4] = 28;
mylookup[5] = 28;

---

The array is an old style lookup, where information for node 3 is stored in position 3, etc. The nodes are numbered from 1-n, so position 0 is a placeholder. The myVector container is a list of nodes that have been identified as a certain case. I need to process this list and get rid of anything that is type 28. The logic looks like,

Code:

---

int current_node;
for(i=0; i<myVector.size(); i++) {
  current_node = myVector[i];
  if(mylookup[current_node] == 28 ) {
      myVector.erase( myVector.begin()+i );
      i = 0;
  }
}

---

so I have to retrieve the node number from myVector, look up the type in myLookup, and if the type matches, i holds the position in myVector that needs to be removed. This works for the first item that needs to be removed, but after that, i is out of registration with the elements in the vector, unless you start over. At most, myVector can never actually have more than 50 nodes, and I would be shocked to ever see more then 3 or 4, so well formedness and stability are far more important then performance (alm ost anything will perform adequately for a 3 member list).

Quote:

---

Originally Posted by Philip Nicoletti

2. If you are using remove like this, why loop at all ?

---

I hope the above makes the more clear. The elements need to be identified in a loop, with the current data structure, but don't need to be removed in the loop, per se. One thing I guess I could do is to move the testing into the bool function,

Code:

---

bool RemoveMe(int val) {
  if(mylookup[val] == 28) {
      return true;
  } else {
      return false;
  }
}

---

but I don't know if that's the best solution or not. It seems odd that I would have to code a separate bool() for every case I need to remove.

Quote:

---

Originally Posted by Paul McKenzie

The beauty with algorithms is that they are never wrong, given the correct arguments -- they do exactly as written.

---

The availability of algorithms is one of the main reasons I went to using c++ in the first place. Coding your own requires so much testing to make sure they are bug-free that it takes forever to get through even simple tasks that require such a thing. I still find bugs in code that is 30+ years old, so it's pretty tough to convince yourself that your code is good. I have much more confidence in code that has been checked by a very large number of programmers over a long time.

LMHmedchem

Quote:

---

Originally Posted by LMHmedchem

so it seems like these functions need to be hard coded case by case.

---

That is what function objects are for. The function object has state, unlike a normal function.

Code:

---

#include <set>
struct RemoveMe
{
    public:
        bool operator() (int n) const
        {
            return ItemsToRemove.find(n) != ItemsToRemove.end();
        }

    void AddItemToRemove(int item)
    { 
          ItemsToRemove.insert(item);
    }

    private:
        std::set<int> ItemsToRemove;
};
//...
RemoveMe RM;
RM.AddItemToRemove( 3 );
RM.AddItemToRemove( 4 );
//...
remove_if(vect.begin(), vect.end(), RM);

---

So if you need to remove 10 items, you don't rewrite the code -- all you need to do is call AddItemToRemove 10 times with the different items. The RemoveMe is called a functor or function object.

Now, do you see the potential for this? That class or struct can be anything you want it to be. It can have constructors, destructors, virtual functions, can be derived from or be a base class, anything. The overloaded operator() is what does the magic. In other words, the entire logic of what gets deleted goes in that class. My job was just to say "if you see this case, remove it". In your example, not only do you have to state what gets deleted, but you've taken on the burden of writing the loop with the deletion logic, which could be faulty.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by LMHmedchem

Let me back up and more clearly define the overall process. There are two data structures involved here. One contains node numbers (myVector), and the second is an array holding a type number associated with each node. The node numbers can be anything 1-300 and occur in any order. There can be any number of them (in theory), but 0-3 would be the most likely cases.

Code:

---

std::vector<int> myVector;

myVector[0] = 3;
myVector[1] = 4;
myVector[2] = 5;

int mylookup[5];

mylookup[0] = 99999;
mylookup[1] = 24;
mylookup[2] = 16;
mylookup[3] = 30;
mylookup[4] = 28;
mylookup[5] = 28;

---

The array is an old style lookup, where information for node 3 is stored in position 3, etc. The nodes are numbered from 1-n, so position 0 is a placeholder. The myVector container is a list of nodes that have been identified as a certain case. I need to process this list and get rid of anything that is type 28. The logic looks like,

Code:

---

int current_node;
for(i=0; i<myVector.size(); i++) {
  current_node = myVector[i];
  if(mylookup[current_node] == 28 ) {
      myVector.erase( myVector.begin()+i );
      i = 0;
  }
}

---

---

Code:

---

#include <set>
#include <vector>
#include <algorithm>

struct LookupEraser
{
    private:
            int *m_pLookup;
            int numberToCheck;

    public:
          LookupEraser(int *mylookup, int numToCheck) : m_pLookup(mylookup), numberToCheck(numToCheck)  {}

    bool operator()(int index) const
    {  return (m_pLookup[index] == numberToCheck); }
};

int main()
{
    std::vector<int> myVector(3);
    myVector[0] = 3;
    myVector[1] = 4;
    myVector[2] = 5;

    int myLookup[] = { 99999, 24, 16, 30, 28, 28 };
    std::vector<int> vect;
    myVector.erase(std::remove_if(myVector.begin(), myVector.end(), LookupEraser(myLookup, 28)), myVector.end());
}

---

That code above, from what I see, does exactly what your loop did. There are absolutely no hard-coded loops. Note the functor in this example, usage of remove_if(), and the guarantee that it works correctly without me even running it. The functor has as a constructor the array you're searching in, plus the number to erase as the second parameter to the constructor. I can even expand on the functor by using a set, similar to my previous post, and check for multiple numbers, not just one number to erase. Again, flexibility, something that a regular pointer to function doesn't really have.

If you want to know how to read what I posted above, the way you read that code is to first look at operator() for the functor. That is most of the work -- if that function returns true, given the argument passed to it, then that item will be "removed" by placing it at the end of the sequence, otherwise it is not removed.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

That code above, from what I see, does exactly what your loop did. There are absolutely no hard-coded loops. Note the functor in this example, usage of remove_if(), and the guarantee that it works correctly without me even running it. The functor has as a constructor the array you're searching in, plus the number to erase as the second parameter to the constructor. I can even expand on the functor by using a set, similar to my previous post, and check for multiple numbers, not just one number to erase. Again, flexibility, something that a regular pointer to function doesn't really have.

If you want to know how to read what I posted above, the way you read that code is to first look at operator() for the functor. That is most of the work -- if that function returns true, given the argument passed to it, then that item will be "removed" by placing it at the end of the sequence, otherwise it is not removed.

---

Sorry, I'm just running a bit behind you. That post was refering to the simple bool function from earlier. I have your post #21 code working now and have read #22 as well.

Quote:

---

Originally Posted by Paul McKenzie

That is what function objects are for. The function object has state, unlike a normal function.

So if you need to remove 10 items, you don't rewrite the code -- all you need to do is call AddItemToRemove 10 times with the different items. The RemoveMe is called a functor or function object.

Now, do you see the potential for this? That class or struct can be anything you want it to be. It can have constructors, destructors, virtual functions, can be derived from or be a base class, anything. The overloaded operator() is what does the magic. In other words, the entire logic of what gets deleted goes in that class. My job was just to say "if you see this case, remove it". In your example, not only do you have to state what gets deleted, but you've taken on the burden of writing the loop with the deletion logic, which could be faulty.

---

Yes, this is something new to me. I set it up like this,

Code:

---


if(myVector.size() != 0) {
  RemoveMe RM;
  for(i=0; i<myVector.size(); i++) {
      current_node = myVector[i];
      if(mylookup[current_node] == 28 ) {
        RM.AddItemToRemove( current_node );
      }
  }
  myVector.erase( remove_if(myVector.begin(), myVector.end(), RM), myVector.end() );
}

---

Do I assume correctly that since RM is defined inside of the if {}, it will go out of scope when the if is complete, or do I have to explicitly destruct it? This is in case I create another instance of RM later in the code with a different list of elements to remove.

As I read this, erase is going to remove elements stating with the iterator returned by remove_if and ending with the end of the container. Remove_if is sorting such that the elements to be removed are last and returns an iterator to the first element to be removed.

There are some differences between your post #21 and #22 code. In #21, I have already evaluated what to remove, and am collecting a list of element values that get passed to erase, through remove_if. In #22, I am passing the lookup value to the LookupEraser() functor (if I am using that term correctly). I am not sure which method is best for this case. Since I may need to evaluate ||, the first method may be better since it would be easier to do things like
if(mylookup[current_node] == 28 || mylookup[current_node] == 35)

This version from post 21 is definitely working, so thanks a heap for the help.

LMHmedchem

Quote:

---

Originally Posted by LMHmedchem

Sorry, I'm just running a bit behind you. That post was refering to the simple bool function from earlier. I have your post #21 code working now and have read #22 as well.

---

The earlier version was a simple way to show you what a functor is. The code in #22 is a complete rewrite of that entire loop you had posted.

Code:

---

if(myVector.size() != 0) {
  RemoveMe RM;
  for(i=0; i<myVector.size(); i++) {
//...

---

There is no need to write an outer loop if using #22.

Quote:

---

Do I assume correctly that since RM is defined inside of the if {}, it will go out of scope when the if is complete, or do I have to explicitly destruct it? This is in case I create another instance of RM later in the code with a different list of elements to remove.

---

Yes, the rules of scoping do not change. You create an object in a scope, when that scope is exited, that object's destructor is called. There is nothing special about RM.

Quote:

---

As I read this, erase is going to remove elements stating with the iterator returned by remove_if and ending with the end of the container. Remove_if is sorting such that the elements to be removed are last and returns an iterator to the first element to be removed.

---

Yes, that's what's happening. You could also break it up into two statements to see what is happening:

Code:

---

std::vector<int>::iterator first_removed = remove_if(vect.begin(), vect.end(), some_function);

vect.erase( first_removed, vect.end());

---

Quote:

---

There are some differences between your post #21 and #22 code. In #21, I have already evaluated what to remove, and am collecting a list of element values that get passed to erase, through remove_if. In #22, I am passing the lookup value to the LookupEraser() functor (if I am using that term correctly). I am not sure which method is best for this case. Since I may need to evaluate ||, the first method may be better since it would be easier to do things like
if(mylookup[current_node] == 28 || mylookup[current_node] == 35)

---

Just change the second example to use a set, just like the first example.

Code:

---

#include <set>
struct LookupEraser
{
    private:
            int *m_pLookup;
            std::set<int> numberToCheck;

    public:
          LookupEraser(int *mylookup) : m_pLookup(mylookup)  {}

    bool operator()(int index) const
    {  return  numberToCheck.find(m_pLookup[index]) != numberToCheck.end(); }

    void AddLookupItem(int item) { numberToCheck.insert(item); }
};
//..
int myArray[5];
//...
LookupEraser LE(myArray);
LE.AddLookupItem( 28 );
LE.AddLookupItem( 35 );
//...
vect.erase(remove_if( vect.begin(), vect.end(), LE), vect.end());

---

No loops at all.

Also, this works even if you add no numbers to LE to search for, or if vect is empty. Again, that's the good thing about this -- if you happen to add no data to LE (for some reason), or if vect has no items in it, and you call vect.erase..., nothing happens -- no "runtime exception", no "null pointer", no undefined behaviour, etc. As long as myArray is a valid int array that is the correct size, and is in scope, the function can be used.

And to that, to be safer, why is myArray an array? Why is it not a vector also? If it were a vector, then this whole thing would be practically bulletproof and would basically work the first time it's coded. With an array, you still have the chance of an out-of-bounds access.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

And to that, to be safer, why is myArray an array? Why is it not a vector also? If it were a vector, then this whole thing would be practically bulletproof and would basically work the first time it's coded. With an array, you still have the chance of an out-of-bounds access.

---

This was originally fortran code, which I ported over to cpp to add some functionality that was easier to code in c, or anything but g77. In short, it's an array in cpp because that's what it was in Fortran. I actually have a cpp object version of that same data. It's an int in that case and would be accessed something like,

node_list[current_node].nodeType

where node list is a vector of objects. Each object holds data about the node, like type. I haven't converted all of the code to use the new data structure because there is allot of it, and it performs well as it is. The old style array lookup performs very well when the array is not large and I tend to put my time into adding new functionality as opposed to re-working older code that seems to function properly. It's on my list, but I haven't got to it yet. I need to more throughly test the class that stores the node data to make sure everything in it is correct and matches the data in the arrays. That will take some time to do.

LMHmedchem