Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been coding for some time (3 years I think), and have recently been wondering if my coding pattern is any good. Any kind of feedback is welcome:

EDIT: The thing that concerns me most is that I have read a lot around that getters/setters are bad for OOP. Is this true?

//Point3D.as
public class Point3D 
{
    private var _x:int;
    private var _y:int;
    private var _z:int;

    public function Point3D(tx:int = 0, ty:int = 0, tz:int = 0)
    {
        _x = tx;
        _y = ty;
        _z = tz;
    }

    public function get x():int { return _x; }
    public function set x(tv:int):void { _x = tv; }

    public function get y():int { return _y; }
    public function set y(tv:int):void { _y = tv; }

    public function get z():int { return _z; }
    public function set z(tv:int):void { _z = tv; }

}

//Geometry3D.as
public class Geometry3D
{
    private var _indices:Vector.<uint>;
    private var _vertices:Vector.<Number>;

    public function Geometry3D(tindices:Vector.<uint>,
    tvertices:Vector.<Number>)
    {
        _indices = tindices;
        _vertices = tvertices;
    }

    public function get indices():Vector.<uint> { return _indices; }
    public function get vertices():Vector.<Number> { return _vertices; }

}

//Primitive3D.as
public class Primitive3D 
{
    private var _position:Point3D;
    private var _geometry:Geometry3D;

    public function Primitive3D(tx:int, ty:int, tz:int, tgeometry:Geometry3D) 
    {
        _position = new Point3D(tx, ty, tz);
        _geometry = tgeometry;
    }

    public function get position():Point3D { return _position; }
    public function get geometry():Geometry3D { return _geometry; }

    public function render():void
    {
        //add render code here
    }

}

//TrianglePrimitive.as
public class TrianglePrimitive extends Primitive3D
{

    public function TrianglePrimitive(tx:int, ty:int, tz:int)
    {
        var indices:Vector.<uint> = Vector.<uint>([
        0, 1, 2]);
        var vertices:Vector.<Number> = Vector.<Number>([
        ]);

        var geo:Geometry3D = new Geometry3D(indices, vertices);
        super(tx, ty, tz, geo);
    }

    public function spin(tradians:Number):void
    {
        //add spin code here
    }

}

//Main.as
...
var tri:TrianglePrimitive = new TrianglePrimitive(0, 0, 0);
tri.position.z = -4;
tri.spin(Math.PI / 2); //90 degrees
tri.render(); 
...

Again, thanks a bunch for any feedback!

share|improve this question
    
You should mention what kind of things concern you about what you wrote. –  Juan Mendes Dec 20 '12 at 0:19
    
I wouldn't consider using getters and setters a good practice in Actionscript, as function calls are pretty expensive there. Especially when you just create them for future changes (one of benefits of getter/setter is that you can easily replace a property with it without changing the rest of code). Also, as a side note, consider creating methods getValue() instead of getters to indicate there's some extra work and it's better to cache the value than call the getter several times in one place. –  Markus von Broady Mar 30 '13 at 16:06

1 Answer 1

Looks good to me. Only thing I would say is:

  • The accessor methods (get / set) in Point3D are unnecessary as they don't do any processing or restrict any access. The class would be better suited to have simple, public x, y, z properties.
  • I don't see the purpose of the Geometry3D class. Maybe it's because I don't know much about your intentions or programming for 3d, but looks like things would be more straight-forward if indices and vertices properties were directly on Primitive3D. If it make sense to separate them off because they're processed as a pair by other parts of your program, you might also consider creating a IGeometry3D interface, which the Primitive3D class can implement. This would prevent there from being any ambiguity around ownership. (Not sure if I'm describing my thoughts, clearly.)

I don't think getters and setters are bad for OOP programming. I think they're fantastic. They allow for validation of property values, pre- and post-processing when changing properties, lazy execution of calculations that aren't always necessary, and a lot of other things. Admittedly, you could do all of that with functions. But, to my thinking, an important part of programming is managing complexity and thoughtful use of accessor methods can simplify APIs, which in turn makes life easier (fewer bugs and easier to maintain).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.