Re: replacing structs in a vector
In article <IytF53.BAE@news.boeing.com>,
Joe Van Dyk <joe.vandyk@boeing.com> wrote:
Hm.
Here's the code I currently have:
std::vector<my_struct>::iterator i;
for (i = vec.begin(); i != vec.end(); ++i)
{
if (i->id == st.id)
{
*i = st;
break;
}
}
if (i == vec.end())
vec.push_back(st);
Would people rather see that, or something like what Daniel wrote?
Now that I know what's supposed to happen if none of the objects have
that id, let me update:
struct has_id : unary_function< my_struct, bool >
{
int id;
has_id( int id_ ): id( id_ ) { }
bool operator()( my_struct st ) const {
return st.id == id;
}
};
The above is just boilerplate code... Now let's look at the actual
location where the job is done...
vector<my_struct>::iterator i =
find_if( vec.begin(), vec.end(), has_id( st.id ) );
if ( i == vec.end() )
vec.push_back( st )
else
*i = st;
Let's compare the two examples. My call explicitly says: "find_if...
has_id" so it's easy to tell what the block of code is supposed to do.
Now I admit that you have to get used to interpreting "i == vec.end()"
as "not found" but otherwise, what I wrote is rather straight forward
and requires little further interpretation from the reader.
Now notice that no where in your loop does the subject of finding
anything ever come up. In fact, we have to read two blocks in before we
even learn that the loop will terminate before reaching the end of the
container!
Also, my example has a Cyclomatic Complexity of 2 (i.e. there are only
two paths of execution.) Yours has a complexity of 4 and 'i' must be
compared to vec.end() at least twice.
Perhaps I need to redo my design, and use a map for when my structs have
a valid id, and another container for the structs that are waiting for
their id.
That would probably be a better choice, and remove the 'id' from the
struct. Instead use it as a key to the map.