Is this bad code...
BlitzMax Forums/BlitzMax Programming/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': More formally, the Law of Demeter for functions requires that any method M of an object O may only invoke the methods of the following kinds of objects: 1. itself 2. its parameters 3. any objects it creates/instantiates 4. its direct component objects In particular, an object should avoid invoking methods of a member object returned by another method. 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. |
   |