Reputation: 10946
The many repetitious switch statements seems like it needs to be DRY'd. Any suggestions? (Including doing nothing!)
AnimMapIter _iter;
_iter = _animations->find(name);
if(_iter == _animations->end()) return;
if(_curName != name) {
_curName = name;
switch(dir) {
case DIR_FORWARD_LOOPING: /* Fall through to DIR_FORWARD_NONLOOPING */
case DIR_FORWARD_NONLOOPING:
_iter->second->First();
break;
case DIR_REVERSE_LOOPING: /* Fall through to DIR_REVERSE_NONLOOPING */
case DIR_REVERSE_NONLOOPING:
_iter->second->Last();
break;
}
} else {
switch(dir) {
case DIR_FORWARD_LOOPING: /* Fall through to DIR_FORWARD_NONLOOPING */
case DIR_FORWARD_NONLOOPING:
_iter->second->Next();
break;
case DIR_REVERSE_LOOPING: /* Fall through to DIR_REVERSE_NONLOOPING */
case DIR_REVERSE_NONLOOPING:
_iter->second->Previous();
break;
}
switch(dir) {
case DIR_FORWARD_LOOPING:
if(_iter->second->IsAtEnd())
_iter->second->First();
break;
case DIR_FORWARD_NONLOOPING:
if(_iter->second->IsAtEnd())
_iter->second->Last();
break;
case DIR_REVERSE_LOOPING:
if(_iter->second->IsAtFront())
_iter->second->Last();
break;
case DIR_REVERSE_NONLOOPING:
if(_iter->second->IsAtFront())
_iter->second->First();
break;
}
}
Upvotes: 0
Views: 211
Reputation: 5431
Everything under the else should collapse into a single switch to bring the related steps closer; e.g.
case DIR_FORWARD_LOOPING:
_iter->second->Next();
if (_iter->second->IsAtEnd()) {
_iter->second->First();
}
break;
...all in that one case. Repetition of a couple of function calls is not a big deal when it makes the overall sequence of actions more clear.
Upvotes: 1
Reputation: 66459
Push the logic into whatever _iter->second
is, along these lines (assuming the methods you've already shown exist):
class WhateverItIs
{
public:
void Start() { if (m_forward) First(); else Last(); }
void Stop() { if (m_forward) Last(); else First(); }
void Advance()
{
if (m_forward)
Next();
else
Previous();
if (IsLast())
{
if (m_loop)
Start();
else
Stop();
}
}
private:
bool IsLast() const
{
return m_forward ? IsAtEnd() : IsAtFront();
}
// Direction and looping are independent concepts.
bool m_forward;
bool m_loop;
};
Then you can write:
AnimMapIter _iter;
_iter = _animations->find(name);
if(_iter == _animations->end()) return;
if(_curName != name) {
_curName = name;
_iter->second->Start();
} else {
_iter->second->Advance();
}
Edit: Example using free functions and keeping the constants.
void Start(Strip* s, bool forward)
{ if (forward) s->First(); else s->Last(); }
void Stop(Strip* s, bool forward)
{ if (forward) s->Last() else s->First(); }
void Advance(Strip* s, bool forward, bool loop)
{
if (forward)
s->Next();
else
s->Previous();
if (IsLast(s, forward))
{
if (loop)
Start(s);
else
Stop(s);
}
}
bool IsLast(const Strip* s, bool forward) const
{
return forward ? s->IsAtEnd() : s->IsAtFront();
}
bool Projector::IsForward() const
{
return dir == DIR_FORWARD_LOOPING || dir == DIR_FORWARD_NONLOOPING;
}
bool Projector::IsLooping() const
{
return dir == DIR_REVERSE_LOOPING || dir == DIR_FORWARD_LOOPING;
}
if(_curName != name) {
_curName = name;
Start(_iter->second, IsForward());
} else {
Advance(_iter->second, IsForward(), IsLooping());
}
Upvotes: 1