patternjavaMinor
Controlling a robot arm in a Java GUI program
Viewed 0 times
robotprogramjavacontrollingarmgui
Problem
I have this funny program for playing around with 2D vectors:
Vector2D.java:
```
package net.coderodde.robohand;
import java.awt.geom.Point2D;
import java.util.List;
import java.util.ArrayList;
import java.util.Objects;
/**
* This class implements a vector. The tail of another vector can be attached to
* the head of this vector.
*
* @author Rodion "rodde" Efremov
* @version 1.6 (Apr 17, 2016)
*/
public class Vector2D {
/**
* The x-coordinate of the tail of this vector.
*/
private float locationX;
/**
* The y-coordinate of the tail of this vector.
*/
private float locationY;
/**
* The length of this vector.
*/
private float length;
/**
* The angle of this vector in radians.
*/
private float angle;
/**
* The list of all the child vectors of this vector. Child vectors' tails
* are attached to the head of this vector.
*/
private final List childVectors = new ArrayList<>();
/**
* Constructs a vector with given length and angle.
*
* @param length the length of a new vector.
* @param angle the angle in radians of a new vector.
*/
public Vector2D(float length, float angle) {
this.length = length;
this.angle = angle;
}
/**
* Connects {@code vector} as a child to this vector.
*
* @param vector the child vector to add.
*/
public void addChildVector(Vector2D vector) {
Objects.requireNonNull(vector, "The input vector is null.");
childVectors.add(vector);
Point2D.Float headLocation = getHeadLocation();
vector.setLocation(headLocation.x, headLocation.y);
}
/**
* Removes a child vector if it is connected to the tail of this vector.
*
* @param vector the vector to remove from the child list.
*/
public void removeChildVector(Vector2D vector) {
Objects.requireNonNull(vector, "The input vector is null.");
childV
Vector2D.java:
```
package net.coderodde.robohand;
import java.awt.geom.Point2D;
import java.util.List;
import java.util.ArrayList;
import java.util.Objects;
/**
* This class implements a vector. The tail of another vector can be attached to
* the head of this vector.
*
* @author Rodion "rodde" Efremov
* @version 1.6 (Apr 17, 2016)
*/
public class Vector2D {
/**
* The x-coordinate of the tail of this vector.
*/
private float locationX;
/**
* The y-coordinate of the tail of this vector.
*/
private float locationY;
/**
* The length of this vector.
*/
private float length;
/**
* The angle of this vector in radians.
*/
private float angle;
/**
* The list of all the child vectors of this vector. Child vectors' tails
* are attached to the head of this vector.
*/
private final List childVectors = new ArrayList<>();
/**
* Constructs a vector with given length and angle.
*
* @param length the length of a new vector.
* @param angle the angle in radians of a new vector.
*/
public Vector2D(float length, float angle) {
this.length = length;
this.angle = angle;
}
/**
* Connects {@code vector} as a child to this vector.
*
* @param vector the child vector to add.
*/
public void addChildVector(Vector2D vector) {
Objects.requireNonNull(vector, "The input vector is null.");
childVectors.add(vector);
Point2D.Float headLocation = getHeadLocation();
vector.setLocation(headLocation.x, headLocation.y);
}
/**
* Removes a child vector if it is connected to the tail of this vector.
*
* @param vector the vector to remove from the child list.
*/
public void removeChildVector(Vector2D vector) {
Objects.requireNonNull(vector, "The input vector is null.");
childV
Solution
float x = locationX + length * (float) Math.cos(angle);
float y = locationY + length * (float) Math.sin(angle);
point.x = x;
point.y = y;These local stores don't add any relevant information and should just be written directly to
point's variables./**
* Chooses a preceding vector in the list as selected. If the current
* selected vector is first in the list, does nothing.
*/
public void decSelectedVectorIndex() {
if (selectedVectorIndex > 0) {
selectedVectorIndex--;
}
repaint();
}If the current selected vector is first in the list, does nothing.
So what does
repaint() do? Probably not nothing./**
* Makes selected vector a little bit longer.
*/
public void enlargeCurrentSelectedVector() {
setSelectedVectorLength(LENGTH_DELTA);
}longer
"set" implies "becomes equal to". But here it adds, instead.
public void rotateSelectedVector(float angleDelta) {
float newAngle = vectorList.get(selectedVectorIndex)
.getAngle() + angleDelta;
vectorList.get(selectedVectorIndex).setAngle(newAngle);
repaint();
}You have to break it up into multiple lines because you don't want to pull the vector out of the list. This makes for a lengthy line...
public void rotateSelectedVector(float angleDelta) {
Vector2D selectedVector = vectorList.get(selectedVectorIndex);
float newAngle = selectedVector.getAngle() + angleDelta;
selectedVector.setAngle(newAngle);
repaint();
}But even when edited it looks weird. Your getters/setters are hindering you here, ideally you'd just do something like this:
selectedVector.angle += angleDelta;Because we can't do this without making access public, the second alternative is making a method that will allow you to add a value.
/**
* Gets the angle of this vector.
*
* @return the angle of this vector.
*/
public float getAngle() {
return angle;
}
/**
* Sets the angle of this vector. If this vector has child vectors, it
* updates their location and angles as well.
*
* @param angle the angle in radians.
*/
public void setAngle(float angle) {
float angleDelta = angle - this.angle;
this.angle = angle;
Point2D.Float headLocation = getHeadLocation();
for (Vector2D childVector : childVectors) {
childVector.setLocation(headLocation.x, headLocation.y);
childVector.setAngle(childVector.getAngle() + angleDelta);
}
}But it looks like your setter is actually quite complex, so we definitely cannot get rid of the setter here. I think a helper which takes
angleDelta would be useful....
Heck, looking at it again, I think making the
set a helper and the delta the real one is the better choice.public void addAngle(float angleDelta) {
this.angle += angleDelta;
Point2D.Float headLocation = getHeadLocation();
for (Vector2D childVector : childVectors) {
childVector.setLocation(headLocation.x, headLocation.y);
childVector.addAngle(angleDelta);
}
}You save having to calculate the delta for every child.
public void update(Graphics g) {
vectorList.get(0).setLocation(getWidth() / 2, getHeight() / 2);What's that for? On a repaint, you update your model. Why does looking at data change the data? Could use a comment. Or a wrapping function. (I get the feeling it's for centering the hand.)
Code Snippets
float x = locationX + length * (float) Math.cos(angle);
float y = locationY + length * (float) Math.sin(angle);
point.x = x;
point.y = y;/**
* Chooses a preceding vector in the list as selected. If the current
* selected vector is first in the list, does nothing.
*/
public void decSelectedVectorIndex() {
if (selectedVectorIndex > 0) {
selectedVectorIndex--;
}
repaint();
}/**
* Makes selected vector a little bit longer.
*/
public void enlargeCurrentSelectedVector() {
setSelectedVectorLength(LENGTH_DELTA);
}public void rotateSelectedVector(float angleDelta) {
float newAngle = vectorList.get(selectedVectorIndex)
.getAngle() + angleDelta;
vectorList.get(selectedVectorIndex).setAngle(newAngle);
repaint();
}public void rotateSelectedVector(float angleDelta) {
Vector2D selectedVector = vectorList.get(selectedVectorIndex);
float newAngle = selectedVector.getAngle() + angleDelta;
selectedVector.setAngle(newAngle);
repaint();
}Context
StackExchange Code Review Q#125948, answer score: 2
Revisions (0)
No revisions yet.