First, some code. I have a class, Circle:

package aag;

public class Circle{
    private final int RADIUS;
    private float xVector;
    private float yVector;
    private float xCurrent;
    private float yCurrent;

    public Circle(int radius, int xCurrent, int yCurrent){
        RADIUS = radius;
        this.xCurrent = xCurrent;
        this.yCurrent = yCurrent;
        xVector = 0;
        yVector = 0;
    }
    public float getXVector(){
        return xVector;
    }
    public void setXVector(float xVector){
        this.xVector = xVector;
    }
    public float getYVector(){
        return yVector;
    }
    public void setYVector(float yVector){
        this.yVector = yVector;
    }
    public float getXPosition(){
        return xCurrent;
    }
    public void setXPosition(float xCurrent){
        this.xCurrent = xCurrent;
    }
    public float getYPosition(){
        return yCurrent;
    }
    public void setYPosition(float yCurrent){
        this.yCurrent = yCurrent;
    }
    public int getRadius(){
        return RADIUS;
    }
    public void tick(){
        xCurrent += xVector;
        yCurrent += yVector;
    }
}  

The Circle's tick() method is called once per cycle, which updates the position members (xCurrent & yCurrent). I have another class, aptly named CollisionDetector, whose responsibility is to assess Circle positions and make changes under certain criteria. In that particular class, I am currently working on the following method:

    public void checkBounds(Circle circle, int fieldWidth, int fieldHeight){
        int radius = circle.getRadius();
        int boundWidth = fieldWidth - radius;
        int boundHeight = fieldHeight - radius;
        float xPosition = circle.getXPosition();
        float yPosition = circle.getYPosition();

        if(xPosition >= boundWidth){
            if(xPosition != boundWidth)
                circle.setXPosition(boundWidth + boundWidth - xPosition);
            circle.setXVector(-circle.getXVector());
        }
        if(xPosition <= radius){
            if(xPosition != radius){
                circle.setXPosition(radius + radius - xPosition);
            circle.setXVector(-circle.getXVector());
            }
        }

        if(yPosition >= boundHeight){
            if(yPosition != boundHeight)
                circle.setYPosition(boundHeight + boundHeight - yPosition);
            circle.setYVector(-circle.getYVector());
        }
        if(yPosition <= radius){
            if(yPosition != radius){
                circle.setYPosition(radius + radius - yPosition);
            circle.setYVector(-circle.getYVector());
            }
        }
    }

I think it is obvious that there are several patterns repeating themselves in this method. I am trying to reduce repetitive code, and have so far come up with the following:

    private void upperCheck(float position, int bound, float vector, Consumer<Float> setPosition, Consumer<Float> setVector){
        if(position >= bound){
            checkHelper(position, bound, setPosition);
            setVector.accept(-vector);
        }
    }
    private void lowerCheck(float position, int bound, float vector, Consumer<Float> setPosition, Consumer<Float> setVector){
        if(position <= bound){
            checkHelper(position, bound, setPosition);
            setVector.accept(-vector);
        }
    }
    private void checkHelper(float position, int bound, Consumer<Float> setPosition){
        if(position != bound)
            setPosition.accept(bound + bound - position);
    }  

At the moment, my gut says that there must be a way to extract the comparison between position and bound so that I have a generic method which will (potentially) take >= or <= as a parameter.

Finally, my questions:

  1. First, Can I extract my comparison in such way as to not require two methods differing only in how they compare values?
  2. Second, how can I structure this method?

I hope I am not asking for something too specific my first time out. I felt that reducing repetition was a general enough idea, as well as having tried for a few days to divine a more efficient solution, that I should pose the question.

P.S.--Much of the code from Circle has been left out since it is not relevant to the scope of my question. Additionally, I have only included the methods from my CollisionDetector relevant to my concern. I could provide full code if necessary.

migrated from programmers.stackexchange.com Nov 12 '15 at 13:07

This question came from our site for professionals, academics, and students working within the systems development life cycle.

up vote 2 down vote accepted

I will just scetch how to approach your problem, but leave the gory implementation details up to you:

  • to avoid duplicate code which differs only in the direction in comparison a<b or a>b, you refactor that part of the code out into a function with an additional int parameter sign. sign should be either -1 or +1, and the comparison can now be written as sign*a < sign*b. After that, you can call either call this new function twice explicitly, or just write a loop like

    for(int sign=-1; sign<=1; sign+=2)

  • to avoid the repetition of X an Y, one possible solution is to implement your xy-vectors in terms of two dimensional arrays:

    private float[] vector = new float[2];

    private float[] current =new float[2]

Again, now it will be possible to loop over the vector components when operating on them, instead of writing almost the same code twice. Of course, that comes for the price of loosing a little bit of readability, and for the price of loosing a little bit performance. In fact, the last two points are the reason why I personally prefer a different solution for this category of problems:

First, create a fairly simple class named Vector2D, with attributes in terms of X and Y. This class should provide all basic 2D vector operations, like adding/substracting 2D vectors, multiplication with a scalar etc. There will be a little bit of similar looking code for all X and Y operations inside the implementation of this class, but not too much. Now, implement your Circle class in terms of Vector2D:

   `private Vector2D vector;`

   `private Vector2D current;`

Do not provide any methods for your Circle with individual X and Y components.

Of course, the latter approach will not directly decrease the size of your checkBounds function much, since that is an operation which cannot be implemented easily with classic 2D vector arithmetics, and it actually has to deal with the X and Y components individually. But it will most probably decrease the size of a lot of other methods inside your program.

  • Thank you! I do not think I understand how I would utilize Vector2D with respect to xCurrent and yCurrent however, as they are positional. I like the usage of a changing sign to evaluate the two arguments; I had not considered that. I do not think I want to use arrays as I do not want to explicitly connect x-axis members with y-axis members. – Judgemental Nov 12 '15 at 16:43
  • @Judgemental: I have "inherited" a fairly amount of legacy code where the author was not aware of (or maybe just too blind to see) the benefits of vector arithmetik. Everywhere in his code he passes X,Y,and Z as individual components - it is horrible. When you have methods dealing with two, three or four vectors, you get huge, unreadable parameter lists. And always the same stupid vector addition/substraction etc in 3 lines of code instead of one. Be careful not to make the same failure, you will regret it, I swear. – Doc Brown Nov 12 '15 at 17:47
  • I still haven't written any code, but after considering the broader implications of storing axis values as part of an array I think I am beginning to grasp how much easier it would be to manipulate. I still don't have a foothold however on your Vector2D class. Would you mind expanding on the idea a bit? I don't think I need to see an implementation, but I'm just not following it yet. – Judgemental Nov 12 '15 at 21:00
  • In fact, there is probably no need to implement your own class, found this one. geom-java.sourceforge.net/api/math/geom2d/Vector2D.html (and by conincidence, the authors of that class picked the same name). That lib also contains a `Point2D´ class, but IMHO that's feels unnatural to me, because one of the first things I learned in school about vectors and points is that they are perfectly interchangeable. – Doc Brown Nov 12 '15 at 21:47
  • Whoa! I think I am starting to grasp what is behind your assertion. I had not understood the concept of a point vs. a vector, but after a little quick research I see what you mean; that's quite an unexpected leap for me as I have limited calculus/physics experience. Looking at my own structure within Circle I can see very clearly that my vector is quite obviously a point: I even treat it as such when I operate on the x/y vectors. Thank you very much, this has been very helpful! – Judgemental Nov 13 '15 at 1:30

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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