views:

176

answers:

5

Hello. I have written program for drawing pythagoras tree fractal. Can anybody see any way of improving it ? Now it is 89 LOC.

import java.awt.*;
import java.util.Scanner;
import javax.swing.*;

public class Main extends JFrame {;

    public Main(int n) {
        setSize(900, 900);
        setTitle("Pythagoras tree");
        add(new Draw(n));
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setVisible(true);
    }

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        System.out.print("Give amount of steps: ");
        new Main(sc.nextInt());
    }
}

class Draw extends JComponent {
    private int height = 800;
    private int width = 800;
    private int steps;

    public Draw(int n) {
        steps = n;

        Dimension d = new Dimension(width, height);
        setMinimumSize(d);
        setPreferredSize(d);
        setMaximumSize(d);
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        g.setColor(Color.white);
        g.fillRect(0, 0, width, height);
        g.setColor(Color.black);

        int x1, x2, x3, y1, y2, y3;
        int base = width/7;

        x1 = (width/2)-(base/2);
        x2 = (width/2)+(base/2);
        x3 = width/2;
        y1 = (height-(height/15))-base;
        y2 = height-(height/15);
        y3 = (height-(height/15))-(base+(base/2));

        g.drawPolygon(new int[]{x1, x1, x2, x2, x1}, new int[]{y1, y2, y2, y1, y1}, 5);

        int n1 = steps;
        if(--n1 > 0){
            g.drawPolygon(new int[] {x1, x3, x2}, new int[] {y1, y3, y1}, 3);
            paintMore(n1, g, x1, x3, x2, y1, y3, y1);
            paintMore(n1, g, x2, x3, x1, y1, y3, y1);
        }
    }

    public void paintMore(int n1, Graphics g, double x1_1, double x2_1, double x3_1, double y1_1, double y2_1, double y3_1){
        int x1, x2, x3, y1, y2, y3;

        x1 = (int)(x1_1 + (x2_1-x3_1));
        x2 = (int)(x2_1 + (x2_1-x3_1));
        x3 = (int)(((x2_1 + (x2_1-x3_1)) + ((x2_1-x3_1)/2)) + ((x1_1-x2_1)/2));
        y1 = (int)(y1_1 + (y2_1-y3_1));
        y2 = (int)(y2_1 + (y2_1-y3_1));
        y3 = (int)(((y1_1 + (y2_1-y3_1)) + ((y2_1-y1_1)/2)) + ((y2_1-y3_1)/2));

        g.setColor(Color.green);
        g.drawPolygon(new int[] {x1, x2, (int)x2_1, x1}, new int[] {y1, y2, (int)y2_1, y1}, 4);
        g.drawLine((int)x1, (int)y1, (int)x1_1, (int)y1_1);
        g.drawLine((int)x2_1, (int)y2_1, (int)x2, (int)y2);
        g.drawLine((int)x1, (int)y1, (int)x2, (int)y2);

        if(--n1 > 0){
            g.drawLine((int)x1, (int)y1, (int)x3, (int)y3);
            g.drawLine((int)x2, (int)y2, (int)x3, (int)y3);
            paintMore(n1, g, x1, x3, x2, y1, y3, y2);
            paintMore(n1, g, x2, x3, x1, y2, y3, y1);
        }
    }
}
A: 

replace:

private int pow(int n){

    int pow = 2;
    for(int i = 1; i < n; i++){
        if(n==0){
            pow = 1;
        }
        pow = pow*2;
    }
    return pow;
}

with

private int pow2(int n) { //Better name, to avoid confusion
    return 1 << n;
}

Although I can't see where you use this function.

Eric
A: 

For starters, you can replace the if in pow() with a ternary operator:

pow = (n==0) ? 1 : pow*2;

and replace this

   private int height;
    private int width;
    private int steps;

with this:

private int height,width,steps

you dont need n1 this :

   int n1 = steps;
            n1--;
            if(n1>0){
                g.drawLine(x1, y1, x5, y5);
                g.drawLine(x4, y4, x5, y5);       
                paintMore(n1, g, x1, x5, x4, y1, y5, y4);
                paintMore(n1, g, x4, x5, x1, y4, y5, y1);
            }

can be replaced with this:

    if( (steps-1) > 0){
        g.drawLine(x1, y1, x5, y5);
        g.drawLine(x4, y4, x5, y5);       
        paintMore(n1, g, x1, x5, x4, y1, y5, y4);
        paintMore(n1, g, x4, x5, x1, y4, y5, y1);
    }
Mihir Mathuria
without the comments 108LOC.
sasquatch90
+2  A: 
  • Use drawPolygon instead of multiply drawLine
  • Remove the unused method pow,
  • Clean up your imports
  • Remove the unnecessary variables h and w
  • put --n > 0 into a one line
  • do some reformatting

and there you go, 90 lines (comments still counted):

import java.awt.*;
import java.util.Scanner;
import javax.swing.*;

public class Main extends JFrame {;

    public Main(int n) {
        setSize(900, 900);
        setTitle("Pythagoras tree");
        add(new Draw(n));
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setVisible(true);
    }

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        System.out.print("Give amount of steps: ");
        new Main(sc.nextInt());
    }
}

class Draw extends JComponent {
    private int height = 800;
    private int width = 800;
    private int steps;

    public Draw(int n) {
        steps = n;

        Dimension d = new Dimension(width, height);
        setMinimumSize(d);
        setPreferredSize(d);
        setMaximumSize(d);
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        g.setColor(Color.white);
        g.fillRect(0, 0, width, height);
        g.setColor(Color.black);

        int x1, x2, x3, y1, y2, y3;
        int base = width/7;

        x1 = (width/2)-(base/2);
        x2 = (width/2)+(base/2);
        x3 = width/2;
        y1 = (height-(height/15))-base;
        y2 = height-(height/15);
        y3 = (height-(height/15))-(base+(base/2));

        //paint
        g.drawPolygon(new int[]{x1, x1, x2, x2, x1}, new int[]{y1, y2, y2, y1, y1}, 5);

        int n1 = steps;
        if(--n1 > 0){
            g.drawPolygon(new int[] {x1, x3, x2}, new int[] {y1, y3, y1}, 3);
            paintMore(n1, g, x1, x3, x2, y1, y3, y1);
            paintMore(n1, g, x2, x3, x1, y1, y3, y1);
        }
    }

    public void paintMore(int n1, Graphics g, double x1_1, double x2_1, double x3_1, double y1_1, double y2_1, double y3_1){
        double x1, x2, x3, y1, y2, y3;
        //counting
        x1 = x1_1 + (x2_1-x3_1);
        x2 = x2_1 + (x2_1-x3_1);
        x3 = ((x2_1 + (x2_1-x3_1)) + ((x2_1-x3_1)/2)) + ((x1_1-x2_1)/2);
        y1 = y1_1 + (y2_1-y3_1);
        y2 = y2_1 + (y2_1-y3_1);
        y3 = ((y1_1 + (y2_1-y3_1)) + ((y2_1-y1_1)/2)) + ((y2_1-y3_1)/2);

        //paint
        g.setColor(Color.green);
        g.drawPolygon(new int[] {(int)x1, (int)x2, (int)x2_1, (int)x1},
                      new int[] {(int)y1, (int)y2, (int)y2_1, (int)y1}, 4);

        g.drawLine((int)x1, (int)y1, (int)x1_1, (int)y1_1);
        g.drawLine((int)x2_1, (int)y2_1, (int)x2, (int)y2);
        g.drawLine((int)x1, (int)y1, (int)x2, (int)y2);

        if(--n1 > 0){
            g.drawLine((int)x1, (int)y1, (int)x3, (int)y3);
            g.drawLine((int)x2, (int)y2, (int)x3, (int)y3);
            paintMore(n1, g, x1, x3, x2, y1, y3, y2);
            paintMore(n1, g, x2, x3, x1, y2, y3, y1);
        }
    }
}
Peter Lang
great, now I will just try to improve the coding. I was thinking about some better usage of recursion there.
sasquatch90
+1  A: 

Rewrite it in 25 lines of F#? ;-)

Jon Harrop
A: 

There are many ways to shorten the code (and the suggestions from Eric and Mihir are a good start), but I would focus on clarity. Variables like x1 and y1 scream for a Point struct to hold these pairs together. You could then pass these points in an array.

Steven Sudit