Reputation: 87
Hey I have got an application that generates random shapes in random locations and now I'm trying to divide the code into classes responsible for certain things. But when I cut out part of the code responsible for "randomizing" and moved it to another class, it's not working (not randomizing) anymore. Any ideas? My code below:
MAIN CLASS
import java.awt.BorderLayout;
public class Main extends JFrame {
public Main() {
setLayout(new BorderLayout());
add(new TestPanel(), BorderLayout.CENTER);
}
public static void main(String[] args) {
new TestPanel();
Main frame = new Main();
frame.setSize(1000, 700);
frame.setTitle("Random things");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
//frame.setResizable(false);
}
}
TESTPANEL CLASS
import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Graphics;
import java.util.Random;
import javax.swing.JFrame;
import javax.swing.JPanel;
public class TestPanel extends JPanel {
Randoms randomized = new Randoms();
int howManyGeometrics = randomized.random1.nextInt(5-1) + 1;
int whichGeometrics = randomized.random1.nextInt(3);
public void paintComponent(Graphics g) {
super.paintComponent(g);
for (int i = 0; i < howManyGeometrics; i++) {
draw(g);
}
}
public void draw(Graphics g) {
if(whichGeometrics==0){
//oval
g.setColor(randomized.randomColour2);
g.fillOval(randomized.x1, randomized.x2, randomized.x4, randomized.x3);
//triangle
g.setColor(randomized.randomColour3);
int xpoints[] = {randomized.x1+10, randomized.x2+15, randomized.x3+20};
int ypoints[] = {randomized.x4-20, randomized.x3-15, randomized.x2-20};
int npoints = 3;
g.fillPolygon(xpoints, ypoints, npoints);
//hourglass
g.setColor(randomized.randomColour4);
int a1=50+randomized.x1;
int a2=200+randomized.x2;
int a3=a1;
int a4=a2;
int b1=a1;
int b2=a2/2;
int b3=a2/2;
int b4=a1;
int x1points[] = {a1, a2, a3, a4};
int y1points[] = {b1, b2, b3, b4};
int ntpoints = 4;
g.fillPolygon(x1points, y1points, ntpoints);
}
else if(whichGeometrics==1){
//rectangle
g.setColor(randomized.randomColour1);
g.fillRect(randomized.x2, randomized.x3, randomized.x1, randomized.x4/2);
//triangle
g.setColor(randomized.randomColour3);
int xpoints[] = {randomized.x1+10, randomized.x2+15, randomized.x3+20};
int ypoints[] = {randomized.x4-20, randomized.x3-15, randomized.x2-20};
int npoints = 3;
g.fillPolygon(xpoints, ypoints, npoints);
//hourglass
g.setColor(randomized.randomColour4);
int a1=50+randomized.x1;
int a2=200+randomized.x2;
int a3=a1;
int a4=a2;
int b1=a1;
int b2=a2/2;
int b3=a2/2;
int b4=a1;
int x1points[] = {a1, a2, a3, a4};
int y1points[] = {b1, b2, b3, b4};
int ntpoints = 4;
g.fillPolygon(x1points, y1points, ntpoints);
}
else if(whichGeometrics==2){
//rectangle
g.setColor(randomized.randomColour1);
g.fillRect(randomized.x2, randomized.x3, randomized.x1, randomized.x4/2);
//oval
g.setColor(randomized.randomColour2);
g.fillOval(randomized.x1, randomized.x2, randomized.x4, randomized.x3);
//hourglass
g.setColor(randomized.randomColour4);
int a1=50+randomized.x1;
int a2=200+randomized.x2;
int a3=a1;
int a4=a2;
int b1=a1;
int b2=a2/2;
int b3=a2/2;
int b4=a1;
int x1points[] = {a1, a2, a3, a4};
int y1points[] = {b1, b2, b3, b4};
int ntpoints = 4;
g.fillPolygon(x1points, y1points, ntpoints);
}
else if(whichGeometrics==3){
//rectangle
g.setColor(randomized.randomColour1);
g.fillRect(randomized.x2, randomized.x3, randomized.x1, randomized.x4/2);
//oval
g.setColor(randomized.randomColour2);
g.fillOval(randomized.x1, randomized.x2, randomized.x4, randomized.x3);
//triangle
g.setColor(randomized.randomColour3);
int xpoints[] = {randomized.x1+10, randomized.x2+15, randomized.x3+20};
int ypoints[] = {randomized.x4-20, randomized.x3-15, randomized.x2-20};
int npoints = 3;
g.fillPolygon(xpoints, ypoints, npoints);
}
}
}
RANDOMS CLASS
import java.awt.Color;
import java.util.Random;
public class Randoms {
Random random1 = new Random(System.currentTimeMillis());
int x1 = random1.nextInt(800);
int x2 = random1.nextInt(800);
int x3 = random1.nextInt(800);
int x4 = random1.nextInt(800);
int red = random1.nextInt(255);
int green = random1.nextInt(255);
int blue = random1.nextInt(255);
Color randomColour1 = new Color(red,green,blue);
Color randomColour2 = new Color(green,blue,red);
Color randomColour3 = new Color(blue,red,green);
Color randomColour4 = new Color(green,red,red,green);
}
I will be grateful for help or/and advices :)
Upvotes: 0
Views: 273
Reputation: 54649
As John3136 already pointed out in https://stackoverflow.com/a/21639680 , the main problem is that you are always using the same Randoms
instance. But admittedly, this Randoms
class itself seems questionable in its current form. The random values are stored as fields implicitly causing the original issue. I could imagine that such a Randoms
class might be more useful if it contained the appropriate methods. For example, methods like
class Randoms
{
private static final Random random =
new Random(System.currentTimeMillis());
public static Color createRandomColor()
{
int red = random.nextInt(255);
int green = random.nextInt(255);
int blue = random.nextInt(255);
return new Color(red,green,blue);
}
public static int nextInt(int min, int max)
{
return random.nextInt(max - min) + min;
}
}
Apart from that: The code for the "hourglass" is always the same. In such cases, you should consider creating a method like
private void paintHourglass(Graphics g, int x0, int x1) {
// Here the repeated code...
}
Maybe you should more clearly describe what you want to achieve. If you just want to generate a set of random shapes, then you can create them and store them in a list:
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Shape;
import java.awt.geom.Ellipse2D;
import java.awt.geom.Rectangle2D;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import javax.swing.JFrame;
import javax.swing.JPanel;
public class RandomsTest extends JFrame {
public static void main(String[] args) {
JFrame frame = new JFrame();
frame.getContentPane().add(new TestPanel());
frame.setSize(1000, 700);
frame.setTitle("Random things");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
//frame.setResizable(false);
}
}
class TestPanel extends JPanel
{
private List<Geometry> geometries;
TestPanel()
{
geometries = new ArrayList<Geometry>();
int numGeometries = Randoms.nextInt(5, 8);
for (int i=0; i<numGeometries; i++)
{
geometries.add(new Geometry());
}
}
public void paintComponent(Graphics gr)
{
super.paintComponent(gr);
Graphics2D g = (Graphics2D)gr;
for (Geometry geometry : geometries)
{
geometry.draw(g);
}
}
}
class Geometry
{
private final Color color;
private final Shape shape;
Geometry()
{
this.color = Randoms.createRandomColor();
this.shape = Shapes.createRandomShape();
}
void draw(Graphics2D g)
{
g.setColor(color);
g.fill(shape);
}
}
class Shapes
{
static Shape createRandomShape()
{
int type = Randoms.nextInt(2);
switch (type)
{
case 0 : return createRandomOval();
case 1 : return createRandomRectangle();
}
return createRandomOval();
}
private static Shape createRandomOval()
{
return new Ellipse2D.Double(
Randoms.nextInt(800),
Randoms.nextInt(800),
Randoms.nextInt(800),
Randoms.nextInt(800));
}
private static Shape createRandomRectangle()
{
return new Rectangle2D.Double(
Randoms.nextInt(800),
Randoms.nextInt(800),
Randoms.nextInt(800),
Randoms.nextInt(800));
}
}
class Randoms
{
private static final Random random =
new Random(System.currentTimeMillis());
public static Color createRandomColor()
{
int red = random.nextInt(255);
int green = random.nextInt(255);
int blue = random.nextInt(255);
return new Color(red,green,blue);
}
public static int nextInt(int min, int max)
{
return random.nextInt(max - min) + min;
}
public static int nextInt(int max)
{
return random.nextInt(max);
}
}
Upvotes: 1
Reputation: 29266
You create one instance of Randoms
and then keep using it's members x1
etc without changing their value.
I suspect your old working code created multiple Randoms
instances?
A simple solution would be to add a reRandomize()
method to Randoms
that sets each member to a new random value and call that before each shape.
Upvotes: 1
Reputation: 19
You should use Math.random() every time you need a new random number.
Upvotes: 0