Cookie Clicker in Java











up vote
1
down vote

favorite












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);
}
}









share|improve this question




























    up vote
    1
    down vote

    favorite












    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);
    }
    }









    share|improve this question


























      up vote
      1
      down vote

      favorite









      up vote
      1
      down vote

      favorite











      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);
      }
      }









      share|improve this question















      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);
      }
      }






      java beginner game swing awt






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Jan 11 at 22:28









      Sᴀᴍ Onᴇᴌᴀ

      8,12861751




      8,12861751










      asked Jan 11 at 22:06









      Dexter Thorn

      622725




      622725






















          2 Answers
          2






          active

          oldest

          votes

















          up vote
          1
          down vote



          accepted










          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.






          share|improve this answer























          • Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
            – Dexter Thorn
            Jan 12 at 20:07












          • You already track the buildings level inside the building, I already used it...
            – Timothy Truckle
            Jan 12 at 20:28






          • 1




            Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
            – Olivier Grégoire
            Oct 9 at 15:21


















          up vote
          0
          down vote













          How can i start this with BlueJ?





          share








          New contributor




          Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
          Check out our Code of Conduct.


















            Your Answer





            StackExchange.ifUsing("editor", function () {
            return StackExchange.using("mathjaxEditing", function () {
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            });
            });
            }, "mathjax-editing");

            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "196"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });














            draft saved

            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184892%2fcookie-clicker-in-java%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            2 Answers
            2






            active

            oldest

            votes








            2 Answers
            2






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            1
            down vote



            accepted










            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.






            share|improve this answer























            • Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
              – Dexter Thorn
              Jan 12 at 20:07












            • You already track the buildings level inside the building, I already used it...
              – Timothy Truckle
              Jan 12 at 20:28






            • 1




              Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
              – Olivier Grégoire
              Oct 9 at 15:21















            up vote
            1
            down vote



            accepted










            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.






            share|improve this answer























            • Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
              – Dexter Thorn
              Jan 12 at 20:07












            • You already track the buildings level inside the building, I already used it...
              – Timothy Truckle
              Jan 12 at 20:28






            • 1




              Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
              – Olivier Grégoire
              Oct 9 at 15:21













            up vote
            1
            down vote



            accepted







            up vote
            1
            down vote



            accepted






            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.






            share|improve this answer














            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.







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited Jan 12 at 10:39

























            answered Jan 12 at 10:16









            Timothy Truckle

            4,748416




            4,748416












            • Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
              – Dexter Thorn
              Jan 12 at 20:07












            • You already track the buildings level inside the building, I already used it...
              – Timothy Truckle
              Jan 12 at 20:28






            • 1




              Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
              – Olivier Grégoire
              Oct 9 at 15:21


















            • Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
              – Dexter Thorn
              Jan 12 at 20:07












            • You already track the buildings level inside the building, I already used it...
              – Timothy Truckle
              Jan 12 at 20:28






            • 1




              Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
              – Olivier Grégoire
              Oct 9 at 15:21
















            Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
            – Dexter Thorn
            Jan 12 at 20:07






            Thank you for that excellent answer. But there is one point I would disagree. The logic that allows to unlock a building can't be implemented inside the building, because it depends on conditions that in turn depend on variables the object has no access to. This may could result into ugly looking code, or what do you think?
            – Dexter Thorn
            Jan 12 at 20:07














            You already track the buildings level inside the building, I already used it...
            – Timothy Truckle
            Jan 12 at 20:28




            You already track the buildings level inside the building, I already used it...
            – Timothy Truckle
            Jan 12 at 20:28




            1




            1




            Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
            – Olivier Grégoire
            Oct 9 at 15:21




            Use a LinkedList instead of an ArrayList to have access to the method removeFirst() and truly avoid magic number.
            – Olivier Grégoire
            Oct 9 at 15:21












            up vote
            0
            down vote













            How can i start this with BlueJ?





            share








            New contributor




            Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






















              up vote
              0
              down vote













              How can i start this with BlueJ?





              share








              New contributor




              Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.




















                up vote
                0
                down vote










                up vote
                0
                down vote









                How can i start this with BlueJ?





                share








                New contributor




                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.









                How can i start this with BlueJ?






                share








                New contributor




                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.








                share


                share






                New contributor




                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.









                answered 7 mins ago









                Smoky

                1




                1




                New contributor




                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.





                New contributor





                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.






                Smoky is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
                Check out our Code of Conduct.






























                    draft saved

                    draft discarded




















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.





                    Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                    Please pay close attention to the following guidance:


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184892%2fcookie-clicker-in-java%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    404 Error Contact Form 7 ajax form submitting

                    How to know if a Active Directory user can login interactively

                    TypeError: fit_transform() missing 1 required positional argument: 'X'