Moving bad code around isn't about hiding it, it's about fixing it. The other piece of the puzzle is to identify the redundant bits and extract common functionality so that you don't have to write it multiple times. Moving things around helps to find redundancy (by putting redundant bits next to each other) and also to extract the functionality (by making it easier to decide where the common implementation logically goes).
In your case, the proposed Sprite::Render would not work with a "Sprite Player", but instead directly with the "this" object (and you only have to write "this-> in cases where there may be ambiguity). With a bit of rearrangement of whitespace, we get something like:
enum {UP, RIGHT, DOWN, LEFT} Direction;void Sprite::Render(Direction d) { switch (d) { case UP: // "Animation_Walk_Up" and "currentFrame" are implicitly the members of // the Sprite upon which this is called. DDback->Blt(&Animation_Walk_Up.Sequence[currentFrame].dest_rect, Animation_Walk_Up.Sequence[currentFrame].DDTile, &Animation_Walk_Up.Sequence[currentFrame].src_rect, DDBLT_WAIT | DDBLT_KEYSRC, NULL); break; [other cases here] }}
Now we can start to notice redundancy. First thing would be the repetition that's actually shown here; that of "Animation_Walk_Up.Sequence[Player.currentFrame]". Applying the principle from before, we're doing a bunch of work *with* some other object; instead, let's have the work done *by* that object - I'll assume it's of type Frame, and give it its own Render method:
enum {UP, RIGHT, DOWN, LEFT} Direction;void Frame::Render() { // At this point, the direction has already been "processed" - it was used // to determine which Frame to Render() - so it's not passed along. DDback->Blt(&dest_rect, DDTile, &src_rect, DDBLT_WAIT | DDBLT_KEYSRC, NULL);}void Sprite::Render(Direction d) { switch (d) { case UP: // Now we just need to use the local members in order to find an appropriate // Frame and Render() it. Animation_Walk_Up.Sequence[currentFrame].Render(); break; [other cases here] }}
Now the next step is to look at that switch statement - in general, switch and if constructs are the first places to look for redundancy (or other reasons to refactor) :) Here, I assume you will end up with basically the same code for each case, except using other members Animation_Walk_Right, Animation_Walk_Down and Animation_Walk_Left. Since an enumeration is convertible to an int (but in C++, not the other way around), we can use it to index an array, which is exactly what we're going to do - physically this will be an array of four Animations indexed 0..3, but logically it will be indexed by "all possible Directions", which is to say it "holds Animation sequences for all possible Directions". Nice and neat. :) I will trust you to be able to make the other necessary changes to the Sprite declartion for this:
enum {UP, RIGHT, DOWN, LEFT} Direction;void Frame::Render() { // At this point, the direction has already been "processed" - it was used // to determine which Frame to Render() - so it's not passed along. DDback->Blt(&dest_rect, DDTile, &src_rect, DDBLT_WAIT | DDBLT_KEYSRC, NULL);}void Sprite::Render(Direction d) { // Now we index an array of walk animations to figure out where to go. Walk_Animations[d].Sequence[currentFrame].Render();}
See how much neater things are now :) Anyway, depending on the rest of the code, we can go further with this. For example, if there will become other Sprite methods that delegate similarly to a Frame, we may want to extract a "helper" function to do the common part "(foo)_Animations[d].Sequence[currentFrame]". This might in turn involve creating another enumeration for "animation type", and collapsing all the sets of animations (walk animations, attack animations etc.) into an array:
// This would be private, since it's just used to facilitate implementation of// "real" functionality, and is not really part of the Sprite interface.Frame& Sprite::getFrame(AnimationType a, Direction d) { return Animations[a][d].Sequence[currentFrame];}void Sprite::Render(Direction d) { getFrame(WALK, d).Render();}
