Good idea but I don't like your naming style
Also I think better to use functors here. Something like this:
template<typename Check>
Unit* Creature::SelectAttackingTarget(AttackingTarget type, uint32 position, Check condition)
{
ThreatList const& threatList = m_creature->getThreatManager().getThreatList();
UnitList targetList;
for (ThreatList::const_iterator itr = threatList.begin(); itr != threatList.end(); ++itr)
{
if (Unit *pUnit = Unit::GetUnit(*this, (*itr)->getUnitGuid()))
{
if (condition(pUnit))
targetList.push_back(pUnit);
}
}
UnitList::iterator itr = targetList.begin();
UnitList::reverse_iterator ritr = targetList.rbegin();
if (targetList.empty() || position >= targetList.size())
return NULL;
switch (target)
{
case ATTACKING_TARGET_RANDOM:
std::advance(itr, urand(position, targetList.size() - 1));
return *itr;
case ATTACKING_TARGET_TOP:
std::advance(itr, position);
return *itr;
case ATTACKING_TARGET_BOTTOM:
std::advance(ritr, position);
return *ritr;
}
return NULL;
}
Then calls would be like this:
if(Unit* target = m_creature->SelectAttackingTarget(ATTACKING_TARGET_TOP, 0, UnitInRangeCheck(m_creature, ATTACK_DISTANCE)))
...
This approach is more universal and simpler in use.