4
\$\begingroup\$

I was wondering how I could streamline the following code. This was the project assigned to us:

The Canadian Forest Service wants to do a simple simulation of the growth and pruning of forests. Each forest has a name and exactly 10 trees. The trees are planted when they are 1' to 5' tall, and each tree has a individual growth rate of 50%-100% per year. For the simulation new trees are constructed randomly within these bounds. A forest is reaped (by lumberjacks) on demand - all trees above a specifed height are cut down and replaced with new trees.

The user interface to the simulation must allow the user to:

  • Display the current forest (with tree heights to 2 decimal places)
  • Discard the current forest and create a new forest
  • Simulate a year's growth in the current forest
  • Reap the current forest of trees over a user specified height, replacing the reaped trees with random new trees.
  • Save the information about the current forest to file (named after the forest)
  • Discard the current forest and load the information about a forest from a file.
import java.util.Scanner;
import java.util.*;
import java.io.*;
public class Main {

    private static Scanner scan = new Scanner(System.in);
    public static void main(String[] args) {

//initialize variables for Forest object, name, height and other inputs
Forest thisForest = null;
        char userInput = ' ';
        String forestName;
        int height = 0;
        String loadName = null;

        while((userInput != 'x') && (userInput != 'X' )){

            switch (userInput) {

//sets the first prompt      
                case ' ':
                    System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                    userInput = scan.next().charAt(0);

                    break;

//Displays the output but only if the forest exists      
                case 'D':
                case 'd':

                    if(thisForest != null){
                        thisForest.display();
                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);
                    }else{
                        System.out.println("No Forrest.\n");
                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);
                    }
                    break;

                case 'N':
                case 'n':

                    System.out.print("What is the name of the forest: ");
                    forestName = scan.next();
                    thisForest = new Forest(forestName);

                    System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                    userInput = scan.next().charAt(0);

                    break;

                case 'Y':
                case 'y':
//grows the equivalent of a "year".   
                    if(thisForest != null){
                        thisForest.yearGrowth();

                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);
                    }else{
                        System.out.print("No forest exists.\n");
                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);
                    }
                    break;

                case 'R':
                case 'r':

                    //reaps, but only if the forest exists. Catches possible exceptions.
                    if(thisForest != null){
                        try{
                            System.out.print("What height to reap at :");
                            height = scan.nextInt();
                        }catch(NumberFormatException e){
                            System.out.println("ERROR: Invalid height\n");

                            System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                            scan.nextLine();
                            userInput = scan.next().charAt(0);
                            break;
                        } catch(InputMismatchException e){
                            System.out.println("ERROR: Invalid height\n");

                            System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                            scan.nextLine();
                            userInput = scan.next().charAt(0);
                            break;
                        }
//end of try-catch       
                        thisForest.reap(height);

                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        scan.nextLine();
                        userInput = scan.next().charAt(0);
                        break;
//output if there is no forrest       
                    }else{
                        System.out.print("There is no forest to reap.\n");

                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        scan.nextLine();
                        userInput = scan.next().charAt(0);
                    }
                    break;



                case 'S':
                case 's':
//saves the program      
                    if(thisForest != null){
                        try{
                            Forest.saveForest(thisForest);
                        }catch(IOException e){
                            System.out.println("Cannot save.");
                        }
                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);
                    }else{
                        System.out.println("No Forrest exists to save.");
                        System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                        userInput = scan.next().charAt(0);

                    }
                    break;

                case 'L':
                case 'l':
// loads the program      
                    try{
                        System.out.print("What is the name of the Forest: ");
                        loadName = scan.next();
                        thisForest = Forest.loadForest(loadName);

                    }catch(IOException e){
                        System.out.println("Cannot load.");
                    }
                    System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                    userInput = scan.next().charAt(0);
                    break;

                default:
                    System.out.println("ERROR: Invalid Option.");
                    System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
                    userInput = scan.next().charAt(0);
                    break;
            }
        }
//after x is received and the while loop breaks.
        System.out.println("Goodbye");

    }
}

Second Class:

import java.io.*;
import java.util.*;
public class Forest implements Serializable{

//creates variables and constants
    private final int MAX_NUM_TREES = 10;
    private String name;
    private Tree[] arrayOfTrees;
    int index;
    public Forest(String forestName){

        index = 0;
        name = forestName;
        arrayOfTrees = new Tree[MAX_NUM_TREES];

        for(index = 0; index < arrayOfTrees.length; index++){

            arrayOfTrees[index] = new Tree();

        }
    }

    public void display(){
// displays the array of trees and the index
        index = 0;

        if(name != null){

            System.out.println(name);
            for(index = 0; index < arrayOfTrees.length; index ++){
                System.out.printf("%2d   :   %s\n", (index + 1), arrayOfTrees[index]);
            }
        }else{
            System.out.println("No forest.");
        }

    }
   public void yearGrowth(){
//grows each tree in the array
        index = 0;

        for(index = 0; index < arrayOfTrees.length ; index ++){

            arrayOfTrees[index].grow();
        }

    }
   public void reap(int reapHeight){
        //reaps the trees and prints out the old and new information
        index = 0;


        for(index = 0; index < arrayOfTrees.length; index++){

            if(arrayOfTrees[index].getHeight() >= reapHeight){

                System.out.println("Cut " + (index+1) + " : " + arrayOfTrees[index] );
                arrayOfTrees[index] = new Tree();
                System.out.println("New " + (index+1) + " : " + arrayOfTrees[index] );

            }
        }

    }
public static void saveForest(Forest forest) throws IOException {
//saves the forest
        String name = forest.getName();
        ObjectOutputStream toStream;

        toStream = new ObjectOutputStream(new FileOutputStream(name));
        toStream.writeObject(forest);
        toStream.close();
    }

   public static Forest loadForest(String fileName) throws IOException {
//loads the forest
        ObjectInputStream fromStream = null;
        Forest local;

        fromStream = new ObjectInputStream(new FileInputStream(fileName));
        try {
            local = (Forest)fromStream.readObject();
        }catch (ClassNotFoundException e) {
            System.out.println(e.getMessage());
            return(null);
        }finally{
            try {
                if (fromStream != null) {
                    fromStream.close();
                }
            } catch (IOException e) {
                System.out.println(e.getMessage());
                return(null);
            }
        }
        return(local);
    }
    public String getName(){

        return (name);
    }
}

Third Class

import java.util.Random;
import java.util.*;
import java.io.*;

public class Tree implements Serializable{

//creates the variables as the
    private double height;
    private double growthRate;
    private static Random rand = new Random();
    final double MIN_HEIGHT = 1;
    final double MIN_GROWTH_RATE = 0.5;
    final double MAX_HEIGHT = 5;
    final double MAX_GROWTH_RATE = 1.0;

    public Tree() {
//creates tree with a height and a growth rate
        Random rand = new Random();

        height = (MIN_HEIGHT + ((Math.random() * (MAX_HEIGHT - MIN_HEIGHT))));
        growthRate = (MIN_GROWTH_RATE + (Math.random() * (MAX_GROWTH_RATE - MIN_GROWTH_RATE)));


    }

    public double grow(){
//tree grows and returns height

        height = height * (1 + growthRate);
        return height;


    }

    public double getHeight(){

        return (height);

    }

    public double getGrowthRate(){

        return (growthRate);

    }

    public String toString(){
//toString formats the output with height and growthrate

        return (String.format("%7.2f (%2d%% pa)", height, ((int)(growthRate * 100))));

    }
}

```
\$\endgroup\$
1
\$\begingroup\$

In your Forest class I don't like how the int index; variable is used. I realize that it's working for you because you always set it to 0 before looping, but local variables are that are scoped to a function is the standard way of doing it.

This

    index = 0;


    for(index = 0; index < arrayOfTrees.length; index++){

can be replaced with

    for(int index = 0; index < arrayOfTrees.length; index++){

That will do the same thing and yet be less confusing.

\$\endgroup\$
0
\$\begingroup\$

Javadoc, Javadoc, Javadoc!


import java.util.*;
import java.io.*;

I'm not a fan of wildcard imports. You might actually end up with the wrong function being imported. Also any modern IDE will happily manage the imports for you.


private static Scanner scan = new Scanner(System.in);

scan is an action, scanner would be a better name.


Forest thisForest = null;
char userInput = ' ';
String forestName;
int height = 0;
String loadName = null;

Either you initialize variables with default values, or you don't, be consistent.


System.out.print("(D)isplay, (N)ew, (Y)ear, (R)eap, (S)ave, (L)oad, e(X)it :");
userInput = scan.next().charAt(0);

You repeat this all over your code, find a way to do this only once (for example at the start of the while).


}catch(IOException e){
    System.out.println("Cannot save.");
}

Now that is helpful. If you do not know how to handle exceptions then do a e.printStackTrace() or System.err.println(e.getMessage()), because then the user at least has a chance to figure out why it did not work.

Swallowing and silencing exceptions is only acceptable if you really know why you want this.


private final int MAX_NUM_TREES = 10;

I know this has been declared in the description, but a better way would be to have the Forest accept the number of trees and the growth rate in the constructor and have the constant in the Main class. That way your Forest class can be easier reused.


int index;

Nice idea to reuse the constantly used int. However, I hate to break it to you that is completely unnecessary, actually it is the opposite of helpful because it makes your code more error prone and a little bit harder to read. If you start reading the class you see index and wonder why the Forest requires to hold an index of itself.

Whether or not the JVM has to allocate one integer doesn't matter. More so, as int is not an Object, neither does its allocation influence garbage collection in away. It is way fucking cheap to allocate an int, always. Well, if you have something like a million ints or so, we can start talking about having them cached, but not a single one.


private Tree[] arrayOfTrees;

Consider using a List instead of an array, as it has more convenience functions and is easier to handle.

You still enforce the limit on the number of trees anyway.


public Forest(String forestName){
    name = forestName;

Normally you'd do this:

public Forest(String name){
    this.name = name;

Don't try to repeat already available information, for example if you have a class which handles the user-input, don't try to have this chain:

org.yourdomain.application.inputhandling.InputHandler.handleInput(InputStream)

You constantly repeat yourself and add nothing of value to the names.

org.yourdomain.application.ui.StreamHandler.process(InputStream)

So I know it is part of the "user interface" package, so the class handles the input from a stream, and then it processes the input.

Naming is hard, though, takes a lot of practice to get halfway good at it.


public void display(){

Ideally you would only provide means to get the needed information from Forest and the main class would handle displaying it.

That decouples your class from the environment it is being used in.


if(name != null){

What? How or why is this a valid state?

public Forest(String name){
    if (name == null || name.trim().isEmpty()) {
        throw new IllegalArgumentException("The forest must have a name.");
    }

public static void saveForest(Forest forest) throws IOException {

Why static? If this would be an instance method, extending classes would have a chance to implement their own logic.

Moreover, this should accept a directory (null for current).


ObjectOutputStream toStream;

toStream = new ObjectOutputStream(new FileOutputStream(name));

Why not directly assign it? Moreover, you should use a try with resources statement to make sure that the stream is always closed.

Creating filestreams has the potential to leak file handles if they have not been closed.


return(local);

Random parenthesis? return is a keyword, not a function.


final double MIN_HEIGHT = 1;
final double MIN_GROWTH_RATE = 0.5;
final double MAX_HEIGHT = 5;
final double MAX_GROWTH_RATE = 1.0;

Ideally these would also be properties handed to the Forest. Consider the following structure:

class Forest
    public static final int DEFAULT_MAX_TREES = 10;
    public static final double DEFAULT_MIN_TREEHEIGHT = 0.5d;
    ...

    public Forest(name) {
        this(name, DEFAULT_MAX_TREES, DEFAULT_MIN_TREEHEIGHT, ...)
    }

    public Forest(name, int maximumTrees, double minTreeHeight) {
        trees.add(new Tree(minTreeHeight, ...)

But I like that the Tree is responsible for its growing, that gives extending classes a chance to easily change the logic and also gives you the possibility to mix different Tree classes in the Forest.

trees.add(new Tree(...));
trees.add(new SlowGrowingTree(...));
trees.add(new FastGrowingTree(...));
trees.add(new SickTree(...));

Random rand = new Random();

As a heads up, please always be aware how the default constructor of a PRNG works! In this case this usage is okay because the default constructor of Random tries very, very hard to not give you to matching instances ever. However, the .NET Random class for example is (or at least was for a decade) initialized with the current timestamp in seconds, so if you did this:

Random a = new Random();
Random b = new Random();
Random c = new Random();
Random d = new Random();

All four instances would return the same numbers.

That's something to keep in mind, always check the documentation if you are safe or not.


Overall looks quite nice, good job.

\$\endgroup\$
0
\$\begingroup\$

You can toUppercase() your userInput. This will reduce the complexity. No need to check if 'a' or 'A', etc.

You should use an ENUM for userInput. Instaed of

case 'D':

you'd have:

case UserChoice.DISPLAY:

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.