Is this bad code...

I was talking with a guy at work about object oriented coding, and he mentioned that it was not a good idea to use too long references to an object. In my current code I'm doing things like this.

hero.pre.t.x

Where hero is the character, pre is a transform object containing the previous position of the hero, t is translation stored as a vector object, and x is the axis. Is this bad code, or is it just fine.

And what if you do the same, but run a method in there as well, like...

hero.t.Distance( enemy.t )

Ragnar

That's all syntactically fine code.

perhaps all he meant by that is that its cleaner to use
a method to extract a var burried deep in your type/class
instance...

hero.pre.t.x - might be more readable as hero.getx()
or something similar...

theres nothing wrong with either solution..its mostly
personal style - ignoring optimisations..

Thanks for the reply.

If it's just down to personal style, I think I'll stick with the way I've got it now.

Still learning OOP, I understand all the basic concepts and how it works, but it's sometimes a bit hard to figure out the best way (if there is such a thing) to organize you types, objects etc. :-)

What I have now is a TVector type, with all the vector calculations I need. A TTransform type which contains translation, rotation and scale fields (translation and scale being of type Vector). The TTransform also has a material field of type TSprite, where I can attach an image or other visual representation. TTransform is then extended to TParticle, which includes physics things (still at a very VERY early stage in development). I'm also adding TForces and different forces and constraints extending that. I don't no if I'm going overboard with diving everything into different Types, but at the moment it feels about right. :-)

Combining TTransform, TParticle and TSprite into one, might possibly simplify things. Not really sure.

Ragnar

you might want to consider not extending types unless
it makes sense...

instead of TTransform - perhaps rename it TEntity..i know
this is nothing more than a name change..but look at it this
way...

shape
box extends shape - "box is a shape!" makes sense

transform
sprite extends transform - "sprite is a transform"..doesnt make sense

entity
sprite - extends entity - "sprite is an entity"..makes sense


type entity
field t:transform
endtype

type sprite extends entity
endtype

now sprite will have a tranform field..and the hierarchy
is meaningful...you dont need to use inheritance simply
beacuse its oopy..use composition unless the it really
makes sense to use inheritance...have transform as field
of the base class i think is cleaner in this case.

im no expert on this....but i hope this is helpful ;)

well if you wanted to get into the nitty gritty about object-oriented design then the code you posted above is generally looked at as 'bad'. As it violates the 'Law of Demeter':


http://en.wikipedia.org/wiki/Law_of_Demeter

Generally, when you start going down the chain of objects to get to a certain field then sometimes there might either be some un-needed complexity or bad design.

accessing your object like:
player.getLastPosition().getTransform().getX()

can generally lead to debugging headaches because now your player class knows intimate details of those lower level classes, so should something change (ie method names/signatures) you start getting errors in your code.

having a player.getX() method that delegates it further down into your classes is usually a safer way, but going too far with that methodology I think causes some other issues.