# Is something like this ok?

This topic is 4882 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Ever since I learnt about classes, I've loved using them. But in my game, I think I may have gone a little overboard. Is this ok?
DDback->Blt(&Player.Animation_Walk_Up.Sequence[Player.currentFrame].dest_rect,Player.Animation_Walk_Up.Sequence[Player.currentFrame].DDTile, &Player.Animation_Walk_Up.Sequence[Player.currentFrame].src_rect,DDBLT_WAIT | DDBLT_KEYSRC,NULL);

OR

Player.Animation_Walk_Right.Sequence[Player.currentFrame].dest_rect.left = Player.x;

The DDback->Blt() is DirectX, so it doesn't count. But, the rest is mine. Is that way too many objects? Cheers

##### Share on other sites
Whether or not it is too many objects depends on the details of the project.

I think it would be better to write a setting method, with a descriptive name, in the interface of the class to encapsulate all the indirection to other member classes.

##### Share on other sites
The problem is that it is a nightmare to read. And there doesn't appear to be any encapsulation of data at all, which would get a lot of people riled up.

It would probably be more generally acceptable to say something like...

Animation *anim = player->getAnimation[ANIM_WALK_RIGHT];Sequence *seq = anim->getSequence( player->getCurrentFrame() );seq->setDestRec( /* whatever */ );

Of course, I'm just making type names up and stuff, but this sort of thing is much more readable in my opinion.

##### Share on other sites
Quote:
 Original post by _SigmaBut in my game, I think I may have gone a little overboard. Is this ok?

It's rarely ok to go overboard with anything. [wink]
But you're the one who have to deal with your code. So you'll have to decide if it's ok.
Looks messy to me though.

But yeah, what happened to encapsulation?

##### Share on other sites
..encapsulation?? Anyone care to explain?

##### Share on other sites
Greatly simplified, encapsulation here means that you should hide implementation of specific code (this case: rendering of a player sprite) from the end-user. This helps to concentrate on the task you actually want to do; Just render the sprite, nothing else.

Here's how I do it in my sprite code:

CGameObject Player; // Create our sprite
Player.MoveRel(-1, 0); // Move 1.0 to left in relative coordinates
Player.SetAnimation(ANIM_WALK_LEFT);
Player.Render(); // ... And render him

You see how data is being hidden from the coder? Talk about cool huh :)

##### Share on other sites
"Encapsulation" means that access to an objects members is restricted to only those accesses that you deem appropriate.

For example... Let's change one of the lines that you used...
Player.Animation_Walk_Right.Sequence[Player.currentFrame].dest_rect.left = -2849309485.2945;

Is that what you want? Probably not... if you only allow access to that variable through an accessor method you can validate the values, etc.

sequence->getDestRec()->setLeft(-2849309485.2945);

Now, the setLeft() method can do some validation on the value.

Generally speaking, if someone (a potential employer, perhaps) looks at your code and sees you directly accessing any class members like you were doing, they will think less of you as a programmer.

Like someone else said, though, if this is just for you and you don't care what anyone else thinks then it doesn't really matter.

It's a good idea though to have getter/setter methods for all of your data, and if I were you I would try to get in the habit.

##### Share on other sites
ALright, I think I follow. So something like this then?
Sprite Player...Sprite::Render(Int Direction){  case(Direction)  {      case Right:                 DDback->Blt(&Player.Animation_Walk_Up.Sequence[Player.currentFrame].dest_rect,Player.Animation_Walk_Up.Sequence[Player.currentFrame].DDTile, &Player.Animation_Walk_Up.Sequence[Player.currentFrame].src_rect,DDBLT_WAIT | DDBLT_KEYSRC,NULL);break;[other cases here]}}...Player.Render(RIGHT);

So something like that then eh?

PS> How do you do the [code=cpp] thing again?

##### Share on other sites
Also, how hidden should "bad" code like that be? :P

##### Share on other sites
There's no need for all that repetition (copy'n'pasting). At least do something like this:
const Sequence &seq = Player.Animation_Walk_Up.Sequence[Player.currentFrame];DDback->Blt(&seq.dest_rect, seq.DDTile, &seq.src_rect, DDBLT_WAIT | DDBLT_KEYSRC, NULL);

1. 1
2. 2
3. 3
Rutin
15
4. 4
5. 5

• 9
• 9
• 11
• 11
• 23
• ### Forum Statistics

• Total Topics
633679
• Total Posts
3013297
×