Return to Answer

• 1

• 34.1k
• 4
• 77
• 145

###Names

Just for example, Info is much too generic a name. Although you haven't said much about what exact sort of info this is, you should rename it to reflect the kind of info being stored.

###Data Structures

As long as you're sure you'll never store more than 16 items, I suppose it's reasonable to use a vector and use linear searching to find items. If there's any possibility of more items, I'd switch to something like a map or set. Even as it stands now, I'd consider using a map or unordered_map anyway, simply because it directly supports so many of the things you want/need to do.

Top-level const:

Consider the parameters to your isNodeDiscovered:

bool isNodeDiscovered(const InfoEnum name,const InfoIdEnum id=InfoIdEnum::NONE)

You're passing these by value, so the const doesn't really mean or accomplish anything and might as well be removed:

bool isNodeDiscovered(InfoEnum name, InfoIdEnum id=InfoIdEnum::NONE)

Note that this is not the case when the const applies to something to which you're passing a reference or pointer. There the const can mean a lot and shouldn't be removed with impunity.

Encapsulation

Since you have a specific idea of how one Info object should be compared to another for equality, I'd encode that into the Info object itself:

bool Info::operator==(Info const &other) const { 
 return info_name == other.info_name && info_id == other.info_id;
}

This lets you use standard algorithms to implement most of your other operations:

bool find_info_element(Info *info, InfoEnum info_name, InfoIdEnum info_id=InfoIdEnum::NONE) {
 Info i { info_name, info_id};
 auto pos = std::find(m_info_vector.begin(), m_info_vector.end(), i);
 if (pos == m_info_vector.end())
 return false;
 *info = *pos;
 return true;
}

I'd at least consider changing this to just return the iterator though--that would be much more in keeping with how things are typically done in C++.

###Using map

As mentioned previously, using std::map (or unordered_map) would simplify the code quite a bit--to the point that I'm tempted to recommend using it, even though it's sort of overkill for just 16 elements. To do this, we'd start by separating an Info into the part that acts as the key and the data that's stored with that key:

typedef std::pair<InfoEnum, InfoIdEnum> Key;
class Data {
 InfoStatesEnum info_state;
 bool info_discovered = false;
};

Then we'd define a map of that type:

std::map<key, data> info_data;

Then (for example) updateInformation would become something like this:

void updateInformation(Key const &key, Data const &data)
{
 info_data[key] = data;
}

Another possibility would be to use a class that provided the same interface as a map, but stored its data in a vector. For one example, Boost's flat_map does exactly this.