This is my first graphical application I have made in my life (except HTML and Javascript-Applications, if that counts). It's a simple implementation of Cookie Clicker, the famous browser game. Unfortunately the size of the buttons equals the size of columns. That does not look good. I have seen a solution that uses two layouts. A GridLayout, and an additional FlotLayout. But the code looks that ugly, that I don't want use this way of programming.

I would be very thankful if you have tips for me how to improve the code quality! Programming a graphical application is much more harder and complex than writing textual applications.

import java.util.*;

import java.awt.*;

import java.awt.event.*;

import javax.swing.*;



public class CookieClicker extends JFrame {

    // non graphical variables

    private int cookies = 0;

    private int clicker = 1;

    private int clickerPrice = 20;



    // graphical variables

    int numberOfColumns = 5;



    Container container;



    JLabel cookieLabel;

    JButton increaseCookiesButton;



    JLabel clickerLabel;

    JButton increaseClickerButton;



    // buildings

    Building bakery;

    boolean bakeryUnlocked;



    Building robot;

    boolean robotUnlocked;



    Building factory;

    boolean factoryUnlocked;



    public CookieClicker() {

        container = getContentPane();

        container.setLayout(new GridLayout(5, 1));



        bakery = new Building("Bakery", 0, 1, 20);

        bakeryUnlocked = false;



        robot = new Building("Robot", 0, 5, 100);

        robotUnlocked = false;



        factory = new Building("Factory", 0, 10, 200);

        factoryUnlocked = false;



        // produce cookies by hand

        cookieLabel = new JLabel("Cookies: " + cookies);

        increaseCookiesButton = new JButton("Increase Cookies");

        increaseCookiesButton.addActionListener(new ActionListener() {

            public void actionPerformed(ActionEvent e) {

                cookies += clicker;

            }

        });



        // improve clicking production rate

        clickerLabel = new JLabel("Clicker Level: " + clicker);

        increaseClickerButton = new JButton("Improve Clicker (Costs: " + clickerPrice + ")");

        increaseClickerButton.addActionListener(new ActionListener() {

            public void actionPerformed(ActionEvent e) {

                increaseClicker();

            }



            private void increaseClicker() {

                if(cookies >= clickerPrice) {

                    clicker++;

                    cookies -= clickerPrice;

                    clickerPrice *= 2;

                    JOptionPane.showMessageDialog(null, "You have improved your clicker!");

                } else {

                    JOptionPane.showMessageDialog(null, "You have not enough money!");

                }

            }

        });



        java.util.Timer actualizeProgress = new java.util.Timer();

        actualizeProgress.scheduleAtFixedRate(new TimerTask() {

            public void run() {

                cookieLabel.setText("Cookies: " + cookies);

                clickerLabel.setText("Clicker Level: " + clicker);

                increaseClickerButton.setText("Improve Clicker (Costs: " + clickerPrice + ")");

            }

        }, 0, 25);



        java.util.Timer getMoreBuildings = new java.util.Timer(); 

        getMoreBuildings.scheduleAtFixedRate(new TimerTask() {

            public void run() {

                if (bakeryUnlocked == false && clicker >= 2) {

                    bakery.unlock();

                    bakeryUnlocked = true;

                }

                if (robotUnlocked == false && bakery.getLevel() >= 2) {

                    robot.unlock();

                    robotUnlocked = true;

                }         

                if (factoryUnlocked == false && robot.getLevel() >= 2) {

                    factory.unlock();

                    factoryUnlocked = true;

                }

            }

        }, 0, 2000);



        java.util.Timer produceWithBuildings = new java.util.Timer();

        produceWithBuildings.scheduleAtFixedRate(new TimerTask() {

            public void run() {

                cookies += bakery.getProductionRate() + robot.getProductionRate() + factory.getProductionRate();

            }

        }, 0, 1000);



        container.add(cookieLabel);

        container.add(increaseCookiesButton);

        container.add(new JLabel("")); // blank label

        container.add(clickerLabel);

        container.add(increaseClickerButton);

    }



    public class Building {

        // non graphical variables

        private String name;

        private int level;

        private int productionRate;

        private int costs;



        // graphical variables

        JLabel label;

        JButton button;



        public Building(String name, int level, int productionRate, int costs) {

            // non graphical variables

            this.name = name;

            this.level = level;

            this.productionRate = productionRate;

            this.costs = costs;



            // graphical variables

            label = new JLabel();

            button = new JButton();

            button.addActionListener(new ActionListener() {

                @Override

                public void actionPerformed(ActionEvent e) {

                    improve();

                }

            });

        }



        public int getLevel() {

            return level;

        }



        public void unlock() {

            numberOfColumns += 3;

            container.setLayout(new GridLayout(numberOfColumns, 1));

            container.add(new JLabel(""));

            container.add(label);

            container.add(button);

            setSize(210, getHeight() + 120);

            actualize();

        }



        public void improve() {

            if(cookies >= costs) {

                level++;

                cookies -= costs;

                costs *= 2;

                JOptionPane.showMessageDialog(null, "You have improved the " + name + "!");

            } else {

                JOptionPane.showMessageDialog(null, "You have not enough money!");

            }

            actualize();

        }



        public int getProductionRate() {

            return productionRate * level;

        }



        public void actualize() {

            label.setText(name + " Prod. Rate: " + getProductionRate());

            button.setText("Improve (costs: " + costs + ")");

        }

    }



    public static void main(String args) {

        CookieClicker cookieClicker = new CookieClicker();

        cookieClicker.setTitle("Cookie Clicker");

        cookieClicker.setSize(210, 200);

        cookieClicker.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        cookieClicker.setVisible(true);

    }

}

Thanks for sharing your code.

It is a really good for the first attempt.

Nevertheless there is something to mention...

Naming

Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.

Choose your names from the problem domain

You have some identifiers which are named after their technical implementation like this:

Container container;

JButton increaseCookiesButton;

JButton button;

They should have names that reveal their task within your application.

Container coockieDisplay;

JButton cookiesIncreaser;

JButton buildingImprover;

Naming Conventions

Please read (and follow) the
Java Naming Conventions

eg.:

Your boolen variables and methods returning a boolean should start with is, has, can or alike.

Methods names should start with a verb.

Class and variable names should be nouns.

Don't suprize your readers

Also you have variable names that start with verbs like

JButton increaseCookiesButton;

Timer actualizeProgress = new java.util.Timer();

Timer getMoreBuildings = new java.util.Timer();

But accoring to the Java Naming Conventions
only methods should start with a verb. So they might better be named like this:

JButton cookiesIncreaser;

Timer progressUpdater = new java.util.Timer();

Timer buildingsUnlocker = new java.util.Timer();

Coding practice

Magic numbers

You code has some litaral numbers with special meaning like here:

if (bakeryUnlocked == false

        && clicker >= 2) {

This should be extracted to *constants with a name that expresses the meaning:

private static final int MINIMUM_UPGRADE_LEVEL = 2;
// ...
if (bakeryUnlocked == false
&& clicker >= MINIMUM_UPGRADE_LEVEL) {

Use of boolean

At some places you compare a boolean variable with a literal boolean value:

if (bakeryUnlocked == false

        && clicker >= 2) {

    bakery.unlock();

Don't do that use the boolean variable directly and use the negation opertator if needed:

if (!isBakeryUnlocked 

        && clicker >= 2) {

    bakery.unlock();

Avoid unnecessary members

You have a variable container which is a member in your class CookieClicker. But in CookieClicker you only need it to initialize its content. You never use it outside the constructor. Therefore it should be a local variable in the constructor.

You use container in your other (named inner) class Building. But there you access the variable of Buildings outer class CookieClicker. There are some cases where this is OK, especially when the accessing class is an anonymous inner class. But in this case you should pass the container as constructor parameter to Building:

public CookieClicker() {

    container = getContentPane();

    container.setLayout(new GridLayout(5, 1));



    bakery = new Building("Bakery", 0, 1, 20, container);

    bakeryUnlocked = false;



    robot = new Building("Robot", 0, 5, 100, container);

    robotUnlocked = false;



    factory = new Building("Factory", 0, 10, 200, container);

// ...



public class Building {

    // ...

    // graphical variables

    JLabel label;

    JButton button;

    Container container;



    public Building(String name, int level, int productionRate, int costs, Container container) {

        //...

        // graphical variables

        this.container = container;

    //...

Same is true for the instances of JButton in this class.

Odd ball solutions

You make a difference in your logic for the initial phase and after having unlocked the first building. Therefore you have similar code at two places.

I'd suggest to create one more instance of class Building to reuse its logic even for the initial phase.

Tell! Don't ask! - avoid feature envy

When unlocking the buildings you acquire the Buildings level property an make a decision. But accessing an objects property violates the information hiding principle / encapsulation. The Building class should know itself how to make that decision and provide a method hasReachedMinimumLevel():

   // in Building

    public boolean hasReachedMinimumLevel() {

        return 2<=level;

    }

   // ...

// in CoockieClicker

  if (!isRobotUnlocked && bakery.hasReachedMinimumLevel()) {

     robot.unlock();

OOP

Inheritance

In OOP we inherit from a super class if we extend its behavior. This is: we override a method to do something more and/or something different then the same method in the super class.

Your class extends JFrame but does not change a JFrames behavior. You only configure its content. So your class should rather use a JFrame instead of extending it:

public class CookieClicker  {

    // non graphical variables

    // ...

    public CookieClicker(JFrame theFrame) {

      container = theFrame.getContentPane();        

    // ...



    public static void main(String args) {

        JFrame theFrame = new JFrame();

        CookieClicker cookieClicker = new CookieClicker(theFrame);

        theFrame.setTitle("Cookie Clicker");

        theFrame.setSize(210, 200);

        theFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        theFrame.setVisible(true);

    }       

}

avoid procedural approaches

Procedural approaches are not bad of their own.

But Java is an Object Oriented programming language and if you want to become a good Java programmer you should start looking for more OO like solutions.

But OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

• information hiding / encapsulation


• single responsibility


• separation of concerns


• KISS (Keep it simple (and) stupid.)


• DRY (Don't repeat yourself.)


• "Tell! Don't ask."


• Law of demeter ("Don't talk to strangers!")

A good example is the way you unlock the buildings.

You introduce boolean variables to track the building states. A more OO-ish approach would be to hold the locked buildings in a List.
Then I'd remove the first element from that list and unlock it until the list is empty:

Building dummyBuildingToAvoidAnotherOddBallSolution = new Building("",0,0,0){ // anonymous inner class

    @Override

    public int hasReachedMinimumLevel() {

        return 2 <= clicker;

    }

}

List<Building> lockedBuildings = 

     new ArrayList<>(

        Arrays.asList(dummyBuildingToAvoidAnotherOddBallSolution, bakery, robot, factory));



buildingsUnlocker.scheduleAtFixedRate(new TimerTask() {

    private final int FIRST_IN_QUEUE = 0; // avoid "magic number", cannot be static in non-static inner class

    Building activeBuilding = lockedBuildings.remove(FIRST_IN_QUEUE);

    @Override

    public void run() {

       if(!lockedBuildings.isEmpty()) {

         if(activeBuilding.hasReachedMinimumLevel()) {

            activeBuilding = lockedBuildings.remove(FIRST_IN_QUEUE);

            activeBuilding.unlock();

         }

       }

    }

}, 0, 2000);

With that approach you don't need the booleanvariables at all. The logic itself becomes shorter and is able to deal with more buildings without any further modification.

How can i start this with BlueJ?