I have a class called PatchCollection. It is in charge of creating a bunch of Patch objects. It stores them in a member variable of type std::set. Now I want to loop over all of the patches from outside the class. That is:

 PatchCollection Patches;
 void DoSomething()
   // Loop over all patches in Patches

What is the "best" way to go about this? I've though of:

1) Of course I could make a std::set<Patch>& GetPatches() function, but then I need to know that the patches are stored in a std::set, which seems to violate some principles of OO (i.e. the calling function should not care about the internal details).

2) I could make a typedef of std::set<Patch> ContainerType. Then I could just use PatchCollection::ContainerType::iterator in the calling function. This is better than (1), but it still indicates an STL container is being used.

3) I could "reimplement" the std::set function - making a PatchCollection::GetIterator(), PatchCollection::NextItem(), etc - but this seems like a lot of duplication.

Any thoughts? What would you do :) ?


You can implement a PatchCollection::iterator and have PatchCollection provide a begin and end function. This would be a good solution because it also makes your collection reusable to STL.

Or you can simply have PatchCollection implement begin(), end(), and getNextItem() function, but this adds more coupling to the class, which should be decoupled into an iterator.

Or even simply, provide a function in PatchCollection :

template<typename Func_1>
void onEachItem(const Func_1 func){
  for each item i, in this set
    call func( i );

Any plans on needing the class to be "thread-safe" ?

Also, @firstPerson, most times you need to pass an instance of a class as a separate param to use with member functions of a class as suggested by his "OO" stuff. I bet there's a boost fix for this additional trouble worth investigating if you should go that route.

//something like
template<typename Func_1, typename Instance_1>
void onEachItem(const Func_1 func, const Instance_1 inst){
  for each item i, in this set
    inst->func( i );

Doesn't it depend on what the anticipated usage of your collection is?

In the simplest case, you might find that the usage of the collection can be entirely satisfied by simply doing typedef std::set< Patch > PatchCollection . This has the advantage of giving good STL compatibility and needs no duplication, since you get all the accessors that you need from the STL container. Using this method, you can obviously find a specific Patch instance and also iterate over all the Patch objects in the set. If you need to perform specific tasks on each member of the collection, you can then make a helper namespace (or a class with all public static functions), called PatchFunctions or something, that has all the functions that you want to apply to all the members of the collection.